-
Notifications
You must be signed in to change notification settings - Fork 312
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
Fields beind solved sequentially in the same VirtualThread #1051
Comments
Can you share a minimal sample so we can have a look? Have you tried recording traces with the observability instrumentation that we provide - a trace is usually helpful to visualize the execution of a request. Additionally, OSS support ended for Spring Boot 2.7.x so please use Spring Boot 3.2.x+ in your sample, if you can reproduce the issue. Thanks! |
This is also probably relevant to your use case: #958 |
Hi @bclozel, This happens even with Spring Boot 3.2.9. I'm checking on how to use observability instrumentation and the case #958 you mentioned. Thanks |
Hello @gpinzegher I forked your sample and created 3 branches demonstrating the behaviors. Synchronous execution, non-batchedhttps://github.com/bclozel/graphql/tree/base Data fetching is performed synchronously and we obviously have the N+1 problem here.
Same thing with
Asynchronous execution, non-batchedhttps://github.com/bclozel/graphql/tree/async Data fetching operations are performed in parallel.
Asynchronous execution, with batch loadinghttps://github.com/bclozel/graphql/tree/batch The trace is still showing multiple data fetching operations executed in parallel. We would like to make batch loading more obvious from a trace perspective. See #1034
If you enable automatic context propagation with
I believe all those behaviors are consistent with the setup. Please let us know what you think is missing. |
The Zipkink UI illustrates the behavior nicely. One thing to note is the BookController has to return before others are called because you need books before you know what authors or selling quantities to get. However, authors and selling can be done in parallel, and that's the case. I'm not sure what you are migrating from, or what the behavior was, but it can't really happen in any other way. |
Hi @bclozel, Thanks a lot for your time. I was checking your changes and I saw that async option is resolved one by one (that means several database connections). Is there any option for only Author and Selling controllers to be executed asynchronous (in parallel) at once? I will expand a bit my Book:
With the return of BookControler (List) I could trigger Author, Selling, Rating and Publisher controllers to be executed in parallel in a batch (for each one) using the list of Books as an input. Doing that in a batch I can bind the database 1 time per controller (in this case 4 times no matter how many books I want to retrieve) and doing it in parallel I would take 1 second to finish BookController and close to 1 second to finish the other 4 controller if executed in parallel. Is there any option to achieve this behavior? |
@gpinzegher isn't that what the batch branch does? |
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed. |
Hi @rstoyanchev, As far as I understand using @BatchMapping the following entities "D" would be resolved separately: Using the BatchLoaderRegistry I could load all the entities "C" and resolve "D" in one time with only 1 database connection. |
@gpinzegher why would they? Can you show any evidence of this behavior? |
Hi @bclozel, My concern about fields being resolved in parallel or sequentially still stands. In my project I need that fields are resolved in parallel when its possible. I created this new branch with an extra field to demonstrate the behavior: Fields author, rating and selling could be resolved at the same time as soon BookController has its results. From the the following branch (asynchronous with batch loading) the AuthorController is using SchemaMapping that means several calls to my database, and that's what I want to avoid. Books are resolved in a batch but authors is not. From the logs here you can see that's executed one after the other and not in parallel. Is asynchronous execution only available when using SchemaMapping? |
The sample is on 1.2.8, but parallel execution on VT's is a 1.3 feature, see #958. When I switch to Boot 3.3.4, it works as expected. Below 1.3.x, controller methods can return @BatchMapping(typeName = "Book", field = "qtySold")
public Callable<Map<Book, Integer>> qtySold(Set<Book> books) {
return () -> service.getSellings(books);
} |
Hello,
I am migrating my GraphQL project to use spring-graphql and I noticed that my Controllers are called sequentially instead of parallel.
All the code I will paste here is a similar code, but with the exact same coding structure:
schema.graphqls
BookController.java
BookService.java
AuthorController.java
AuthorService.java
SellingController.java
SellingService.java
On my understanding as soon as I have all Books solved GraphQL would trigger to retrieve both author and qtySold to be executed in the same time in different Threads as they rely only on Book's.
What I noticed is that both author and qtySold Controllers/Services are triggered sequentially on the same VirtualThread.
I also tried using an AsyncConfiguration (setting Core and Max PoolSize and also QueueCapacity) but I always get the same.
Just remember: this is a translated sample from the project I'm developing. The entire entities structure is way bigger and when I migrated to spring-grapql from graphql-java-kickstart one query that took from 3 to 5 seconds now is taking from 15 to 20+ seconds.
I am using
The text was updated successfully, but these errors were encountered: