Virtual thread support for smallrye-graphql #47802

robp94 · 2025-05-12T11:35:25Z

Fixes #35605
~~Needs a branch from smallrye grapgql smallrye/smallrye-graphql#2287~~

Some questions:

Is there really no context handling needed? As stated here:io.quarkus.virtual.threads.ContextPreservingExecutorService
~~How to prevent the virtual thread executor from being removed?~~

Probably we will need more tests, but at least io.quarkus.virtual.graphql.RunOnVirtualThreadTest#testAnnotatedBlockingObject tests for virtual thread and works.

quarkus-bot · 2025-05-12T11:35:45Z

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

title should preferably start with an uppercase character (if it makes sense!)

_{This message is automatically generated by a bot.}

jmartisk · 2025-05-12T12:37:10Z

We'll need @cescoffier for this as well

cescoffier

It looks ok, but we need to check if there is any pinning or monopolization happening.

For pinning you can add a flag when running tests (https://quarkus.io/guides/virtual-threads#detect-pinned-thread-in-tests).

Unfortunately for monopolization, it's more tricky. We need to make sure that the operation is not CPU intensive.

phillip-kruger · 2025-05-13T00:05:47Z

Nice !. @jmartisk let's get the smallrye PR in and released, then we can get this builing in CI

robp94 · 2025-05-13T08:38:05Z

@cescoffier in the quarkus-virtual-threads-integration-tests-parent this is already configured. See here integration-tests/virtual-threads/pom.xml:107

I added some tests for pinning (Query and Mutation) seems there is no pinning, should we test more variants for pinning?

Do we need to support combinations of @RunOnVirtualThread and @NonBlocking or uni/completionstage?

cescoffier · 2025-05-15T08:35:33Z

Do we need to support combinations of @RunOnVirtualThread and @nonblocking or uni/completionstage?

No, virtual threads are only for blocking (it could work, but would be quite convoluted). In general, we declare these configurations invalid.

I added some tests for pinning (Query and Mutation) seems there is no pinning, should we test more variants for pinning?

Unfortunately, there is no magic wand here. But it should not block merging this.

...untime/src/main/java/io/quarkus/smallrye/graphql/runtime/spi/datafetcher/BlockingHelper.java

jmartisk · 2025-05-19T11:25:41Z

You will need to add

 <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-smallrye-graphql-deployment</artifactId>
            <version>${project.version}</version>
            <type>pom</type>
            <scope>test</scope>
            <exclusions>
                <exclusion>
                    <groupId>*</groupId>
                    <artifactId>*</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

to integration-tests/virtual-threads/graphql-virtual-threads/pom.xml

I'm planning to release SmallRye GraphQL 2.14 with this soon so that the Quarkus side can move forward..

jmartisk · 2025-05-19T11:27:34Z

Btw I am also getting 10 failures in io.quarkus.virtual.graphql.GraphQLThreadTest with java.net.ConnectException: Connection refused, is this supposed to be working already?

robp94 · 2025-05-20T07:07:36Z

Btw I am also getting 10 failures in io.quarkus.virtual.graphql.GraphQLThreadTest with java.net.ConnectException: Connection refused, is this supposed to be working already?

It should have, but I only ran the tests via intellijs test runner, where they worked. Running via maven, I get the same errors.

I missed one annotation, now it should work.

jmartisk · 2025-05-21T06:55:00Z

...graphql-virtual-threads/src/test/java/io/quarkus/virtual/graphql/RunOnVirtualThreadTest.java

+@ShouldNotPin
+class RunOnVirtualThreadTest extends AbstractGraphQLTest {
+
+    //todo how to make sure vt executor is not removed?


Maybe @cescoffier knows - do we have any general mechanism to detect usage of virtual threads?

At minimum though, in case we are to keep this injection, I would move it into the RunOnVirtualThreadObjectTestThreadResource subclass because then it won't prevent running this test in native mode

I moved it to the subclass.

robp94 · 2025-06-03T08:14:07Z

I'm planning to release SmallRye GraphQL 2.14 with this soon so that the Quarkus side can move forward.

Any news on smallrye graphql release? @jmartisk

jmartisk · 2025-06-03T08:18:02Z

Ah right, thanks for reminding me, I'll do it today

jmartisk · 2025-06-03T10:05:11Z

@robp94 2.14.0 should be released now, can you upgrade it in the pom here and unmark as draft?

robp94 · 2025-06-03T12:44:48Z

@jmartisk I upgraded it and marked the pr as ready, so there are still some open questions, like how to make sure the vt executor is not removed.

github-actions · 2025-06-03T13:35:17Z

🙈 The PR is closed and the preview is expired.

cescoffier · 2025-06-16T05:57:45Z

Good question. @ozangunalp do we need both, or just the executor?

robp94 · 2025-06-16T06:18:33Z

That did not work, I now use

unremovableBeans.produce(UnremovableBeanBuildItem
                    .beanTypes(DotName.createSimple("java.util.concurrent.ExecutorService"),
                            DotName.createSimple("java.util.concurrent.Executor"),
                            DotName.createSimple("io.quarkus.virtual.threads.VirtualThreads")));

But I am not that familiar with the build stuff of quarkus, so not sure if this is correct or we can do somthing else.

ozangunalp · 2025-06-18T09:19:30Z

There shouldn't be any need to check for VT executor. In the runBlocking you can get the executor using VirtualThreadsRecorder.getCurrent(). I'd remove also the isVirtualThreadsExecutorAvailable method.

Again, no need to add unremovable bean.

robp94 · 2025-06-18T11:34:21Z

Well that makes things simpler, I changed it as supposed.

ozangunalp · 2025-06-18T13:55:58Z

@robp94 The change looks good, can you squash your commits?

quarkus-bot · 2025-06-19T07:52:59Z

Status for workflow `Quarkus Documentation CI`

This is the status report for running Quarkus Documentation CI on commit 3280ec4.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

quarkus-bot · 2025-06-19T10:56:07Z

Status for workflow `Quarkus CI`

This is the status report for running Quarkus CI on commit 3280ec4.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Integration Tests - JDK 17

📦 integration-tests/opentelemetry

✖ io.quarkus.it.opentelemetry.LoggingResourceTest.testException - History

Condition with Lambda expression in io.quarkus.it.opentelemetry.LoggingResourceTest was not fulfilled within 2 minutes. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.LoggingResourceTest was not fulfilled within 2 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1160)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1129)
	at io.quarkus.it.opentelemetry.LoggingResourceTest.testException(LoggingResourceTest.java:113)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

