Optimizing incremental delivery

[yaacovCR]

UPDATE this has been moved to two separate discussion topics:

1. [graphql-js] Yielding batches of results in resolver functions #40
2. Yielding batches of payloads from defer/stream GraphQL requests #41

Please continue discussion there.

---

From: #32 (comment)

Suggestion

(A) Field resolvers that return an async iterable should return an async iterable of iterables, ie an async iterable that returns chunks of the larger list.
(B) Similarly, the AsyncGenerator returned by GraphQL should return an array of available payloads rather than a single item.

Motivation

If our underlying services have been optimized to produce values in chunks, we should yield values in chunks when possible.

Reference from outside GraphQL: https://medium.com/netscape/async-iterators-these-promises-are-killing-my-performance-4767df03d85b

Timing

The main motivation for introducing this change now is to avoid a breaking change by introducing it later.

Spec change?

A spec change does not seem necessary, but may be advisable. As far as I can tell, the exact implementation around asynchronous execution is not specified, and so these changes, so adding batching to asynchronous handling does not seem to conflict with the spec.

It may be advisable for the spec to directly indicate that the use of batching may be a performance optimization.

Implementation

See yaacovCR/graphql-js#154 for a worked example

Suggested type signature for execute:

export function execute(
  args: ExecutionArgs,
): PromiseOrValue<
  | ExecutionResult
  | AsyncGenerator<ReadonlyArray<AsyncExecutionResult>, void, void>
> {
  ...
}

Suggested type signature for subscribe

Same as above.

Note that graphql-executor does not have a separate subscribe function, as subscription operations have been completely integrated into the execution code/algorithm, so the above signature already is the de facto signature within graphql-executor for the corresponding graphql-js subscribe function.

Note also that a choice was made to simplify the signature for subscriptions, as the signature could have theoretically been:

export function subscribe(
  args: ExecutionArgs,
): PromiseOrValue<
  | ExecutionResult
  | AsyncGenerator<ExecutionResult | ReadonlyArray<AsyncExecutionResult>, void, void>
> {
  ...
}

If a subscription operation does not use defer or stream, it should yield only single ExecutionResults rather than arrays. Even if it does include defer/stream, depending on initialCount values and/or other factors, the same operation may sometimes result in a single value or an array. For simplicity, the implementation wraps single values in arrays. The extra wrapping (and later unwrapping) (1) allows subscribe to have the same, simpler signature as query and mutation operations and (2) allows the caller to not have to check to see whether the generator yielded an array or not, as an array is always yielded.

Example queries/responses

Defer with fragments
Defer with slow field in initial payload
Stream
Stream with chunks of greater than 1
Stream in correct order even when the first item is slow
Stream in parallel
Stream with asyncIterableList
Stream with asyncIterableList with chunk size > 1
Stream where a non-nullable item returns null

[yaacovCR]

@robrichard -- i brought the above up at the most recent graphql-js wg, but it wasn't felt to be the best forum. @IvanGoncharov suggested perhaps breaking out a separate implementer group for defer-stream.

I am honestly not scared by releasing as-is and then introducing this later as a breaking change, as breaking changes in my opinion are not The Worst Thing Ever.

But, if we could get consensus around this now, I suppose it can't hurt to just go for it.

[yaacovCR]

Notes after April WG:

1. Thanks everyone for feedback at the meeting, sorry for the information dump at the end of a long meeting.
2. The examples listed above are potential future examples that include parallel (out-of-order) and chunked (batched) streaming.
3. The initial release of incremental delivery will not include these features -- the examples are only used to provide context of how batching payloads is still useful even if we later batch items within stream payloads.

[yaacovCR]

Additional thoughts on different meanings/motivations batching with incremental delivery. This discussion is only about the first type of batching described below, but it it helpful to try to spell out as much as possible the other types of batching just to disambiguate the discussion.

Batching available payloads into a single payload

