Fix high memory usage when extracting subgraphs for some fed1 supergr…

…aphs (apollographql#2089)

Fed1 supergraph lacks information on value types regarding which
subgraphs defines them. We use to brute-force add all value types to all
extract subgraphs, on the idea that if some extracted subgraphs have a
few unused types, it's useless but has no functional impact.
Unfortunately, in some special cases (lots of subgraphs and value
types), those useless types can lead to a significant increase in
memory consumptions.

This patch instead look at type reachability within subgraphs to avoid
including those useless value types, and thus lower the memory
consumptions.

Note that fed2 supergraphs are not affected by this problem has they
have all the information needed to only extract types in the proper
subgraphs.

Also note that this patch also include a few small improvements to reduce
memory consumption in general, and opportunistically remove the (essentially
unused) dependency on the `core-schema-js` library.

Fixes apollographql#2085

docs/source/errors.md

@@ -91,6 +91,7 @@ The following errors might be raised during composition:
 | `UNKNOWN_FEDERATION_LINK_VERSION` | The version of federation in a @link directive on the schema is unknown. | 2.0.0 |  |
 | `UNKNOWN_LINK_VERSION` | The version of @link set on the schema is unknown. | 2.1.0 |  |
 | `UNSUPPORTED_FEATURE` | Indicates an error due to feature currently unsupported by federation. | 2.1.0 |  |
+| `UNSUPPORTED_LINKED_FEATURE` | Indicates that a feature used in a @link is either unsupported or is used with unsupported options. | 2.0.0 |  |
 
 </div>
 

gateway-js/CHANGELOG.md

@@ -4,6 +4,8 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The
 
 ## vNext
 
+- Fix abnormally high memory usage when extracting subgraphs for some fed1 supergraphs (and small other memory footprint improvements) [PR #2089](https://github.com/apollographql/federation/pull/2089).
+- Fix issue with fragment reusing code something mistakenly re-expanding fragments [PR #2098](https://github.com/apollographql/federation/pull/2098).
 - Fix issue when type is only reachable through a @provides [PR #2083](https://github.com/apollographql/federation/pull/2083).
 - Fix case where some key field necessary to a `@require` fetch were not previously fetched [PR #2075](https://github.com/apollographql/federation/pull/2075).
 - Add type definitions to schema extensions [PR #2081](https://github.com/apollographql/federation/pull/2081)

gateway-js/package.json

@@ -26,7 +26,6 @@
   },
   "dependencies": {
     "@apollo/composition": "file:../composition-js",
-    "@apollo/core-schema": "~0.3.0",
     "@apollo/federation-internals": "file:../internals-js",
     "@apollo/query-planner": "file:../query-planner-js",
     "@apollo/server-gateway-interface": "^1.0.2",

internals-js/package.json

@@ -23,7 +23,6 @@
     "node": ">=12.13.0"
   },
   "dependencies": {
-    "@apollo/core-schema": "~0.3.0",
     "chalk": "^4.1.0",
     "js-levenshtein": "^1.1.6"
   },

internals-js/src/__tests__/coreSpec.test.ts

@@ -1,10 +1,10 @@
 import { DocumentNode, GraphQLError } from "graphql";
 import gql from "graphql-tag";
 import { buildSubgraph } from "../federation";
-import { errorCauses } from "../definitions";
 import { assert } from "../utils";
 import { buildSchemaFromAST } from "../buildSchema";
 import { removeAllCoreFeatures } from "../coreSpec";
+import { errorCauses } from "../error";
 
 function expectErrors(
   subgraphDefs: DocumentNode,

internals-js/src/__tests__/extractSubgraphsFromSupergraph.test.ts

@@ -598,7 +598,6 @@ test('throw meaningful error for invalid federation directive fieldSet', () => {
   );
 })
 
-
 test('throw meaningful error for type erased from supergraph due to extending an entity without a key', () => {
   // Supergraph generated by fed1 composition from:
   // ServiceA:
@@ -616,13 +615,14 @@ test('throw meaningful error for type erased from supergraph due to extending an
   //       x: Int! @external
   //     }
   //
-  //     type Other {
+  //     type Other @key(fields: "id") {
+  //       id: ID!
   //       f: T @provides(fields: "x")
   //     }
   //
   // The issue of that schema is that `T` is referenced in `ServiceB`, but because it extends an entity type
-  // without a key and has noly external field, ther is no remaining traces of it's definition in `ServiceB`
-  // in the supergraph. As extract cannot make up the original definition out of thin air, it ends up erroring
+  // without a key and has only external fields, there is no remaining traces of its definition in `ServiceB`
+  // in the supergraph. As extraction cannot make up the original definition out of thin air, it ends up erroring
   // when extracting `Other.t` due to not knowing that type.
   const supergraph = `
     schema
@@ -642,7 +642,11 @@ test('throw meaningful error for type erased from supergraph due to extending an
 
     directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on INTERFACE | OBJECT
 
-    type Other {
+    type Other
+      @join__owner(graph: SERVICEB)
+      @join__type(graph: SERVICEB, key: "id")
+    {
+      id: ID! @join__field(graph: SERVICEB)
       f: T @join__field(graph: SERVICEB, provides: "x")
     }
 

internals-js/src/__tests__/removeInaccessibleElements.test.ts

@@ -1,15 +1,14 @@
 import {
   ArgumentDefinition,
-  errorCauses,
   FieldDefinition,
   InterfaceType,
   ObjectType,
   UnionType,
 } from "../definitions";
 import { buildSchema } from "../buildSchema";
 import { removeInaccessibleElements } from "../inaccessibleSpec";
-import { GraphQLErrorExt } from "@apollo/core-schema/dist/error";
 import { GraphQLError } from "graphql";
+import { errorCauses } from "../error";
 
 describe("removeInaccessibleElements", () => {
   const INACCESSIBLE_V02_HEADER = `
@@ -31,7 +30,6 @@ describe("removeInaccessibleElements", () => {
   `;
 
   function getCauses(e: unknown): GraphQLError[] {
-    expect(e instanceof GraphQLErrorExt).toBeTruthy();
     const causes = errorCauses(e as Error);
     expect(causes).toBeDefined();
     expect(Array.isArray(causes)).toBeTruthy();

internals-js/src/__tests__/schemaUpgrader.test.ts

@@ -167,7 +167,6 @@ test('update federation directive non-string arguments', () => {
   `);
 })
 
-
 test('remove tag on external field if found on definition', () => {
   const s1 = `
     type Query {

internals-js/src/__tests__/subgraphValidation.test.ts

@@ -1,7 +1,6 @@
 import { DocumentNode } from 'graphql';
 import gql from 'graphql-tag';
-import { Subgraph } from '..';
-import { errorCauses } from '../definitions';
+import { Subgraph, errorCauses } from '..';
 import { asFed2SubgraphDocument, buildSubgraph } from "../federation"
 import { defaultPrintOptions, printSchema } from '../print';
 import './matchers';

internals-js/src/buildSchema.ts

@@ -52,10 +52,9 @@ import {
   EnumType,
   Extension,
   ErrGraphQLValidationFailed,
-  errorCauses,
   NamedSchemaElement,
 } from "./definitions";
-import { ERRORS, withModifiedErrorNodes } from "./error";
+import { ERRORS, errorCauses, withModifiedErrorNodes } from "./error";
 
 function buildValue(value?: ValueNode): any {
   return value ? valueFromASTUntyped(value) : undefined;

internals-js/src/coreSpec.ts

@@ -2,9 +2,8 @@ import { ASTNode, DirectiveLocation, GraphQLError, StringValueNode } from "graph
 import { URL } from "url";
 import { CoreFeature, Directive, DirectiveDefinition, EnumType, ErrGraphQLAPISchemaValidationFailed, ErrGraphQLValidationFailed, InputType, ListType, NamedType, NonNullType, ScalarType, Schema, SchemaDefinition, SchemaElement, sourceASTs } from "./definitions";
 import { sameType } from "./types";
-import { err } from '@apollo/core-schema';
 import { assert, firstOf } from './utils';
-import { ERRORS } from "./error";
+import { aggregateError, ERRORS } from "./error";
 import { valueToString } from "./values";
 import { coreFeatureDefinitionIfKnown, registerKnownFeature } from "./knownCoreFeatures";
 import { didYouMean, suggestionList } from "./suggestions";
@@ -15,11 +14,7 @@ export const linkIdentity = 'https://specs.apollo.dev/link';
 
 export const linkDirectiveDefaultName = 'link';
 
-export const ErrCoreCheckFailed = (causes: Error[]) =>
-  err('CheckFailed', {
-    message: 'one or more checks failed',
-    causes
-  })
+export const ErrCoreCheckFailed = (causes: GraphQLError[]) => aggregateError('CheckFailed', 'one or more checks failed', causes);
 
 function buildError(message: string): Error {
   // Maybe not the right error for this?