⚙️ JVM Integration Tests - JDK 21

📦 integration-tests/opentelemetry

✖ io.quarkus.it.opentelemetry.LoggingResourceTest.testException - History

Condition with Lambda expression in io.quarkus.it.opentelemetry.LoggingResourceTest was not fulfilled within 2 minutes. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.LoggingResourceTest was not fulfilled within 2 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1160)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1129)
	at io.quarkus.it.opentelemetry.LoggingResourceTest.testException(LoggingResourceTest.java:113)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

jmartisk · 2025-06-19T11:40:38Z

Thanks @robp94 !!! Nice work

kyungseogoh · 2025-08-07T15:58:08Z

is there plan to backport this to 3.20 version of quarkus.

jmartisk · 2025-08-07T16:39:04Z

is there plan to backport this to 3.20 version of quarkus.

As a feature that is both new and experimental, this is not planned to be backported. With LTS branches, we tend to be rather conservative.

quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/graphql area/smallrye labels May 12, 2025

robp94 mentioned this pull request May 12, 2025

add run on virtual threads to Execute smallrye/smallrye-graphql#2287

Merged

robp94 marked this pull request as draft May 12, 2025 11:38

robp94 changed the title ~~vt for graphql~~ Virtual thread support for smallrye-graphql May 12, 2025

cescoffier reviewed May 12, 2025

View reviewed changes

geoand added the release/noteworthy-feature label May 14, 2025

