Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gateway: Allow use of APQ when querying downstream services. #3744

Merged
merged 5 commits into from
Feb 7, 2020

Conversation

abernix
Copy link
Member

@abernix abernix commented Jan 31, 2020

Note: This PR targets #3743 #3763 (Release 2.11.0).

This introduces support to Apollo Gateway which can leverage Automated Persisted Queries (APQ) when communicating with downstream implementing services in a federated architecture.

In an identical way as APQ saves bytes on the wire (read: network) in a traditional handshake between a client and the server, this implements that behavior in the internal communication between federated servers and the gateway that fronts them.

This is accomplished by first attempting to utilizing a SHA-256 hex hash (consistently, 32 bytes) of the operation which is destined for the downstream server, rather than the complete, typically much larger query itself.

In the event that the downstream server supports APQ (Apollo Server does by default, unless it's been disabled), the downstream server will check its APQ registry for the full operation body. If it has it, it will use that cached body to process the request and return the results. If it does not find the query in its existing registry, it will return a message to the gateway that the query is not found in its registry, and the gateway will re-transmit the request with the full operation payload. On receipt of this full query, the downstream server will cache the operation for future requests (keyed by the SHA-256 hash), and return the expected result without further delay.

This means that once a server has warmed up its APQ registry with repeated operations, subsequent network chatter will be greatly reduced. Furthermore, as noted in the attached documentation, the APQ registry can be backed by a distributed store, such as Memcached or Redis, allowing multiple downstream GraphQL servers to share the same cache, and persist it across restarts.

By default, APQ behavior is disabled on RemoteGraphQLDataSource. To enable it, the apq property should be set to true. Future versions of the RemoteGraphQLDataSource could negotiate this capability on their own, but this does not attempt to make that negotiation right now.

@abernix abernix requested a review from trevor-scheer January 31, 2020 01:36
abernix added a commit that referenced this pull request Jan 31, 2020
@abernix abernix requested a review from jbaxleyiii January 31, 2020 01:40
@abernix abernix added this to the Release 2.11.0 milestone Feb 6, 2020
@abernix abernix changed the base branch from abernix/breaking-standarize-and-simplify-gateway-didReceiveResponse to release-2.11.0 February 7, 2020 15:04
@abernix abernix force-pushed the abernix/gateway-fetch-with-apq-part-deux branch from 27da23a to 469dcd4 Compare February 7, 2020 15:11
…hes.

It's plausible that we should change every existing `toHaveFetch` to use
this matcher which enforces order.   Though it also seems plausible that
custom matchers aren't as flexible as they need to be in practice, since in
addition to the need to use Jest-built in methods (like the `nth`-call
matchers) there are other specific usages of this which are just surfacing
now (with APQ) that could be tested less precisely using
`expect.objectContaining()`, rather than testing concerns which are not
really necessary (like matching the hash).

If this matcher still supported partial matches then this would be possible.
However, since we're serializing the body into an `Request` before matching it
(which I'm not sure why we do and there is no comment to indicate why) this
isn't possible as Jest's matchers cannot survive that serialization.
Without this change, every single usage of our `Matcher` is represented as a
type error since Jest has changed their own implementation of `Matcher` to
introduce a new generic type argument.

It's too bad that this didn't fail tests at the time that that Jest package
was updated!
…` method.

This introduces a new private `sendRequest` method that handles existing
behavior which existed within `RemoteGraphQLDataSource`'s `process`.

It should be a no-op change.

An upcoming commit will make multiple requests to downstream services in its
course of attempting APQ negotiation and this should facilitate that change
and avoid repetition of logic.
This introduces support to Apollo Gateway which can leverage [Automated
Persisted Queries] (APQ) when communicating with downstream
implementing services in a federated architecture.

In an identical way as APQ saves bytes on the wire (read: network) in a
traditional handshake between a client and the server, this implements that behavior
in the internal communication between federated servers and the gateway that
fronts them.

This is accomplished by first attempting to utilizing a SHA-256 hex hash
(consistently, 32 bytes) of the operation which is destined for the
downstream server, rather than the complete, typically much larger query
itself.

In the event that the downstream server supports APQ (Apollo Server does by
default, unless it's been disabled), the downstream server will check its
APQ registry for the full operation body.  If it has it, it will use that
cached body to process the request and return the results.  If it does not
find the query in its existing registry, it will return a message to the
gateway that the query is not found in its registry, and the gateway will
re-transmit the request with the full operation payload.  On receipt of this
full query, the downstream server will cache the operation for future
requests (keyed by the SHA-256 hash), and return the expected result without
further delay.

This means that once a server has warmed up its APQ registry with repeated
operations, subsequent network chatter will be greatly reduced.
Furthermore, as noted in the attached documentation, the APQ registry can be
backed by a distributed store, such as Memcached or Redis, allowing multiple
downstream GraphQL servers to share the same cache, and persist it across
restarts.

By default, APQ behavior is disabled on `RemoteGraphQLDataSource`.  To
enable it, the `apq` property should be set to true.  Future versions of the
`RemoteGraphQLDataSource` could negotiate this capability on their own, but
this does not attempt to make that negotiation right now.

[Automated Persisted Queries]: https://www.apollographql.com/docs/apollo-server/performance/apq/
@abernix abernix force-pushed the abernix/gateway-fetch-with-apq-part-deux branch from 469dcd4 to 24c0be5 Compare February 7, 2020 15:13
@abernix abernix merged commit 7a8826a into release-2.11.0 Feb 7, 2020
@abernix abernix deleted the abernix/gateway-fetch-with-apq-part-deux branch February 7, 2020 15:20
@trevor-scheer trevor-scheer added ⛲️ feature New addition or enhancement to existing solutions 👩‍🚀 federation labels Feb 17, 2020
Comment on lines +118 to +121
const requestWithQuery: GraphQLRequest = {
query,
...requestWithoutQuery,
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: I think requestWithQuery === request from above (the one that we destructured).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wrong, since in the APQ case, requestWithoutQuery.extensions is mutated and it seems we need to pass those along in the request as well. I'll leave this for you to resolve in case you arrive at a different conclusion than I.

const DataSource = new MyDataSource();
const spyDidReceiveResponse = jest.spyOn(DataSource, 'didReceiveResponse');

fetch.mockJSONResponseOnce({ data: { me: 'james' } });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to also simulate the initial miss for this test to be valid?

Via this line which is used in other tests:
fetch.mockJSONResponseOnce(apqNotFoundResponse);

abernix added a commit to apollographql/federation that referenced this pull request Sep 4, 2020
…raphql/apollo-server#3744)

* gateway: Allow use of APQ when querying downstream services.

This introduces support to Apollo Gateway which can leverage [Automated
Persisted Queries] (APQ) when communicating with downstream
implementing services in a federated architecture.

In an identical way as APQ saves bytes on the wire (read: network) in a
traditional handshake between a client and the server, this implements that behavior
in the internal communication between federated servers and the gateway that
fronts them.

This is accomplished by first attempting to utilizing a SHA-256 hex hash
(consistently, 32 bytes) of the operation which is destined for the
downstream server, rather than the complete, typically much larger query
itself.

In the event that the downstream server supports APQ (Apollo Server does by
default, unless it's been disabled), the downstream server will check its
APQ registry for the full operation body.  If it has it, it will use that
cached body to process the request and return the results.  If it does not
find the query in its existing registry, it will return a message to the
gateway that the query is not found in its registry, and the gateway will
re-transmit the request with the full operation payload.  On receipt of this
full query, the downstream server will cache the operation for future
requests (keyed by the SHA-256 hash), and return the expected result without
further delay.

This means that once a server has warmed up its APQ registry with repeated
operations, subsequent network chatter will be greatly reduced.
Furthermore, as noted in the attached documentation, the APQ registry can be
backed by a distributed store, such as Memcached or Redis, allowing multiple
downstream GraphQL servers to share the same cache, and persist it across
restarts.

By default, APQ behavior is disabled on `RemoteGraphQLDataSource`.  To
enable it, the `apq` property should be set to true.  Future versions of the
`RemoteGraphQLDataSource` could negotiate this capability on their own, but
this does not attempt to make that negotiation right now.

[Automated Persisted Queries]: https://www.apollographql.com/docs/apollo-server/performance/apq/

* tests: De-compose `toHaveFetched` in preparation for similar matchers.

* tests: Introduce `toHaveFetchNth` with to test critical order of fetches.

It's plausible that we should change every existing `toHaveFetch` to use
this matcher which enforces order.   Though it also seems plausible that
custom matchers aren't as flexible as they need to be in practice, since in
addition to the need to use Jest-built in methods (like the `nth`-call
matchers) there are other specific usages of this which are just surfacing
now (with APQ) that could be tested less precisely using
`expect.objectContaining()`, rather than testing concerns which are not
really necessary (like matching the hash).

If this matcher still supported partial matches then this would be possible.
However, since we're serializing the body into an `Request` before matching it
(which I'm not sure why we do and there is no comment to indicate why) this
isn't possible as Jest's matchers cannot survive that serialization.

* tests: Make `Matcher` declaration merges match new Jest definitions.

Without this change, every single usage of our `Matcher` is represented as a
type error since Jest has changed their own implementation of `Matcher` to
introduce a new generic type argument.

It's too bad that this didn't fail tests at the time that that Jest package
was updated!

* Re-jigger `RemoteGraphQLDataSource`'s `process` with new `sendRequest` method.

This introduces a new private `sendRequest` method that handles existing
behavior which existed within `RemoteGraphQLDataSource`'s `process`.

It should be a no-op change.

An upcoming commit will make multiple requests to downstream services in its
course of attempting APQ negotiation and this should facilitate that change
and avoid repetition of logic.

Apollo-Orig-Commit-AS: apollographql/apollo-server@7a8826a
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⛲️ feature New addition or enhancement to existing solutions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants