Async execution of blocking controller methods on Java 21+

[jord1e]

Hello everyone,

I looked into the virtual thread support today.

It seems, that we have to wrap all @schemamapping annotated methods to return Callable:

  @SchemaMapping
  public boolean isLatest(ParentObject parent) {
    return true; // Long running I/O task
  }

Would have to become

  @SchemaMapping
  public Callable<Boolean> isLatest(ParentObject parent) {
    return () -> {    
        return true; // Long running I/O task
    };
  }

DGS automatically works with virtual threads on annotated controllers. Would it be an idea to implement this in Spring for GraphQL?
An opt-in configuration option would probably be required, and it would only work on JDK 21+.

https://netflix.github.io/dgs/advanced/virtual-threads/

It filters out CompletableFutures etc.:
https://github.com/Netflix/dgs-framework/blob/ee3d9eba9485cafadfc4cc9d2bbf0c6d4c8bdd89/graphql-dgs/src/main/java21/com.netflix.graphql.dgs.internal.VirtualThreadTaskExecutor/VirtualThreadTaskExecutor.java#L35
https://github.com/Netflix/dgs-framework/blob/master/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/CompletableFutureWrapper.kt#L51-L53

[rstoyanchev]

I think we can do this similar to the blocking execution support for WebFlux controller methods where you can configure an Executor and, optionally, a controller method Predicate<HandlerMethod> to control which methods are considered blocking. The default predicate matches all methods that don't return a type that's not a reactive type, nor future, nor a Kotlin suspending method.

[jord1e]

I think it's a solid proposal.

I think we can do this similar to the blocking execution support for WebFlux controller methods where you can configure an Executor

This would create two separate points to define an Executor:

1. Using the AnnotatedControllerConfigurer for methods returning Callable (how this is done properly should maybe be documented?)
2. For this new feature where we wrap controller methods with virtual threads using this new configurable Executor

The default predicate matches all methods that don't return a type that's not a reactive type, nor future, nor a Kotlin suspending method.

Reactive type would be everything mentioned in the Javadoc of ReactiveAdapterRegistry right?
I think this is reasonable. As long as it applies to methods annotated by SpringGraphQL-controller-annotations

Should Callable be in this list? I assume it is a special case within Spring for GraphQL

and, optionally, a controller method Predicate to control which methods are considered blocking.

Within GraphQL Java it is perfectly possible to have something like this:

@QueryMapping
public Object x() {
  if (...) {
    return CompletableFuture.supplyAsync(() -> "String Result", cpuBoundExecutor);
  } else {
    return "Invalid String result";
  }
}

Thus it would execute everything in a virtual thread (including the nested CompletableFuture), because the return type is Object.

In DGS they also use the return type of the method for determining this, which makes sense.
Maybe this edge-case should be documented/a warning message produced?

[rstoyanchev]

Maybe we are saying the same thing, but for this we would re-use the existing Executor configurable via AnnotatedControllerConfigurer. However, instead of Callable return types only, we would also use it for imperative controller methods, and that would be possible to control with an additional blockingMethodPredicate property. We'll apply this behavior automatically with Java 21+ when an Executor is configured.

Reactive type would be everything mentioned in the Javadoc of ReactiveAdapterRegistry right?

That's correct. As for Callable, by itself is not a reactive/async type.

it would execute everything in a virtual thread (including the nested CompletableFuture), because the return type is Object.

We could have special handling for Object to ensure the actual instance isn't a reactive/async type already.

[jord1e]

Maybe we are saying the same thing, but for this we would re-use the existing Executor configurable via AnnotatedControllerConfigurer. However, instead of Callable return types only, we would also use it for imperative controller methods, and that would be possible to control with an additional blockingMethodPredicate property. We'll apply this behavior automatically with Java 21+ when an Executor is configured.

👍
Do we really want this to be default behaviour?
Not all libraries are fully adapted yet (MySQL connector and HikariCP are ones I know of). The carrier thread of the virtual thread will stay pinned until a later JDK version: https://mail.openjdk.org/pipermail/loom-dev/2024-February/006433.html

I think DGS approaches this in a good way:
https://netflix.github.io/dgs/advanced/virtual-threads/#virtual-threads-jdk-21

They have a configuration option that creates a virtual thread group specifically for the data fetchers.
(defined here https://github.com/Netflix/dgs-framework/blob/ee3d9eba9485cafadfc4cc9d2bbf0c6d4c8bdd89/graphql-dgs/src/main/java21/com.netflix.graphql.dgs.internal.VirtualThreadTaskExecutor/VirtualThreadTaskExecutor.java#L43)
Although what you propose is a bit more general than virtual threads, users could also just refer to a ForkJoinPool executor (although it would make little sense).

And you need to manually enable Spring Framework support (spring.threads.virtual.enabled=true)

We could have special handling for Object to ensure the actual instance isn't a reactive/async type already.
This would require calling the method. If we want the method to execute in a virtual thread, this is thus not possible (as the type will be a non-"async" type)

Only after the execution/return of the method do we know if the return type is "async/CF/Callable/Kotlin Couritine" and if this edge-case applies (it might only return the non-async-type in a fast path or something).
If that makes sense?
I think we shouldn't worry about it for now.

[rstoyanchev]

Do we really want this to be default behaviour?

It's not on by default. You'd have to explicitly configure an Executor on AnnotatedControllerConfigurer, and the Boot starter does this when spring.threads.virtual.enabled=true. I don't imagine anyone would pass a non-elastic (e.g. forkjoin threadpool) for data fetching purposes. If this doesn't work well, you can get control through the blockingMethodPredicate.

Only after the execution/return of the method do we know if the return type is "async/CF/Callable/Kotlin Couritine" and if this edge-case applies (it might only return the non-async-type in a fast path or something).

Yes, this would have to be a request time check. This is more of a, what we could do if the need arises, but I don't think it needs to be there to start.

[rstoyanchev]

This is now in to try. The behavior is enabled if running on Java 21+ and the Executor is not a thread pool executor.