Virtual Thread Support in GraphQL

[pipinet]

Description

I saw help documents about gRPC and reactive messaging on the official website, but there is no help document about graphql?

Implementation ideas

No response

[quarkus-bot[bot]]

/cc @jmartisk (graphql), @phillip-kruger (graphql)

[jmartisk]

We haven't really got around to it yet I guess. If you want, try it (with @RunOnVirtualThread) and let us know your findings...

[geoand]

cc @cescoffier

[cescoffier]

The client can be used. The server will require a lot of work, as graphql-java is not virtual thread-friendly.

I can not give you an ETA.

[bbakerman]

as graphql-java is not virtual thread-friendly.

We would love some direct feedback on what it would take to make graphql-java virtual thread-friendly.

Its code base is heavily based on ConmpleteableFuture (and its composition mechanism like .thenCompose() etc..)

I naively think that

• if the outside call to graphql.execute() (eg request handling) is a VT then graphql-java should be able to execute as it is today via CFs

• if a DataFetcher call causes a blocking operation then the VT will yield to the carrier thread and that graphql request will wait and the carrier thread will do other work

• if a DataFetcher decides it want to run its work in a VT then it can do this

Future f = vtExecutor.submit( () -> valueMethodCall())
return f.join();

and I expect that if this blocks the same as above if we are running inside a VT

However we haven't spent much time looking at VT and graphql-java and I might be way of course here.

We would love to know more about being virtual thread-friendly.

[bbakerman]

note - graphql-java today (and likely in the future) nevers start a thread - it is up to the outside calling code or the DataFetchers to decide to make a call that crosses thread boundaries.

I suspect that would still be true in a VT world if we had more support for it

[cescoffier]

@bbakerman My initial analysis (and first tests) shows potential pinning. There is a bunch of synchronized, which can be problematic if an I/O happens when holding the lock.

Using CompletionStage is ok (as waiting on it is not blocking the carrier thread). Still, I've seen a few usages of *Async() method, which then runs on an F/J pool (or a given executor), so there will be a jump of threads (if I remember correctly, it was in the DataFetcher).

Then, Quarkus / SmallRye has a lot of customizations, which would need to be checked and probably reworked.

[bbakerman]

Can I asked (in a coaching sense) if we use RentrantLocks instead of synchronised does that stop pinning

(as I said we havent done a lot on Loom yet - sees like we should)

I created a PR to remove most of the synchronised calls in graphql-java and replace them with Renetrant locks as a POC

https://github.com/graphql-java/graphql-java/pull/3317

Still, I've seen a few usages of *Async() method, which then runs on an F/J pool (or a given executor), so there will be a jump of threads (if I remember correctly, it was in the DataFetcher).

This won't be in graphql-java code itself because we never start threads or submit onto executors. That said it could be in the Quarkus graphql code say.

[cescoffier]

@bbakerman Yes, some of the work need to happen in Quarkus and SmallRye GraphQL.

About the locks, yes, they will remove the pinning, see: https://github.com/hibernate/hibernate-orm/pull/7085 as an example.

[pipinet]

Are there any plans for Virtual Thread Support in GraphQL？

[t1]

I'm also currently exploring virtual threads and I have encountered some issues with the typesafe client when it's running in the application while parallel requests come in. It seems to run fine in the tests themselves, also when they are executed in parallel.

I've found a syncronized block in io.smallrye.graphql.client.vertx.VertxManager#getOrCreateCustom; but my first experiment with replacing it with a ReenterantLock didn't seem to help.

[sgift]

I'd also be interested in this. Has there been an update in the support or are there plans to support this in the (near) future?

Thanks.

[cescoffier]

No, unfortunately, no plan right now. But contributions are more than welcome (however, I believe graphql-java may need some work too).

[bbakerman]

I am hopeful there will not be many (if any??) changes in graphql-java required. We moved away from syncronized blocks to avoid virtual thread pinning and graphql-java itself never starts a thread and always leaves that to the "outer" framework to decide on how graphql fields are "dispatched" in thread terms.

We have done some basic VT testing but of course it always matters most on how the outer code ends up calling that determines if things need to be fixed. Please let us know if we need to tweak things more.

[cescoffier]

Thanks, @bbakerman, I'm more concerned by the "downstream" calls (graphql -> your backend) because if your backend is pinning, monopolizing, using thread locals, and classloading followed with I/O.... then you will end up in trouble (it's not the responsibility of GraphQL java).

[bbakerman]

One thing of note in v21 - we used to wrap everything in a CompletableFuture as a way to compose together async and sync values elegantly in code. However this means that simple POJO values get wrapped in a CF thats completed. So elegant code, less efficient.

In V21 we made the code more ugly and a lot more efficient in memory terms. Truly completed values are not wrapped.

I bring this up because in a Virtual Threads world, the values coming back are synchronous at the time graphql-java gets them - sure they may have been asynchronously fetched via a VT but they present themselves as a materialised value when they get back to graphql-java. This should mean that in a VT world, this ugly code will work just as efficiently.

[kito99]

+1