A first type of batching is about sending multiple payloads to the client at once. If multiple deferred payloads are ready, or if multiple stream payloads are ready -- from the same or even from different streams -- we might as well send everything to the client as we have it, as a list that wraps all of the available payloads. This discussion is about this type of batching, and the format we would use for it, reflected in the return type for execute. This is the exact same performance issue reflected in the return type of the schema's list resolvers when using async iterables, and is the reason why the two discussions are linked. Below are spec changes suggested to specify this behavior:

yaacovCR/graphql-spec#1: Send all available payloads
yaacovCR/graphql-spec#2: Underlying resolvers returning AsyncIterables should return AsyncIterables of Iterables

Besides for performance on the server, it also may potentially optimize battery life on the client in terms of raw number of network requests. It may also optimize performance of the client application just like on the server.

Delaying stream results until a "batch" is ready

A totally separate meaning of batching is whether results should not be sent to the client at all unless a certain threshold number of results is reached. I prefer to refer to this as "chunking" or "bundling" rather than batching to avoid confusion. This behavior may also potentially save battery life and increase performance, but comes at the cost that the client must wait until a certain chunk threshold has been reached. In the implementation at graphql-executor we provide maxChunkSize and maxInterval arguments that send a chunk when either threshold has been triggered, as per suggestion of @nodkz (#23 (comment)).

This type of batching or chunking, because it by definition applies to a single stream, could then include all of those stream results within the data field of a single stream payload -- which is what my examples do above.

I think it's important to emphasize, that it could also be appropriate to utilize this form of batching, but instead of including all of the stream results within the data field of a single stream payload, the executor could send separate stream payloads that are then batched as above. This chunking/batching behavior would still be useful, because in many cases, the client does not need each additional stream result as it comes, and it could be highly performant to delay/group these updates until the thresholds are reached.

Batching stream results without constraints

Alternatively, a third method of batching might be a combination of aspects of the above. Perhaps one could suggest that even if the first type of batching is used to consolidate deferred payloads and multiple streams, when completed results from the same stream are ready, they should be bundled within a single stream payload. That way, for each set of payloads the client receives, each stream's results would be consolidated. After all, this is information the server has already, and the client will have to iterate through the payloads to group them, and so by encoding this within the response, some small performance benefit may be accrued. The goals of this type of batching are dissimilar to both of the options above, although implementation wise, this might be a special case of the second option above, equivalent to a maxChunkSize=Infinity and a maxInterval=0.

[yaacovCR]

What do you think the payloads yielded by execute's AsyncGenerator should consist of?

A Single AsyncExecutionResult

All Available AsyncExecutionResults

Note: the format of the second option is up for debate, it could be an array of AsyncExecutionResults or some other type.

[yaacovCR]

Wow, great response number here! Please vote on the additional poll in the comment below. Please feel free to share feedback as to your choice.

[robrichard]

I voted for "All Available AsyncExecutionResults".

Concerns about large number of async payloads causing many client rerenders have been raised about defer/stream. By yielding lists of payloads, clients can do one render pass for each list of payloads that are received. If we yield single payloads, clients would either render on receipt of each payload or have to insert a debouncing delay to see if additional payloads are being sent in quick succession.

If we launch this new api as an AsyncGenerator that yields a single payload, we may need another breaking change in the future to support returning batches of payloads.

[yaacovCR]

Note: I restarted this poll, as I think the question wording was off.

For resolvers that return async iterables, what should the async iterable yield?

Next Available Value

Next Set of Available Values

Other

For Other, please add your comments below!

[yaacovCR]

note -- i restarted the above poll, as I took another look, and saw all the others, and realized that the question wording was off.

[yaacovCR]

please take another look and see if it makes more sense

[robrichard]

I think there will be many cases where underlying api yields single values and many other cases where the underlying api yields lists of values. One example of a database api that yields lists of values is the DynamoDB paginateQuery API.

Option 1: If we choose to yield single values, servers will need to convert apis the yield lists to yield each individual value.

Option 2: If we choose to yield lists, servers will need to convert apis that yield individual values to wrap each value in a list before yielding.

With option 1, we remove the ability for the GraphQL execution to synchronously process more than one value at a time. While the resolver has access to multiple values, each value will have the overhead of an event loop tick added before the GraphQL execution can start processing it.

Option 2 doesn't introduce any overhead, but requires more code for what might be the simpler or more common use-case.

I think the tradeoffs for Options 2 are worth it. Some number of users will have to adapt their resolver code to fit whatever pattern we decide on, so it makes sense to me to favor the one that is more efficient and flexible.

[yaacovCR]

Hello all — thanks again for participating in the above polls. Please share your opinions on the issues so we can all get a sense of the relevant issues. It would be great if we could do some work on this in advance of the next meeting and present thoughts. If anyone is interested in a breakout discussion of the above aspects of incremental delivery, I would love to set something up prior to next wg so that the options could be best presented and resolved at that meeting.

[LegNeato]

Perhaps I missed it in all the discussions, but is there a method for including backpressure? What if the server produces too quickly for the client?

[yaacovCR]

Great question! Easiest thing is to use async iterators that respond to back pressure a la repeaters https://repeater.js.org/

[yaacovCR]

Hi @LegNeato, actually my comment above is both misleading and incorrect. I am afraid that although repeaters can provide a simple-to-use backpressure mechanism, this does not address what you were asking.

Taking a step back, with regard to subscriptions, repeaters can be a very helpful tool to easily communicate backpressure. Repeaters can do this very simply by allowing the async iterator to be lazy and to not produce the next value until the last is consumed. Additional more complex strategies are also possible. Subscriptions involve long-running event streams, and a fast producer situation over time can certainly lead to problems. Repeaters are indeed helpful in such a situation -- but that's not what you asked about.

In the incremental delivery context, we will have resolvers that will be newly able to return async iterators (instead of just iterables), with the completed items possibly later themselves streamed to the client (as determined by whether the operation includes the stream directive). If stream is indeed used, the executor will return an async generator which will then be consumed by a graphql server application, with each payload streamed to the client. If any "consumer" step in this chain does not consume the payloads as fast as they are produced, they may build up, and it could be helpful to utilize backpressure to communicate this to the producers and respond accordingly.

However, the current JS defer/stream implementations, however, do not do this. The official graphql-js implementation and my alternative implementation at graphql-executor, both attempt to consume the async-iterator as returned by a field as fast as possible, without regard to the rate at which the completed values they produce are consumed.

Should there be a mechanism to communicate backpressure as it builds up? Well, with subscriptions, this backpressure basically could signal a failure of an expected real-time event stream, or, even if the client does not care if the stream is "real-time" it could simply mean that the client will never be made aware of all of the data. Strategies such as buffering and dropping values can provide a quasi-solution to the fast producer problem with tradeoffs that still make the stream usable.

With incremental delivery, there is no expectation of real-time data, and there is also an expectation that fields returning async iterators are returning async iterators are bounded, and so after time, the stream will end and the executor, server, client can all "catch up." The only potential problem is that a fast producer could ultimately deliver more payloads that are possible for the executor to hold in memory all at one time. But, because the client may not utilize the stream directive, the executor has to be prepared to hold all of the items in memory at one time anyway, so this possibility is something that has to be accounted for in a different way. It cannot and therefore should not be solved by utilizing backpressure with the async iterator.

At least those are my current thoughts! Would love to hear your response, additional people's ideas, etc. etc. @robrichard @brainkim

This might deserve its own discussion post!

[benjie]

I do not believe this is something that the spec needs to explicitly specify - can we leave it to implementations to figure out what works best for their needs?

[robrichard]

I created two new discussions to separately track the topics being discussed here.

1. [graphql-js] Yielding batches of results in resolver functions #40
2. Yielding batches of payloads from defer/stream GraphQL requests #41