@SchemaMapping may cause the order to be disrupted. How to solve it #949
Comments
spring-projects-issues
added
the
status: waiting-for-triage
|
I don't think this is strictly related to Spring for GraphQL. I think that what happens here is that the "favorite" property is fetched asynchronously and this means that videos are completely resolved by the GraphQL engine when all properties are resolved, in this case "out of order". I'm not sure if anything can be done at the Spring level to work around that. |
|
There might be something less obvious going on. I would be surprised if async execution impacts the order. I imagine the AsyncExecutionStrategy prepares a list of futures, and fills out the results, keeping the same order, but I could be wrong. A couple of suggestions for further investigation.
@Bean
public RuntimeWiringConfigurer runtimeWiringConfigurer() {
return wiringBuilder -> {
wiringBuilder.type("Query", builder -> builder.dataFetcher("searchVideo", env -> ""));
wiringBuilder.type("Video", builder -> builder.dataFetcher("isFavorite", env -> ""));
// ...
};
}If you provide an isolated sample, we can also have a look. |
rstoyanchev
added
the
status: waiting-for-feedback
|
https://github.com/madwind/async-graphql-test I use GraphiQL to test subscription MySubscription {
searchVideo {
id
name
isFavorite
}
}@Controller
public class GraphqlController {
long interval = 100;
long multiple = 8;
long firstId = 5;
@SubscriptionMapping
public Flux<Video> searchVideo() {
return Flux.interval(Duration.ofMillis(interval)).map(s -> new Video(s.intValue() + 1))
.take(9)
.doOnNext(video -> System.out.println("searchVideo: " + video.id));
}
@SchemaMapping
public String name(Video video) {
return String.valueOf((char) (video.id + 64));
}
@SchemaMapping
public Mono<Boolean> isFavorite(Video video) {
long delay = video.id == firstId ? 0 : (interval * multiple) - (interval * multiple / 10) * video.id;
return Mono.delay(Duration.ofMillis(delay))
.then(Mono.just(Boolean.TRUE))
.doOnNext(s -> System.out.println("SchemaMapping: " + video.id + " delay: " + delay));
}
}when 'multiple' increases to 8，sometimes will get searchVideo: 1
searchVideo: 2
searchVideo: 3
searchVideo: 4
searchVideo: 5
SchemaMapping: 5 delay: 0
searchVideo: 6
searchVideo: 7
searchVideo: 8
SchemaMapping: 1 delay: 720
SchemaMapping: 3 delay: 560
SchemaMapping: 2 delay: 640
searchVideo: 9
SchemaMapping: 4 delay: 480
SchemaMapping: 6 delay: 320
SchemaMapping: 7 delay: 240
SchemaMapping: 8 delay: 160
SchemaMapping: 9 delay: 80websocket message {"type":"connection_init","payload":{}} | 39 | 07:11:57.383
{"id":null,"type":"connection_ack","payload":{}} | 48 | 07:11:57.383
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"subscribe","payload":{"query":"subscription MySubscription {\n searchVideo {\n id\n name\n isFavorite\n }\n}","operationName":"MySubscription"}} | 208 | 07:11:57.383
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"next","payload":{"data":{"searchVideo":{"id":"5","name":"E","isFavorite":true}}}} | 134 | 07:11:57.892
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"next","payload":{"data":{"searchVideo":{"id":"1","name":"A","isFavorite":true}}}} | 134 | 07:11:58.223
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"next","payload":{"data":{"searchVideo":{"id":"3","name":"C","isFavorite":true}}}} | 134 | 07:11:58.255
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"next","payload":{"data":{"searchVideo":{"id":"2","name":"B","isFavorite":true}}}} | 134 | 07:11:58.257
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"next","payload":{"data":{"searchVideo":{"id":"4","name":"D","isFavorite":true}}}} | 134 | 07:11:58.287
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"next","payload":{"data":{"searchVideo":{"id":"6","name":"F","isFavorite":true}}}} | 134 | 07:11:58.333
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"next","payload":{"data":{"searchVideo":{"id":"7","name":"G","isFavorite":true}}}} | 134 | 07:11:58.348
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"next","payload":{"data":{"searchVideo":{"id":"8","name":"H","isFavorite":true}}}} | 134 | 07:11:58.365
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"next","payload":{"data":{"searchVideo":{"id":"9","name":"I","isFavorite":true}}}} | 134 | 07:11:58.380
{"id":"d6425dce-592e-4e33-ac52-603b1b534e0b","type":"complete","payload":{}} | 76 | 07:11:58.383when 'multiple' increases to more than 8, It is missing some data. searchVideo: 1
searchVideo: 2
searchVideo: 3
searchVideo: 4
searchVideo: 5
SchemaMapping: 5 delay: 0
searchVideo: 6
searchVideo: 7
searchVideo: 8
searchVideo: 9
SchemaMapping: 3 delay: 700
SchemaMapping: 4 delay: 600
SchemaMapping: 9 delay: 100
SchemaMapping: 8 delay: 200
SchemaMapping: 1 delay: 900
SchemaMapping: 7 delay: 300
SchemaMapping: 2 delay: 800
SchemaMapping: 6 delay: 400websocket message {"type":"connection_init","payload":{}} | 39 | 07:15:59.685
{"id":null,"type":"connection_ack","payload":{}} | 48 | 07:15:59.703
{"id":"342dc0a6-04e7-4171-874f-5a74399d301b","type":"subscribe","payload":{"query":"subscription MySubscription {\n searchVideo {\n id\n name\n isFavorite\n }\n}","operationName":"MySubscription"}} | 208 | 07:15:59.703
{"id":"342dc0a6-04e7-4171-874f-5a74399d301b","type":"next","payload":{"data":{"searchVideo":{"id":"5","name":"E","isFavorite":true}}}} | 134 | 07:16:00.247
{"id":"342dc0a6-04e7-4171-874f-5a74399d301b","type":"next","payload":{"data":{"searchVideo":{"id":"8","name":"H","isFavorite":true}}}} | 134 | 07:16:00.756
{"id":"342dc0a6-04e7-4171-874f-5a74399d301b","type":"next","payload":{"data":{"searchVideo":{"id":"7","name":"G","isFavorite":true}}}} | 134 | 07:16:00.759
{"id":"342dc0a6-04e7-4171-874f-5a74399d301b","type":"next","payload":{"data":{"searchVideo":{"id":"6","name":"F","isFavorite":true}}}} | 134 | 07:16:00.771
{"id":"342dc0a6-04e7-4171-874f-5a74399d301b","type":"next","payload":{"data":{"searchVideo":{"id":"2","name":"B","isFavorite":true}}}} | 134 | 07:16:00.774
{"id":"342dc0a6-04e7-4171-874f-5a74399d301b","type":"complete","payload":{}} | 76 | 07:16:00.776 |
spring-projects-issues
added
status: feedback-provided
|
I am reproducing the same behavior when commenting out the package com.example.asyncgraphqltest;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.graphql.execution.RuntimeWiringConfigurer;
@Configuration
public class GraphQlConfiguration {
@Bean
public RuntimeWiringConfigurer runtimeWiringConfigurer() {
GraphqlController controller = new GraphqlController();
return wiringBuilder -> {
wiringBuilder.type("Subscription", builder -> builder.dataFetcher("searchVideo", env -> controller.searchVideo()));
wiringBuilder.type("Video", builder -> builder.dataFetcher("isFavorite", env -> controller.isFavorite(env.getSource()).toFuture()));
wiringBuilder.type("Video", builder -> builder.dataFetcher("name", env -> controller.name(env.getSource())));
};
}
}Using the following query: subscription MySubscription {
searchVideo {
id
name
isFavorite
}
} |
|
Nominally graphql-java keeps fields in order. Its async but its waits for all futures the complete and then keeps them in a LinkedHashMap in query field order query q {
searchVideo {
a
b
c
}
}will produce a result like {
data :{
searchVideo : [ {
a : "..."
b : "..."
c : "..."
}
}
}However this is a The original issue was reported here at graphql-java/graphql-java#3563 and it never mentioned subscriptions and we missed the I don't know off hand what is happening here but Subscriptions do run differently. The first field is expected to produce a See Its ends up calling back to This should then use the normal "keep everything in order" as per the sub selection but we seem to have evidence it does not in this case. I wonder why. This needs more debugging I think but at first glance it should keep results in sub selection order. |
|
I created graphql-java/graphql-java#3574 to try to show how async fields (which can complete at different times- can still keep the field order. I dont know whats going on here but I am not sure how Srring will compose I would assume the first is a CF ( |
|
I ran the test graphql-java/graphql-java#3574. The order is correct when requests come in one after another. However, it becomes incorrect when multiple requests are made at once. |
Can you outline how you ran multiple at a time? We are really trying to get into a better reproduction state. What setup did you have when you say multiple at a time ? Ahh you did it on the other PR... thanks |
|
I think I know what is happening here - I will answer here rather than the graphql-java PR because the audience that knows Reactor / Reactive better is here The graphql-java subscriptions code uses a
This is the code So for ever video object published down the flux is makes a a callback to a mapper on that object In this case the The downstream private BiConsumer<D, Throwable> whenNextFinished(CompletionStage<D> completionStage) {
return (d, throwable) -> {
try {
if (throwable != null) {
handleThrowable(throwable);
} else {
downstreamSubscriber.onNext(d); // the CF has completed - run onNext() on the downstream
}
} finally {
Runnable runOnCompleteOrErrorRun = onCompleteOrErrorRun.get();
boolean empty = removeFromInFlightQAndCheckIfEmpty(completionStage);
if (empty && runOnCompleteOrErrorRun != null) {
onCompleteOrErrorRun.set(null);
runOnCompleteOrErrorRun.run();
}
}
};
}Now the thing I dont know is what the Reactor / Reactive contract should be here. We have a series of values eg I suspect we probably want to keep them in original emission order in a graphql sense. We do when we have a BUT reactive is different - we dont know when the next If we buffer in graphql-java, we need to decide on the size of the buffer and it probably should come from user code, not the library. |
|
Is it possible to provide a method to make the onNext invocation synchronous in user code, similar to Reactor's concatMap? @Override
public void onNext(U u) {
// for safety - no more data after we have called done/error - we should not get this BUT belts and braces
if (onCompleteOrErrorRunCalled.get()) {
return;
}
try {
// CompletionStage<D> completionStage = mapper.apply(u);
// offerToInFlightQ(completionStage);
// completionStage.whenComplete(whenNextFinished(completionStage));
downstreamSubscriber.onNext(mapper.apply(u).toCompletableFuture().join());
} catch (RuntimeException throwable) {
handleThrowable(throwable);
}
} |
|
@bbakerman thanks for your analysis. In my opinion, the current behavior makes sense after all. Queries/mutations return bounded collections in the response. The API can provide a way for clients to sort entries and paginate for large collections. In the case of subscriptions, the intent is more about getting state updates on large entities, or unbounded stream of events (pub/sub style). With these use cases, I think the GraphQL engine should indeed push events as soon as they are fully available. In my opinion, applications expecting any other sorting/ordering should instead use queries with pagination. As you have pointed out, implementing this behavior in graphql-java would require limited buffering (which would still show that behavior when the buffering limit is reached); this would also artificially delay available data and then send large bursts of events. Introducing such behavior would probably "break" existing applications. Let's hear from @rstoyanchev first, but my vote goes to closing this issue here and not considering it on the graphql-java side. |
we can't do this - this would block the upstream |
|
Indeed, a subscriber must not block the Pagination and streaming are not always interchangeable, and preserving the order of publication may be important. However, as it has a cost associated, it should be possible to opt in/out |
|
https://github.com/graphql-java/graphql-java/pull/3574/files is a PR that improves the current mechanism (cancelling inflight futures so that underlying The default mode is still "emit events as the graphql subselection completes not as they originally arrived" but there is a new mode that can be hinted at that causes the subscription code to preserve the original ordering of the source Publisher |
I have a list sorted by vodtime. When I use @schemamapping, I find that if the return type is Mono, it causes the order to be disrupted.
ts:
java log:
ts log
If I remove 'isFavorite' in GraphQL, its order will become correct.
java log:
ts log
The text was updated successfully, but these errors were encountered: