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

Enable connection keep-alive on RemoteGraphQLDataSources by default. #1232

Closed
abernix opened this issue Nov 23, 2021 · 0 comments · Fixed by #1284
Closed

Enable connection keep-alive on RemoteGraphQLDataSources by default. #1232

abernix opened this issue Nov 23, 2021 · 0 comments · Fixed by #1284

Comments

@abernix
Copy link
Member

abernix commented Nov 23, 2021

In Apollo Gateway, the default fetcher that the RemoteGraphQLDataSource uses doesn't configure a Node.js http.Agent with the keepAlive: true option. This means that the default behavior is to destroy the underlying TCP sockets when there are not active connections rather than maintaining a pool of established connections.

The current default fetcher in Apollo Gateway is node-fetch. Some alternative fetchers support having keep-alive on by default, but it's also possible to configure node-fetch to have this option enabled by default, as documented in their documentation.

We've found a number of reasons in the past to recommend make-fetch-happen — notably it's out of the box support for proxies, conditional cache control headers, gzip/deflate support, and its request pooling. We should:

  • Evaluate whether it's feasible to change our default fetcher to make-fetch-happen, which solves this. This is a bit of a larger change and something we've punted before. This is actually tracked in Apollo Gateway: Add make-fetch-happen fetcher as the default fetcher for the downstream services #192, so if we take that approach (which has some comments with reading on it and its own associated references!), we may be able to just close this issue as resolved.
  • If that approach is deemed too large, we may want to just keep the default fetcher as node-fetch but configure it to use keepAlive: true.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant