Skip to content

Commit

Permalink
Disable exposeDocumentNodeInFetchNode by default
Browse files Browse the repository at this point in the history
Since apollographql#1878 (added in 2.1.0), the full AST of subgraph fetch operations
has been store in query plans. However, this is not used by the gateway
by default, while it noticeably increases the size of query plans,
leading to either a less efficient query plan cache (if its size is
limited) or increased memory consumption/pressue (if the cache is given
plenty of size).

This patch disable this option by default. Users that explicitely
need to access the subgraph fetch ASTs can still make the trade-off
of additional memory pressure by manually using the
`GatewayConfig.queryPlannerConfig.exposeDocumentNodeInFetchNode` config
option.

Fixes apollographql#2227.
  • Loading branch information
Sylvain Lebresne committed Nov 2, 2022
1 parent cf3bfcc commit dd8cd4e
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 8 deletions.
4 changes: 4 additions & 0 deletions gateway-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The

## vNext

- __BREAKING__: Disable exposing full document to sub-query by default (introduce in in 2.1.0):
- This change decrease memory consumption in general (which is the reason for disabling this by
default), but users that have custom code making use of `GraphQLDataSourceProcessOptions.document`
will now need to explicitly set `GatewayConfig.queryPlannerConfig.exposeDocumentNodeInFetchNode`.
- Allows `@shareable` to be repeatable so it can be allowed on both a type definition and its extensions [PR #2175](https://github.com/apollographql/federation/pull/2175).
- Note that this require the use of the new 2.2 version of the federation spec introduced in this change.
- Preserve default values of input object fields [PR #2218](https://github.com/apollographql/federation/pull/2218).
Expand Down
2 changes: 1 addition & 1 deletion gateway-js/src/__tests__/execution-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export function getFederatedTestingSchema(services: ServiceDefinitionModule[] =
throw new GraphQLSchemaValidationError(compositionResult.errors);
}

const queryPlanner = new QueryPlanner(compositionResult.schema, { exposeDocumentNodeInFetchNode: false} );
const queryPlanner = new QueryPlanner(compositionResult.schema);
const schema = buildSchema(compositionResult.supergraphSdl);

const serviceMap = Object.fromEntries(
Expand Down
4 changes: 4 additions & 0 deletions gateway-js/src/datasources/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ export type GraphQLDataSourceProcessOptions<
context: GatewayGraphQLRequestContext<TContext>['context'];
/**
* The document representation of the request's query being sent to the subgraph, if available.
*
* Note that this field is not populated by default. You can enable it by setting the
* `GatewayConfig.queryPlannerConfig.exposeDocumentNodeInFetchNode` configuration but note that
* this will increase the memory used by the gateway query plan cache.
*/
document?: GatewayGraphQLRequestContext<TContext>['document'];
}
Expand Down
14 changes: 8 additions & 6 deletions gateway-js/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,15 @@ export class ApolloGateway implements GatewayInterface {
}

private initQueryPlanStore(approximateQueryPlanStoreMiB?: number) {
// Create ~about~ a 30MiB InMemoryLRUCache (or 50MiB if the full operation ASTs are
// enabled in query plans as this plan then use quite a bit more memory). This is
// less than precise since the technique to calculate the size of a DocumentNode is
// only using JSON.stringify on the DocumentNode (and thus doesn't account
// for unicode characters, etc.), but it should do a reasonable job at
// providing a caching document store for most operations.
const defaultSize = (this.config.queryPlannerConfig?.exposeDocumentNodeInFetchNode ?? false) ? 50 : 30;
return new LRUCache<string, QueryPlan>({
// Create ~about~ a 30MiB InMemoryLRUCache. This is less than precise
// since the technique to calculate the size of a DocumentNode is
// only using JSON.stringify on the DocumentNode (and thus doesn't account
// for unicode characters, etc.), but it should do a reasonable job at
// providing a caching document store for most operations.
maxSize: Math.pow(2, 20) * (approximateQueryPlanStoreMiB || 30),
maxSize: Math.pow(2, 20) * (approximateQueryPlanStoreMiB || defaultSize),
sizeCalculation: approximateObjectSize,
});
}
Expand Down
4 changes: 4 additions & 0 deletions query-planner-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The

## vNext

- __BREAKING__: Disable exposing full document to sub-query by default (introduce in in 2.1.0):
- This change decrease memory consumption in general (which is the reason for disabling this by
default), but users that have custom code making use of `GraphQLDataSourceProcessOptions.document`
will now need to explicitly set `GatewayConfig.queryPlannerConfig.exposeDocumentNodeInFetchNode`.
- Drop support for node12 [PR #2202](https://github.com/apollographql/federation/pull/2202)

## 2.1.4
Expand Down
16 changes: 15 additions & 1 deletion query-planner-js/src/config.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
import { Concrete } from "@apollo/federation-internals";

export type QueryPlannerConfig = {
/**
* If enabled, the `FetchNode.operationDocumentNode` field in query plan will be populated with the AST
* of the underlying operation (_on top_ of the "serialized" string `FetchNode.operation` which is always
* present). This can used by specific gateway user code that needs read-only access to such AST in
* order to save having to parse `FetchNode.operation`. Without this option, `FetchNode.operationDocumentNode`
* will always be `undefined`.
*
* Enabling this option will make query plans use more memory and you should consider increasing the
* query plan cache size (though `GatewayConfig.experimental_approximateQueryPlanStoreMiB`) if you enable it.
*
* Defaults to false (at least since 2.2; it temporarily defaulted to true before 2.2, but this resulted
* to additional memory consumption even though it is only useful for users that explicitely want to
* use that field).
*/
exposeDocumentNodeInFetchNode?: boolean;

/**
Expand Down Expand Up @@ -39,7 +53,7 @@ export function enforceQueryPlannerConfigDefaults(
config?: QueryPlannerConfig
): Concrete<QueryPlannerConfig> {
return {
exposeDocumentNodeInFetchNode: true,
exposeDocumentNodeInFetchNode: false,
reuseQueryFragments: true,
incrementalDelivery: {
enableDefer: false,
Expand Down

0 comments on commit dd8cd4e

Please sign in to comment.