Should BatchHttpLink exclude mutations by default?

Client SDKs
,
1

BatchHttpLink’s default batchKey function does not distinguish between queries and mutations. Both get grouped into the same batch. I understand that in theory it’s a valid transport level optimization since the server always executes mutations in serial. however…

  1. executing operations in batch is unspecified behavior. is there a risk that a federation router may send mutations to different upstream services to execute in parallel

2. it seems like a large risk of stale data / race conditions / confusing states is large (e.g. Auto-refetch has a race-condition with the mutation and request batching is broken · Issue #11219 · apollographql/apollo-client · GitHub ) to the point where having batching disabled for mutations as a default seems worthwhile

2

Hey @markl :waving_hand:

Let me get back to you on how Apollo Router handles batching mutations so I can get you accurate info.

I can see an argument for wanting to turn off batch mutations. Let me think about this a bit. We might be able to add an option for this in BatchHttpLink, but I’ll see how router handles it first.

3

I got confirmation that router batch executes mutations in parallel, so you’re correct. That said, the more I think about it, does batching really change anything if you have mutations that depend on others? Even if you turned off batching for mutations, sending multiple mutations to the server could still result in out-of-order execution of those mutations right? The safest thing would be to execute serially at the call site:

await mutation(mutation1)
await mutations(mutation2)
// etc
4

@jerelmiller thanks for the response!

From my teammate:

In this case, our mutations don’t depend on each other - we just want to fire them off at the same time. The batching behavior is annoying because network failures from one of the mutations can corrupt the other one or slow it down. So less to do with the race conditiony stuff and more the inherent limitations of using one network req for two operations

the nature of what mutations are doing leads to them wanting to be independent reqs, IMO

error recovery is almost always happening at an individual mutation level, not a batch level

so i think tl;dr while technically a valid optimization strictly for performance, for us at least, it’s more often desirable to disable batching than enabling for mutations

5

because network failures from one of the mutations can corrupt the other one or slow it down

Ah this makes a lot of sense. Let me talk with the team to see if it makes sense to introduce a new option for BatchHttpLink that would let you disable batch mutations by default. Thanks!