[CDRIVER-4736] Custom _mongoc_usleep function Created: 27/Sep/23 Updated: 11/Jan/24 Resolved: 11/Jan/24 |
|
| Status: | Closed |
| Project: | C Driver |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 1.26.0 |
| Type: | New Feature | Priority: | Minor - P4 |
| Reporter: | itrofimow N/A | Assignee: | Kevin Albertson |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Description |
|
Hi!
At my workplace we use libmongoc in a single-threaded mode atop of stackful-coroutines-based asynchronicity: we have a small amount of threads that manage lots of coroutines, and instead of suspending a thread for I/O or sleeping we suspend a coroutine, while a thread is free to process other bits of workload, if any.
It works wonders with custom implementation of mongoc_stream_t, however because of blocking usleep call usage in topology rescan we had to patch libmongoc to allow users to provide their own implementation of _mongoc_usleep (coroutine-friendly nonblocking sleep in our case).
Would you be interested in reviewing/accepting such a patch, which adds a possibility for users to provide their own sleep implementation? |
| Comments |
| Comment by Githook User [ 11/Jan/24 ] |
|
Author: {'name': 'itrofimow', 'email': 'i.trofimow@yandex.ru', 'username': 'itrofimow'}Message: |
| Comment by itrofimow N/A [ 22/Nov/23 ] |
|
Hi kevin.albertson@mongodb.com ! Happy to hear that; i've addressed your comments, please see the updated PR. |
| Comment by Kevin Albertson [ 20/Nov/23 ] |
|
Thank you for the feedback i.trofimow@yandex.ru. I do not see a feasible workaround either for the open-source users of userver. The PR has been re-opened and reviewed. |
| Comment by itrofimow N/A [ 16/Nov/23 ] |
|
Hi kevin.albertson@mongodb.com ! Sorry for being late with the reply, vacation.
Patching C driver is indeed sufficient for the needs of userver, as we do inhouse, however it is not really feasible for our open-source users to do so.
I find the decision to not add a possibility to set a custom usleep function very unfortunate: as it stands right now libmongoc does support some async runtimes, but the possibility of blocking in usleep in topology rescan effectively throws this out of the window.
If the proposed API change is off limits, would it be possible to export "_mongoc_usleep" function from the library as a weak symbol, so users could override it if needed? |
| Comment by PM Bot [ 03/Nov/23 ] |
|
There hasn't been any recent activity on this ticket, so we're resolving it. Thanks for reaching out! Please feel free to reopen this ticket if you're still experiencing the issue, and add a comment if you're able to provide more information. |
| Comment by Kevin Albertson [ 20/Oct/23 ] |
|
Discussion with the team resulted in a preference not to merge this PR. At present, this appears to be an unsupported use-case of the C driver: Supporting suspending threads before blocking in a single-threaded mongoc_client_t. Is patching the C driver sufficient for the needs of userver? Revising this ticket type to "New Feature". If this use-case is supported by the C driver in the future, this may be re-evaluated. |
| Comment by itrofimow N/A [ 17/Oct/23 ] |
|
Thank you for the link.
I've read through the forum post, and i understand the request and motivation behind it clearly, however it's not at all related to out use case: even though our I/O implementation is indeed non-blocking, it looks sync and behaves almost exactly as a sync implementation would, thus playing nicely with what libmongoc provides as of now.
Even if some kind of an event-based API was available today, we would still prefer to use the API we have right now, just because it's easier to use and works fine with our I/O implementation. To summarize: i don't find CDRIVER-4708 related to the ticket at hand and still think that the feature proposed here would a) be of use in some cases even in the presence of an event-based API b) has a much more predictable estimate to implement, and a much more straightforward way to leverage its benefits
kevin.albertson@mongodb.com what do you think? |
| Comment by Kevin Albertson [ 16/Oct/23 ] |
Sorry. I learned that ticket is intentionally internal. The public facing request is tracked here: https://feedback.mongodb.com/forums/924286-drivers/suggestions/47080057-asynchronous-variant-of-mongodb-c-driver |
| Comment by itrofimow N/A [ 14/Oct/23 ] |
|
Hi kevin.albertson@mongodb.com! Apparently i don't have an access to CDRIVER-4708, could you please grant me a read access to the ticket?
More about the use case: We use libmongoc at userver. Userver is based on a hand-written stackfull-coroutines asynchronicity, which consists of three key components: a scheduler, an event-loop and a thread-pool, on which user code is executed within aforementioned coroutines. Consider following situation: user wants to read some data from a socket. In userver terms it would look something like 'socket.ReadSome(buffer, buffer_size);', and what happens behind the scenes is:
The process for a `sleep` is almost exactly the same, but instead of a socket read-interest we set up a timer in event-loop.
All this machinery allows one to have a non-coloring async without any `async`/`await` keywords, with a straightforward sync-looking code and a very few OS-threads, however it requires user-code to never block on its own, and use userver-provided I/O, sleep etc. implementations instead (basically, "don't block the executor"). Now here is where we encounter a problem with libmongoc: its `usleep` usage blocks our worker-threads, breaking the requirement of "don't block in worker-threads" and degrading overall service performance. With the provided patch we are able to replace `usleep` usage with the userver-provided sleep implementation, which doesn't block the whole thread, but rather suspends a coroutine, when the thread is able to execute other coroutines.
Edit: userver doesn't play that well with other async-aware frameworks, but there are means to integrate them somehow. |
| Comment by Kevin Albertson [ 13/Oct/23 ] |
|
Thank you for the PR i.trofimow@yandex.ru. There is planned investigation into better support for async runtimes for CDRIVER-4708. That may provide a solution without adding a hook for the sleep function. This ticket has been noted in CDRIVER-4708 to consider. My inclination is to decline the PR. Aside: Knowing more about the use-case may help in investigating CDRIVER-4708. Example: What async runtime is used? Does the application integrate with other frameworks that are async aware? |
| Comment by itrofimow N/A [ 12/Oct/23 ] |
|
Here's the patch: https://github.com/mongodb/mongo-c-driver/pull/1442 |
| Comment by PM Bot [ 10/Oct/23 ] |
|
Hi i.trofimow@yandex.ru! If this is still an issue for you, please open Jira to review the latest status and provide your feedback. Thanks! |
| Comment by Esha Bhargava [ 02/Oct/23 ] |
|
i.trofimow@yandex.ru You are welcome to submit a PR for review. |
| Comment by PM Bot [ 27/Sep/23 ] |
|
Hi i.trofimow@yandex.ru, thank you for reporting this issue! The team will look into it and get back to you soon. |