Variable deduplication in query planner

[bnjjj]

close #87

Before this change we had this __entities:

entities.len() 8
entities [
    Object({
        "name": String(
            "Table",
        ),
    }),
    Object({
        "name": String(
            "Couch",
        ),
    }),
    Object({
        "name": String(
            "Chair",
        ),
    }),
    Object({
        "name": String(
            "Table",
        ),
    }),
    Object({
        "name": String(
            "Table",
        ),
    }),
    Object({
        "name": String(
            "Couch",
        ),
    }),
    Object({
        "name": String(
            "Chair",
        ),
    }),
    Object({
        "name": String(
            "Table",
        ),
    }),
]

Now we have:

entities len 3
entities [
    Object({
        "name": String(
            "Table",
        ),
    }),
    Object({
        "name": String(
            "Couch",
        ),
    }),
    Object({
        "name": String(
            "Chair",
        ),
    }),
]

in responses, but I also deduplicate in the requests so it's less traffic either on the request and the response.

[netlify]

✅ Deploy Preview for apollo-router-docs canceled.

Name	Link
🔨 Latest commit	54b11d2
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-router-docs/deploys/629f1bab4f715b000aabc08c

[BrynCooke]

Let's not merge this before GA.

[abernix]

I love this, with a timing catch. 😉

As long as we're doing deep comparisons on the representations, this is safe and desirable. This is also something that we've wanted to do for a very long time in the Gateway and something users have wanted!

With that in mind, my suggestion is that we either:

1. Punting this until after GA; or
2. Making this a configuration option that is off by default

I suspect my preference could be option 2.

My primary motivation for this suggestion is that I believe this could make it more challenging for adopters (e.g., SREs analyzing such a deployment) to accurately analyze the differences in behavior between the Gateway and the Router, which we have tried to keep largely the same. It would be easier if the shapes of the subgraph requests were as similar as possible — this might tie into whatever tool/diffs that come out of #765 if someone was comparing Router vs Gateway.

As a secondary motivation, because this is an optimization while we're in a push to GA (which I'd claim should primarily on stabilizing what we have, rather than new features that differentiate things from the Gateway — even if desirable!).

There are, I think, a number of users who are interested in this that have probably opened issues on the federation repository that we might want to have try it out, so a configuration option could enable that. It's also just a great fast-follow to GA.

My one technical note on this PR is that I think it should introduce some tests, and administratively, looking for a CHANGELOG.md.

Thoughts?

[Geal]

Deep comparisons should not be a huge issue, as the Hash implementation does it. If there's a case where a representation is similar but does not hash the same way (example: fields in different order), it should be safe, because it would only result in more data requested.
I agree though, that needs more tests. But still, great work 😃

[bnjjj]

Discussed with @BrynCooke, let's merge it POST-GA

[BrynCooke]

I still believe that this can be activated via the traffic shaping plugin.

QueryPlannerRequest should have QueryPlanOptions as a field and the traffic shaping plugin should mutate the options to enable dedup.

[bnjjj]

Ok if we're ok to expose the queryplan options then indeed I think it could work. But all logics about dedup will stay in core and not in the plugin itself, we're just talking about moving the configuration to the plugin.