:wave: I’m looking at 2.5.8 release, and query bat...

# apollo-kotlin

[wasyl]

👋 I’m looking at 2.5.8 release, and query batching looks great, pretty excited to check it out! While looking at it, I recalled one similar optimization: attaching new queries to ones that are still running. For example I query Foo(a, b) and it will make network request that finishes after ~500ms. ~200ms from the first query I make second, same Foo(a, b) query, so the first one is still running.
Would it make sense for Apollo to merge the same requests internally, and if a network request is already running — just attach the new observer to the existing one and share the result? Similarly, everything related to observing the cache could be shared as well.
This is something that could be done on the app side, but seems like a useful optimisation that could be part of Apollo. Any thoughts on this?

[mbonnin]

including @Joaquim who implemented the query batching in this conversation

[mbonnin]

That sounds legit. It doesn't work in the general case because the client might actually want different results (if polling for an example)

[wasyl]

(if polling for an example)

that’s related more the latter part, about sharing observing cache, right? Or you mean some people would want to make network requests so often that they overlap, and they still want each overlapping request to hit the remote server?

[wasyl]

Of course someone will want this 😄 I’d just expect this to be an exception rather than norm, even with polling

[mbonnin]

some people would want to make network requests so often that they overlap

Yea that sounds like a stretch but this is what I meant indeed...

[mbonnin]

This will be someone's workflow

[mbonnin]

Since queries and input types are data classes, I think we could use the whole query as a key to lookup the operations in-flight...

[wasyl]

So for clarity I’ll just split this into two things:
1. attaching new potential network calls to a running one, if such exists — changes behavior
2. attaching new callbacks to observe cache to an existing one — purely an optimisation to avoid unnecessary calculations and database queries

[mbonnin]

Could that be the same thing? If the check is made early in the interceptor chain?

[mbonnin]

That becomes more complicated with watchers...

[wasyl]

Sure, I don’t have an opinion here 🙂 I thought since the first point should be configurable it’d make sense to have a distinction

[mbonnin]

In theory I think 2. is also the same kind of behaviour change. If you run multiple queries in parallel, the cache could be updated between 2 other queries

[mbonnin]

Of course that's like the 0.01%

[wasyl]

Not sure I get the scenario, but yes, maybe after such change a stream could get an emission more/less because it’d be faster to attach to an existing cache stream than to create a new one. Is that what you mean?

[mbonnin]

Yea, I might be overthinking this

[wasyl]

Is the a place for (1) in the code where you think it’d be relatively straightforward to implement? We attempted something a while back on app side with an apollo interceptor but it was pretty chaotic at the time, and we were very unsure about threading concerns etc.

[mbonnin]

I'd do that in dev-3.x if you're willing to try it out. Maybe a custom DedupHttpTransport that maintains a set of queries in flight.

[mbonnin]

In a way, this problem is similar to subscriptions where the requests are serialized into a websocket. Except that here the dispatch would use the operation hashCode instead of the subscription id

[wasyl]

I’d do that in dev-3.x if you’re willing to try it out.

Depends on how much breaking changes there are 😄 I only have one, not trivial, project I can try stuff on

[mbonnin]

There will be breaking changes

[mbonnin]

Let me see if I find a place to do this in main

[mbonnin]

Ah I remember now. main is more difficult because it doesn't allow modifying the last interceptors in the chain

[wasyl]

overall I’m willing to test new releases and give feedback, but it’s not something we could push to prod obviously, and we have limited throughput for such things. So I’ll try but no promises

[mbonnin]

Yep, that's fair.

[mbonnin]

How much of an issue that is? I'd expect that a typical project would not have that many duplicate operations but maybe you have a use case where it's more frequent?

[wasyl]

It’s not a huge deal, just something we’ve had in a backlog for quite some time because it affects our UI tests (but that’s UI tests fault as well). It’s usually maybe 2 queries duplicated twice running from different parts of the app on startup

[wasyl]

And it’s only an issue because queries make network requests by default, and it’d be easier (I think) to centralize network requests if all queries use cache only and a network call is explicit. But that’s another topic and a much bigger refactor for us

[mbonnin]

Got it, thanks for the details 👍