jmartisk reviewed May 19, 2025

View reviewed changes

...untime/src/main/java/io/quarkus/smallrye/graphql/runtime/spi/datafetcher/BlockingHelper.java Outdated Show resolved Hide resolved

quarkus-bot bot added the area/documentation label May 20, 2025

jmartisk reviewed May 21, 2025

View reviewed changes

robp94 marked this pull request as ready for review June 3, 2025 12:43

robp94 force-pushed the feature/graphql-vt branch from 0657061 to 5cc15e3 Compare June 3, 2025 12:43

This comment has been minimized.

Sign in to view

robp94 force-pushed the feature/graphql-vt branch 2 times, most recently from 62a6de5 to a719272 Compare June 15, 2025 18:05

This comment has been minimized.

Sign in to view

quarkus-bot bot added the triage/flaky-test label Jun 15, 2025

robp94 force-pushed the feature/graphql-vt branch 2 times, most recently from 7544357 to f23b2e0 Compare June 18, 2025 11:24

This comment has been minimized.

Sign in to view

robp94 force-pushed the feature/graphql-vt branch from f23b2e0 to 67d73d8 Compare June 18, 2025 13:57

This comment has been minimized.

Sign in to view

add virtual thread support for graphql

3280ec4

robp94 force-pushed the feature/graphql-vt branch from 67d73d8 to 3280ec4 Compare June 19, 2025 07:15

jmartisk approved these changes Jun 19, 2025

View reviewed changes

ozangunalp approved these changes Jun 19, 2025

View reviewed changes

ozangunalp added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 19, 2025

jmartisk merged commit 4435c0d into quarkusio:main Jun 19, 2025
60 checks passed

quarkus-bot bot added this to the 3.25 - main milestone Jun 19, 2025

quarkus-bot bot added kind/enhancement New feature or request and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Jun 19, 2025

Virtual thread support for smallrye-graphql #47802

Virtual thread support for smallrye-graphql #47802

Uh oh!

Conversation

robp94 commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quarkus-bot bot commented May 12, 2025

Uh oh!

jmartisk commented May 12, 2025

Uh oh!

cescoffier left a comment

Choose a reason for hiding this comment

Uh oh!

phillip-kruger commented May 13, 2025

Uh oh!

robp94 commented May 13, 2025

Uh oh!

cescoffier commented May 15, 2025

Uh oh!

Uh oh!

jmartisk commented May 19, 2025

Uh oh!

jmartisk commented May 19, 2025

Uh oh!

robp94 commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmartisk May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robp94 Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

robp94 commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmartisk commented Jun 3, 2025

Uh oh!

jmartisk commented Jun 3, 2025

Uh oh!

robp94 commented Jun 3, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

cescoffier commented Jun 16, 2025

Uh oh!

robp94 commented Jun 16, 2025

Uh oh!

ozangunalp commented Jun 18, 2025

Uh oh!

robp94 commented Jun 18, 2025

Uh oh!

This comment has been minimized.

ozangunalp commented Jun 18, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

quarkus-bot bot commented Jun 19, 2025

Status for workflow Quarkus Documentation CI

Uh oh!

quarkus-bot bot commented Jun 19, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status for workflow Quarkus CI

Flaky tests - Develocity

⚙️ JVM Integration Tests - JDK 17

📦 integration-tests/opentelemetry

⚙️ JVM Integration Tests - JDK 21

📦 integration-tests/opentelemetry

Uh oh!

Uh oh!

jmartisk commented Jun 19, 2025

robp94 commented May 12, 2025 •

edited

Loading

robp94 commented May 20, 2025 •

edited

Loading

jmartisk May 21, 2025 •

edited

Loading

robp94 commented Jun 3, 2025 •

edited

Loading

github-actions bot commented Jun 3, 2025 •

edited

Loading

Status for workflow `Quarkus Documentation CI`

quarkus-bot bot commented Jun 19, 2025 •

edited by github-actions bot

Loading

Status for workflow `Quarkus CI`