Could the schema execution be made even more async by adding an async executor

[japrogramer]

DjangoChannelsGraphqlWs/channels_graphql_ws/graphql_ws.py

Line 532 in bb043d4

result = await db_sync_to_async(self.schema.execute)(

executor=AsyncioExecutor(loop=asyncio.get_event_loop()),

if this line is changed to include the above as a parameter, does it raise this same issue
graphql-python/graphene-django#452

Really appreciate you sharing your code, I intend to migrate over to this implementation after i do some testing

[prokher]

@japrogramer That is exactly what we are experimenting on right now!

For example, if we provide return_promise=True to the schema executor (the line you cited above) then all the resolvers can be be made async functions executed in the main even loop (main thread where the ASGI server runs). I find this quite interesting, cause we can alway offload the function to a worker thread (with sync_to_async) from any resolver function. On the other hand, if we offload schema execution to the worker thread like we do it now, then any (even super-fast) resolver function is run in worker thread which can be considered as unnecessary overhead. Anyway, it is just intermediate thoughts...

Please, feel free to share you opinion. Why do you want/need an async executor? Do you want it to run in a separate thread or you want it to be based on the main even loop where web-server runs?

Probably, we need to introduce an options to make user able to switch between these variants.

Speaking of migration you mentioned. Thank you for you trust, but I suppose it is too early to use it in production. We need to elaborate more on this package until we can state it is production-ready. Moreover, I suppose that some backward compatibility braking changes can also be made.

[japrogramer]

@prokher one of the reason's I would like and currently have my own implementation to have an async executor is that I would like to make async calls to aioredis. Im not to experienced to know what the difference it would make to run in the main thread than to offloading to a separate thread in terms of scalability which is my main concern.

[prokher]

@japrogramer I've made the following. Now (since aaa5232) server processes requests (WebSocket messages) in parallel in different worker threads and AsyncioExecutor is used inside these worker threads. This allows to implement async resolvers and at the same time protects from locking the main eventloop where networking happens.

In spite of query and mutation resolvers seem to work fine, I indeed faced an issue with subscriptions. It looks very similar to the issue you posted to the graphql-core graphql-python/graphql-core#149 .

Now I need to think about a workaround. I always have an option to execute subscription "resolvers" ( subscribe and publish) in a worker thread with its own eventloop. This will have negative influence on performance, but will allow users to use async resolvers for subscriptions as well. And once graphql-core issue is fixed, this kluge can easily be removed. I am still not sure it is a proper way to.

[japrogramer]

@prokher I think the issue is more closely related to this graphql-python/graphene-django#452

[japrogramer]

I think this is the piece of code that may be preventing async subscription resolvers to execute as expected.
graphql-python/graphql-core#102 (comment)

[prokher]

It seems that finally (in v0.1.18) all "resolver" methods (such as subscribe, publish, unsubscribed) can be implemented as both synchronous and asynchronous functions. So I am closing this. Reopen if it is not.

[japrogramer]

do you mean RxPy now allows async resolves .. or graphene resolver can be async?
if it's the latter yes, i believe i tried that.

Im looking into https://github.com/strawberry-graphql/strawberry

[prokher]

@japrogramer Yes, I meant that "resolvers" of the Query, Mutation & Subscription classes can be implemented as async def methods. Actually, since v0.1.18 it is possible to implement all Subscription methods as async def.

Indeed, strawberry looks interesting. Taking into account the appearance of graphql-core-next and the progress of tartiflette (which seems to develop quite intensively these days), I will probably have to consider adding a replaceable "GraphQL-processing" layer to the current implementation. I am not sure, though.