-
Notifications
You must be signed in to change notification settings - Fork 10.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gevent compatibility #4629
Comments
@zj8487 did you get gRPC(python) working with gevent.monkeypatch? Could you please share the work-around. |
@vinays |
grpc make async io well, why do you want to make grpc client and server code to work with gevent? they do the same thing. |
Thanks @zj8487 @mayflaver in our case 'grpc' and 'gevent' are transitive dependencies of gcloud-python and gunicorn respectively in our legacy app. If I was writing an app from scratch I would agree with you, but in this case these dependencies are not enforced by me. :( |
Reopening as this seems to warrant investigating. |
@soltanmm any resolution on this problem? It will be really help if we can get a rough timeline for it. |
@vinays There's no timeline for it at present (and without in-depth |
@vinays We have a hard time justifying interoperability with a library feature that seems to be unable to work with standard library elements in Python 3 because it literally changes the standard library and incompletely so. That said, it's worth documenting this, and I don't know much about |
@vinays Actually, this issue comment might be of relevance to you: benoitc/gunicorn#927 (comment)
EDIT: Hmm, nevermind. Reading further down the thread I see a face-heel-turn... |
Thanks @soltanmm for looking into the issue. @mickgarvey @grourk might have more insight into why we did not choose other options in the list. From what I know, since our app is IO bound 'sync' worker does not scale well because of blocking IO. @soltanmm did your investigation conclude a root cause of the issue? Do you know what part of standard library patching was causing the hang in Python 2.7? Would excluding 'thread' from patch_all, fix the issue? I can test that myself, but I am looking for any side-effects you might know and most likely won't surface itself in a simple test. Any other information from your investigation might also help. |
@vinays I'll have to guiltily admit that after reading about what I have a strong feeling it's just going to be the usual, "Green-threads cannot be cooperative if they block on something that isn't cooperative," sort of deal, though (where 'something that isn't cooperative' could be anything the |
It's basically that. gRPC Python uses multiple threads for a server (e.g. one for the server receiving messages, one to pass the call data along to the application). These trip over each other when they don't cooperatively release polling to Short of getting in-depth knowledge of I'm not going to close this as it's possible that there exists a way to force the use of standard Python threads despite monkey patching where they are necessary, or that some other hacky-solution-against-a-hacky-solution exists, but we will not attempt to support gevent's monkeypatching for the time being. |
I'm still coming up to speed on what's being asked here, but from what I've gathered so far, I suspect it will be a lot: one of the core design assumptions of gRPC Python is that a reasonable number of threads will always be available to be devoted to blocking operations such as C extension calls and calls into application code. Rolling back that assumption and moving forward with a different structure are going to be substantial work, if they're something that we ever do at all. I feel like I'm still missing quite a bit. What's the minimum sample program you could write that you would expect to work but that doesn't? |
@soltanmm thanks for the explanation, it resonates with my understanding. |
@vinays: yes, a code sample would be delightful - something that is enough to illustrate a use of gRPC Python, how you expected gRPC Python to behave, and how gRPC Python actually behaved. |
@nathanielmanistaatgoogle here is some sample code to illustrate the issue. Let me know if you have any questions. |
Just another use-case we have been running into. We are using graphql (https://github.com/graphql-python/graphene) as our public API and grpc for internal communication. The graphql library supports running resolving requests in parallel using gevent but the problem is, that any grpc calls we are making (in different resolve methods) are run serially, which now is the cause of quite a few performance issues. I have just started to investigate to find any potential solutions or workarounds so the grpc requests can run in parallel, but good to see there are other people who are working towards interoperability. |
@nathanielmanistaatgoogle Thoughts on gevent/gevent#786? We could leave an unsupported (private, but documented) hook for the underlying thread pool executor somewhere in the gRPC implementation and allow users to swap it out post-import/pre-absolutely-anything-else if they want to at their own risk (facilitating the 'be selective' approach @jamadden mentioned). |
Better yet: we could do that and attach warnings (similar to the deprecation warnings) to it in terminal emboldened firetruck-red all-caps font that the users have entered monkey territory and shouldn't be surprised if their hats get stolen and everything hurts twice as much. |
Thank you @jamadden for your investigation. Use of the same thread, with program control passing from application to gRPC Python to the gRPC Core and then to a blocking system call is an explicit part of gRPC’s design, is the basis for many of the performance improvements that we’ll be making in the next few months, and would be very hard to change. In particular major alterations would have to be made to the Core to enable its completion queues to be used in the manner gevent would want to use them. I’ve spoken with @ctiller about it and this work may be scheduled engineering effort if it is forecast to be valuable enough to be prioritized above other goals. There may be a certain recipe for monkeypatching that happens to work today but we wouldn’t be able to offer proper support (with test coverage and a promise not to break in the future) for anything based around selectively patching thread pools used internally within gRPC Python. In particular we have near-term plans to scale back how widely we use thread pools. If you want today to do something based on selectively patching it may work fantastically for you and then break in a future release. I would celebrate your bravery and wish you fun. |
@scorpioWalker: I think this is something you should create a new issue about. This one issue here is more integrating grpc with gevent. That's different from simply being able to run next to it. |
Google Datastore with Gunicorn blocks forever in app engine flexible. gevent==1.2.2 |
Hello! Would love to hear the status of this! |
We could use this as well, spawning processes is a waste of memory and cpu for heavy-I/O bound tasks... |
any update? |
I'm hoping to have a PR out this week for gevent support. |
hi @kpayson64 & devs, |
@kpayson64 I saw in your repo that you are still working on this. Thanks! Does it seem to work? |
@smetj This is very much at the top of my stack right now. I'm currently tackling some lingering test failures, but the bulk of this seems to be working. |
Sounds great! Thanks :) I hope we can get our gRPC working with locust soon. |
Super excited this is moving forward! Thanks for the feedback ... |
is aiohttp a viable alternative? |
Same! I am excited to try this out. |
Would be great to get #14561 merged in |
With #14561 in, you should now be able to use gevent. Note that you have to explicitly monkey patch Python, as well as enabling the gevent loop for Python (Before doing anything with gRPC)
This will be included in the 1.11 release. I'm closing this master issue, and separate issues can be created with any issues encountered using gevent. |
For those who want asyncio support, can #7910 be reopened or should we open a new issue? |
Or is #6046 that? |
Finally. Testing away! |
@kpayson64 thanks, great job! |
@mitar |
I opened #14876. Based on existing gist (linked in the issue) it seems this could be easily done. |
Hi, I want to make grpc client and server code to work with gevent, is grpc compatible with gevent?
The text was updated successfully, but these errors were encountered: