feat: optimize gateway by batching queries

[giacomorebonato]

#649

• Current implementation doesn't break any tests and seems to work also when enabling batched queries.
• At the moment I am explicitly telling to the gateway instance of Fastify which ones are the services that support query batching:

gateway.register(mercurius, {
  gateway: {
    services: [
      {
        name: 'add',
        url: 'http://localhost:4001/graphql',
        allowBatchedQueries: true
      }
    ]
  }
})

• I copy pasted some unit test and enabled batched queries on them just for quick feeback

To do:

• verify why in the existing implementation each query is picking its own context and request-headers (shouldn't it be the same) - Left a comment in lib/gateway.js about this
• verify if error handling is consistent to not batched implementation - test/gateway/errors-with-batching.js
• verify if Fastify gateway instance can implicitly know which services have batched queries enabled - This is not possible as I am aware
• verify that a Gateway supports batched queries itself - test/gateway/batching-on-both-gateway-and-services.js

[mcollina]

Good work!

[mcollina]

Why a service would not support batched queries? I would enable that by default.

[giacomorebonato]

I would enable that by default

I agree with you @mcollina and I'll apply this configurable default on the Gateway, as I see it as well as a best practice.

Your question made me think though: I think that maybe a service could have batching disabled because it's not aware of being used as a service from a gateway instance.

[mcollina]

Your question made me think though: I think that maybe a service could have batching disabled because it's not aware of being used as a service from a gateway instance.

How could you disable batching in a service? I think that's part of the GraphQL spec.

[giacomorebonato]

My thought goes to a Service made with Fastify + Mercurius and batched queries disabled:

app.register(mercurius, {
  schema,
  resolvers,
  graphiql: true,
  allowBatchedQueries: false // false is default value when the property is omitted
})

Do you suggest that batched queries should be enabled by default in Mercurius, regardless if it's about building a service for a gateway?

I am a very new contributor to the project, so I might be missing something.

[Eomm]

I would add some comments on the code to understand the context/logic

[mcollina]

Thanks @giacomorebonato, I know understand this better.

When I wrote the issue originally, I thought we would use queries like the following:

query user1 {
  ...
}

query user2 {
  ...
}

Instead, you implemented this using https://mercurius.dev/#/docs/batched-queries - which is a feature I even forgot we supported.

As things are, I think you implemented this correctly and it should be enabled on demand and not by default as it is off-standard.

Note that there is currently no way to execute multiple queries in parallel in GraphQL by the spec: graphql/graphql-spec#375.

Note that batching is currently at rfc stage at: https://github.com/graphql/graphql-over-http/blob/main/rfcs/Batching.md.

Could you please set the default to back `false? I'm sorry for the confusion.

[mcollina]

please uncomment this

[mcollina]

This does not match the implementation anymore.

[mcollina]

lgtm