From 178d77f731249bfe692da26d3dd22e884017d626 Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Mon, 3 Mar 2025 16:05:02 -0600 Subject: [PATCH 1/8] Add support to automatically cancel query plan request if it takes longer than a pre-configured amount of time. --- .changeset/beige-suits-occur.md | 6 ++++++ query-planner-js/src/buildPlan.ts | 22 +++++++++++++++++----- query-planner-js/src/config.ts | 6 ++++++ 3 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 .changeset/beige-suits-occur.md diff --git a/.changeset/beige-suits-occur.md b/.changeset/beige-suits-occur.md new file mode 100644 index 000000000..e206d47a6 --- /dev/null +++ b/.changeset/beige-suits-occur.md @@ -0,0 +1,6 @@ +--- +"@apollo/query-planner": minor +"@apollo/gateway": minor +--- + +Query planner now has support to automatically abort query plan requests that take longer than a configured amount of time. Default value is 2 minutes. Value is set by `maxQueryPlanningTime` value in `QueryPlannerConfig` options. diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index 9c51f5b63..b3336cbb6 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -105,6 +105,7 @@ import { Conditions, conditionsOfSelectionSet, isConstantCondition, mergeConditi import { enforceQueryPlannerConfigDefaults, QueryPlannerConfig, validateQueryPlannerConfig } from "./config"; import { generateAllPlansAndFindBest } from "./generateAllPlans"; import { QueryPlan, ResponsePath, SequenceNode, PlanNode, ParallelNode, FetchNode, SubscriptionNode, trimSelectionNodes } from "./QueryPlan"; +import { performance } from 'perf_hooks'; const debug = newDebugLogger('plan'); @@ -394,6 +395,7 @@ class QueryPlanningTraversal { readonly costFunction: CostFunction, initialContext: PathContext, typeConditionedFetching: boolean, + readonly deadline: number, excludedDestinations: ExcludedDestinations = [], excludedConditions: ExcludedConditions = [], ) { @@ -445,6 +447,9 @@ class QueryPlanningTraversal { } private handleOpenBranch(selection: Selection, options: SimultaneousPathsWithLazyIndirectPaths[]) { + if (performance.now() > this.deadline) { + throw new Error('Query plan took too long to execute'); + } const operation = selection.element; debug.group(() => `Handling open branch: ${operation}`); let newOptions: SimultaneousPathsWithLazyIndirectPaths[] = []; @@ -771,6 +776,7 @@ class QueryPlanningTraversal { this.costFunction, context, this.typeConditionedFetching, + this.deadline, excludedDestinations, addConditionExclusion(excludedConditions, edge.conditions), ).findBestPlan(); @@ -3531,10 +3537,10 @@ function computePlanInternal({ let main: PlanNode | undefined = undefined; let primarySelection: MutableSelectionSet | undefined = undefined; let deferred: DeferredNode[] = []; - - const { operation, processor } = parameters; + const { operation, processor, config } = parameters; + const deadline = performance.now() + config.maxQueryPlanningTime; if (operation.rootKind === 'mutation') { - const dependencyGraphs = computeRootSerialDependencyGraph(parameters, hasDefers); + const dependencyGraphs = computeRootSerialDependencyGraph(parameters, hasDefers, deadline); for (const dependencyGraph of dependencyGraphs) { const { main: localMain, deferred: localDeferred } = dependencyGraph.process(processor, operation.rootKind); // Note that `reduceSequence` "flatten" sequence if needs be. @@ -3554,6 +3560,7 @@ function computePlanInternal({ parameters, 0, hasDefers, + deadline, ); ({ main, deferred } = dependencyGraph.process(processor, operation.rootKind)); primarySelection = dependencyGraph.deferTracking.primarySelection; @@ -3675,12 +3682,14 @@ function computeRootParallelDependencyGraph( parameters: PlanningParameters, startFetchIdGen: number, hasDefer: boolean, + deadline: number, ): FetchDependencyGraph { return computeRootParallelBestPlan( parameters, parameters.operation.selectionSet, startFetchIdGen, hasDefer, + deadline, )[0]; } @@ -3689,6 +3698,7 @@ function computeRootParallelBestPlan( selection: SelectionSet, startFetchIdGen: number, hasDefers: boolean, + deadline: number, ): [FetchDependencyGraph, OpPathTree, number] { const planningTraversal = new QueryPlanningTraversal( parameters, @@ -3699,6 +3709,7 @@ function computeRootParallelBestPlan( defaultCostFunction, emptyContext, parameters.config.typeConditionedFetching, + deadline, ); const plan = planningTraversal.findBestPlan(); // Getting no plan means the query is essentially unsatisfiable (it's a valid query, but we can prove it will never return a result), @@ -3726,6 +3737,7 @@ function onlyRootSubgraph(graph: FetchDependencyGraph): string { function computeRootSerialDependencyGraph( parameters: PlanningParameters, hasDefers: boolean, + deadline: number, ): FetchDependencyGraph[] { const { supergraphSchema, federatedQueryGraph, operation, root } = parameters; const rootType = hasDefers ? supergraphSchema.schemaDefinition.rootType(root.rootKind) : undefined; @@ -3733,10 +3745,10 @@ function computeRootSerialDependencyGraph( const splittedRoots = splitTopLevelFields(operation.selectionSet); const graphs: FetchDependencyGraph[] = []; let startingFetchId = 0; - let [prevDepGraph, prevPaths] = computeRootParallelBestPlan(parameters, splittedRoots[0], startingFetchId, hasDefers); + let [prevDepGraph, prevPaths] = computeRootParallelBestPlan(parameters, splittedRoots[0], startingFetchId, hasDefers, deadline); let prevSubgraph = onlyRootSubgraph(prevDepGraph); for (let i = 1; i < splittedRoots.length; i++) { - const [newDepGraph, newPaths] = computeRootParallelBestPlan(parameters, splittedRoots[i], prevDepGraph.nextFetchId(), hasDefers); + const [newDepGraph, newPaths] = computeRootParallelBestPlan(parameters, splittedRoots[i], prevDepGraph.nextFetchId(), hasDefers, deadline); const newSubgraph = onlyRootSubgraph(newDepGraph); if (prevSubgraph === newSubgraph) { // The new operation (think 'mutation' operation) is on the same subgraph than the previous one, so we can concat them in a single fetch diff --git a/query-planner-js/src/config.ts b/query-planner-js/src/config.ts index db460174f..edc29d79d 100644 --- a/query-planner-js/src/config.ts +++ b/query-planner-js/src/config.ts @@ -118,6 +118,11 @@ export type QueryPlannerConfig = { * If you aren't aware of this flag, you probably don't need it. */ typeConditionedFetching?: boolean, + + /** + * The maximium time to wait for query planning to complete before giving up and throwing an exception. Defaults to 2 minutes + */ + maxQueryPlanningTime?: number, } export function enforceQueryPlannerConfigDefaults( @@ -143,6 +148,7 @@ export function enforceQueryPlannerConfigDefaults( ...config?.debug, }, typeConditionedFetching: config?.typeConditionedFetching || false, + maxQueryPlanningTime: 2 * 60 * 1000, // two minutes }; } From f207d438a88f7b9e3a6c119b740ae49b16451862 Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Mon, 3 Mar 2025 16:08:22 -0600 Subject: [PATCH 2/8] update deadline comment --- query-planner-js/src/buildPlan.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index b3336cbb6..30eb6ec19 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -395,6 +395,8 @@ class QueryPlanningTraversal { readonly costFunction: CostFunction, initialContext: PathContext, typeConditionedFetching: boolean, + + // time (calculated relative from performance.now) after which query plan calcuation may be aborted readonly deadline: number, excludedDestinations: ExcludedDestinations = [], excludedConditions: ExcludedConditions = [], From 566751ec19d0fbc6d86a2afdd91afd813c998769 Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Mon, 3 Mar 2025 16:30:25 -0600 Subject: [PATCH 3/8] change error message --- query-planner-js/src/buildPlan.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index 30eb6ec19..6037f5b1e 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -450,7 +450,7 @@ class QueryPlanningTraversal { private handleOpenBranch(selection: Selection, options: SimultaneousPathsWithLazyIndirectPaths[]) { if (performance.now() > this.deadline) { - throw new Error('Query plan took too long to execute'); + throw new Error('Query plan took too long to calculate'); } const operation = selection.element; debug.group(() => `Handling open branch: ${operation}`); From d525d96f4aac03a3c8b4d48c709ead29386c0dde Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Mon, 3 Mar 2025 20:13:55 -0600 Subject: [PATCH 4/8] Adding test and fixing a bug --- .../src/__tests__/buildPlan.test.ts | 93 +++++++++++++++++++ query-planner-js/src/config.ts | 2 +- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/query-planner-js/src/__tests__/buildPlan.test.ts b/query-planner-js/src/__tests__/buildPlan.test.ts index 97a008564..a74fe512b 100644 --- a/query-planner-js/src/__tests__/buildPlan.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.test.ts @@ -10721,3 +10721,96 @@ test('ensure we are removing all useless groups', () => { } `); }); + +describe('`maxQueryPlanningTime` configuration', () => { + // Simple schema that should still take longer the a millisecond to plan + const typeDefs = gql` + type Query { + t: T @shareable + } + + type T @key(fields: "id") @shareable { + id: ID! + v1: Int + v2: Int + v3: Int + v4: Int + } + `; + + const subgraphs = [ + { + name: 'Subgraph1', + typeDefs, + }, + { + name: 'Subgraph2', + typeDefs, + }, + ]; + + test('maxQueryPlanningTime works when not set', () => { + const config = { debug: { maxEvaluatedPlans: undefined } }; + const [api, queryPlanner] = composeAndCreatePlannerWithOptions( + subgraphs, + config, + ); + const operation = operationFromDocument( + api, + gql` + { + t { + v1 + v2 + v3 + v4 + } + } + `, + ); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Fetch(service: "Subgraph1") { + { + t { + v1 + v2 + v3 + v4 + } + } + }, + } + `); + + const stats = queryPlanner.lastGeneratedPlanStatistics(); + expect(stats?.evaluatedPlanCount).toBe(16); + }); + + test('maxQueryPlanningTime exceeded (500 microseconds)', () => { + const config = { debug: { maxEvaluatedPlans: undefined }, maxQueryPlanningTime: .50 }; + const [api, queryPlanner] = composeAndCreatePlannerWithOptions( + subgraphs, + config, + ); + const operation = operationFromDocument( + api, + gql` + { + t { + v1 + v2 + v3 + v4 + } + } + `, + ); + + expect(() => queryPlanner.buildQueryPlan(operation)).toThrow( + 'Query plan took too long to calculate', + ); + }); +}); diff --git a/query-planner-js/src/config.ts b/query-planner-js/src/config.ts index edc29d79d..0423662cd 100644 --- a/query-planner-js/src/config.ts +++ b/query-planner-js/src/config.ts @@ -133,6 +133,7 @@ export function enforceQueryPlannerConfigDefaults( reuseQueryFragments: true, generateQueryFragments: false, cache: new InMemoryLRUCache({maxSize: Math.pow(2, 20) * 50 }), + maxQueryPlanningTime: 2 * 60 * 1000, // two minutes ...config, incrementalDelivery: { enableDefer: false, @@ -148,7 +149,6 @@ export function enforceQueryPlannerConfigDefaults( ...config?.debug, }, typeConditionedFetching: config?.typeConditionedFetching || false, - maxQueryPlanningTime: 2 * 60 * 1000, // two minutes }; } From 819cf1c5e230ff84dad70002180a31afc30b035f Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Tue, 4 Mar 2025 09:45:10 -0600 Subject: [PATCH 5/8] Run prettier --- query-planner-js/src/__tests__/buildPlan.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/query-planner-js/src/__tests__/buildPlan.test.ts b/query-planner-js/src/__tests__/buildPlan.test.ts index a74fe512b..f6e6be0a1 100644 --- a/query-planner-js/src/__tests__/buildPlan.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.test.ts @@ -10790,7 +10790,10 @@ describe('`maxQueryPlanningTime` configuration', () => { }); test('maxQueryPlanningTime exceeded (500 microseconds)', () => { - const config = { debug: { maxEvaluatedPlans: undefined }, maxQueryPlanningTime: .50 }; + const config = { + debug: { maxEvaluatedPlans: undefined }, + maxQueryPlanningTime: 0.5, + }; const [api, queryPlanner] = composeAndCreatePlannerWithOptions( subgraphs, config, From f6d45b597445121506b721268d448dc13dca229f Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Tue, 4 Mar 2025 09:47:03 -0600 Subject: [PATCH 6/8] fix spelling errors --- query-planner-js/src/buildPlan.ts | 2 +- query-planner-js/src/config.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index 6037f5b1e..f85c879df 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -396,7 +396,7 @@ class QueryPlanningTraversal { initialContext: PathContext, typeConditionedFetching: boolean, - // time (calculated relative from performance.now) after which query plan calcuation may be aborted + // time (calculated relative from performance.now) after which query plan calculation may be aborted readonly deadline: number, excludedDestinations: ExcludedDestinations = [], excludedConditions: ExcludedConditions = [], diff --git a/query-planner-js/src/config.ts b/query-planner-js/src/config.ts index 0423662cd..914a399a0 100644 --- a/query-planner-js/src/config.ts +++ b/query-planner-js/src/config.ts @@ -120,7 +120,7 @@ export type QueryPlannerConfig = { typeConditionedFetching?: boolean, /** - * The maximium time to wait for query planning to complete before giving up and throwing an exception. Defaults to 2 minutes + * The maximum time to wait for query planning to complete before giving up and throwing an exception. Defaults to 2 minutes */ maxQueryPlanningTime?: number, } From 3f15f2f1bb0f4935ba160c46fca531c4d2d60fee Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Tue, 4 Mar 2025 09:52:46 -0600 Subject: [PATCH 7/8] making test more strict --- query-planner-js/src/__tests__/buildPlan.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/query-planner-js/src/__tests__/buildPlan.test.ts b/query-planner-js/src/__tests__/buildPlan.test.ts index f6e6be0a1..67292c292 100644 --- a/query-planner-js/src/__tests__/buildPlan.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.test.ts @@ -10789,10 +10789,10 @@ describe('`maxQueryPlanningTime` configuration', () => { expect(stats?.evaluatedPlanCount).toBe(16); }); - test('maxQueryPlanningTime exceeded (500 microseconds)', () => { + test('maxQueryPlanningTime exceeded (10 microseconds)', () => { const config = { debug: { maxEvaluatedPlans: undefined }, - maxQueryPlanningTime: 0.5, + maxQueryPlanningTime: 0.01, }; const [api, queryPlanner] = composeAndCreatePlannerWithOptions( subgraphs, From 51c3c1735aac81d609b7a127cd422b9ff63268a4 Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Mon, 10 Mar 2025 09:33:36 -0500 Subject: [PATCH 8/8] Add deadline to advancePathsWithOperation --- composition-js/src/compose.ts | 6 +++++- composition-js/src/validate.ts | 4 ++++ query-graphs-js/src/__tests__/graphPath.test.ts | 15 +++++++++------ query-graphs-js/src/conditionsValidation.ts | 9 ++++++++- query-graphs-js/src/graphPath.ts | 13 +++++++++++++ query-planner-js/src/buildPlan.ts | 1 + query-planner-js/src/config.ts | 2 +- 7 files changed, 41 insertions(+), 9 deletions(-) diff --git a/composition-js/src/compose.ts b/composition-js/src/compose.ts index 3ec5711fc..4ad3964df 100644 --- a/composition-js/src/compose.ts +++ b/composition-js/src/compose.ts @@ -17,6 +17,7 @@ import { buildFederatedQueryGraph, buildSupergraphAPIQueryGraph } from "@apollo/ import { MergeResult, mergeSubgraphs } from "./merging"; import { validateGraphComposition } from "./validate"; import { CompositionHint } from "./hints"; +import { performance } from 'perf_hooks'; export type CompositionResult = CompositionFailure | CompositionSuccess; @@ -133,7 +134,10 @@ export function validateSatisfiability({ supergraphSchema, supergraphSdl} : Sati const supergraph = supergraphSchema ? new Supergraph(supergraphSchema, null) : Supergraph.build(supergraphSdl, { supportedFeatures: null }); const supergraphQueryGraph = buildSupergraphAPIQueryGraph(supergraph); const federatedQueryGraph = buildFederatedQueryGraph(supergraph, false); - return validateGraphComposition(supergraph.schema, supergraph.subgraphNameToGraphEnumValue(), supergraphQueryGraph, federatedQueryGraph); + + // for satisfiability, we don't want to really have a deadline here, so set it far into the future + const deadline = performance.now() + 1000 * 60 * 60 * 24; // 1 day + return validateGraphComposition(deadline, supergraph.schema, supergraph.subgraphNameToGraphEnumValue(), supergraphQueryGraph, federatedQueryGraph); } type ValidateSubgraphsAndMergeResult = MergeResult | { errors: GraphQLError[] }; diff --git a/composition-js/src/validate.ts b/composition-js/src/validate.ts index 1cb477e5a..c77bcc0dc 100644 --- a/composition-js/src/validate.ts +++ b/composition-js/src/validate.ts @@ -306,6 +306,7 @@ function generateWitnessValue(type: InputType): any { * @param federatedQueryGraph the (federated) `QueryGraph` corresponding the subgraphs having been composed to obtain `supergraphSchema`. */ export function validateGraphComposition( + deadline: number, supergraphSchema: Schema, subgraphNameToGraphEnumValue: Map, supergraphAPI: QueryGraph, @@ -315,6 +316,7 @@ export function validateGraphComposition( hints? : CompositionHint[], } { const { errors, hints } = new ValidationTraversal( + deadline, supergraphSchema, subgraphNameToGraphEnumValue, supergraphAPI, @@ -697,12 +699,14 @@ class ValidationTraversal { private readonly context: ValidationContext; constructor( + deadline: number, supergraphSchema: Schema, subgraphNameToGraphEnumValue: Map, supergraphAPI: QueryGraph, federatedQueryGraph: QueryGraph, ) { this.conditionResolver = simpleValidationConditionResolver({ + deadline, supergraph: supergraphSchema, queryGraph: federatedQueryGraph, withCaching: true, diff --git a/query-graphs-js/src/__tests__/graphPath.test.ts b/query-graphs-js/src/__tests__/graphPath.test.ts index a1326938e..d3900b55a 100644 --- a/query-graphs-js/src/__tests__/graphPath.test.ts +++ b/query-graphs-js/src/__tests__/graphPath.test.ts @@ -15,6 +15,9 @@ import { import { QueryGraph, Vertex, buildFederatedQueryGraph } from "../querygraph"; import { emptyContext } from "../pathContext"; import { simpleValidationConditionResolver } from "../conditionsValidation"; +import { performance } from 'perf_hooks'; + +const FAR_FUTURE_DEADLINE = performance.now() + 1000 * 60 * 60 * 24; function parseSupergraph(subgraphs: number, schema: string): { supergraph: Schema, api: Schema, queryGraph: QueryGraph } { assert(subgraphs >= 1, 'Should have at least 1 subgraph'); @@ -68,7 +71,7 @@ function createOptions(supergraph: Schema, queryGraph: QueryGraph): Simultaneous return createInitialOptions( initialPath, emptyContext, - simpleValidationConditionResolver({ supergraph, queryGraph }), + simpleValidationConditionResolver({ deadline: FAR_FUTURE_DEADLINE, supergraph, queryGraph }), [], [], new Map(), @@ -104,7 +107,7 @@ describe("advanceSimultaneousPathsWithOperation", () => { const initial = createOptions(supergraph, queryGraph)[0]; // Then picking `t`, which should be just the one option of picking it in S1 at this point. - const allAfterT = advanceSimultaneousPathsWithOperation(supergraph, initial, field(api, "Query.t"), new Map()); + const allAfterT = advanceSimultaneousPathsWithOperation(FAR_FUTURE_DEADLINE, supergraph, initial, field(api, "Query.t"), new Map()); assert(allAfterT, 'Should have advanced correctly'); expect(allAfterT).toHaveLength(1); const afterT = allAfterT[0]; @@ -118,7 +121,7 @@ describe("advanceSimultaneousPathsWithOperation", () => { expect(indirect.paths[0].toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ otherId } ⊢ key()]--> T(S2) (types: [T])`); expect(indirect.paths[1].toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ id } ⊢ key()]--> T(S3) (types: [T])`); - const allForId = advanceSimultaneousPathsWithOperation(supergraph, afterT, field(api, "T.id"), new Map()); + const allForId = advanceSimultaneousPathsWithOperation(FAR_FUTURE_DEADLINE, supergraph, afterT, field(api, "T.id"), new Map()); assert(allForId, 'Should have advanced correctly'); // Here, `id` is a direct path from both of our indirect paths. However, it makes no sense to use the 2nd @@ -156,7 +159,7 @@ describe("advanceSimultaneousPathsWithOperation", () => { const initial = createOptions(supergraph, queryGraph)[0]; // Then picking `t`, which should be just the one option of picking it in S1 at this point. - const allAfterT = advanceSimultaneousPathsWithOperation(supergraph, initial, field(api, "Query.t"), new Map()); + const allAfterT = advanceSimultaneousPathsWithOperation(FAR_FUTURE_DEADLINE, supergraph, initial, field(api, "Query.t"), new Map()); assert(allAfterT, 'Should have advanced correctly'); expect(allAfterT).toHaveLength(1); const afterT = allAfterT[0]; @@ -170,7 +173,7 @@ describe("advanceSimultaneousPathsWithOperation", () => { expect(indirect.paths[0].toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ otherId } ⊢ key()]--> T(S2) (types: [T])`); expect(indirect.paths[1].toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ id1 id2 } ⊢ key()]--> T(S3) (types: [T])`); - const allForId = advanceSimultaneousPathsWithOperation(supergraph, afterT, field(api, "T.id1"), new Map()); + const allForId = advanceSimultaneousPathsWithOperation(FAR_FUTURE_DEADLINE, supergraph, afterT, field(api, "T.id1"), new Map()); assert(allForId, 'Should have advanced correctly'); // Here, `id1` is a direct path from both of our indirect paths. However, it makes no sense to use the 2nd @@ -202,7 +205,7 @@ describe("advanceSimultaneousPathsWithOperation", () => { const initial = createOptions(supergraph, queryGraph)[0]; // Then picking `t`, which should be just the one option of picking it in S1 at this point. - const allAfterT = advanceSimultaneousPathsWithOperation(supergraph, initial, field(api, "Query.t"), new Map()); + const allAfterT = advanceSimultaneousPathsWithOperation(FAR_FUTURE_DEADLINE, supergraph, initial, field(api, "Query.t"), new Map()); assert(allAfterT, 'Should have advanced correctly'); expect(allAfterT).toHaveLength(1); const afterT = allAfterT[0]; diff --git a/query-graphs-js/src/conditionsValidation.ts b/query-graphs-js/src/conditionsValidation.ts index 7763c170c..6f0df7147 100644 --- a/query-graphs-js/src/conditionsValidation.ts +++ b/query-graphs-js/src/conditionsValidation.ts @@ -18,6 +18,8 @@ import { cachingConditionResolver } from "./conditionsCaching"; class ConditionValidationState { constructor( + // deadline by which time we want the computation to abort + readonly deadline: number, // Selection that belongs to the condition we're validating. readonly selection: Selection, // All the possible "simultaneous paths" we could be in the subgraph when we reach this state selection. @@ -28,6 +30,7 @@ class ConditionValidationState { const newOptions: SimultaneousPathsWithLazyIndirectPaths[] = []; for (const paths of this.subgraphOptions) { const pathsOptions = advanceSimultaneousPathsWithOperation( + this.deadline, supergraph, paths, this.selection.element, @@ -50,6 +53,7 @@ class ConditionValidationState { } return this.selection.selectionSet ? this.selection.selectionSet.selections().map( s => new ConditionValidationState( + this.deadline, s, newOptions, ) @@ -69,10 +73,12 @@ class ConditionValidationState { * conditions are satisfied. */ export function simpleValidationConditionResolver({ + deadline, supergraph, queryGraph, withCaching, }: { + deadline: number, supergraph: Schema, queryGraph: QueryGraph, withCaching?: boolean, @@ -92,7 +98,7 @@ export function simpleValidationConditionResolver({ new SimultaneousPathsWithLazyIndirectPaths( [initialPath], context, - simpleValidationConditionResolver({ supergraph, queryGraph, withCaching }), + simpleValidationConditionResolver({ deadline, supergraph, queryGraph, withCaching }), excludedDestinations, excludedConditions, new Map(), @@ -103,6 +109,7 @@ export function simpleValidationConditionResolver({ for (const selection of conditions.selections()) { stack.push( new ConditionValidationState( + deadline, selection, initialOptions, ), diff --git a/query-graphs-js/src/graphPath.ts b/query-graphs-js/src/graphPath.ts index ba87dc723..37ef14588 100644 --- a/query-graphs-js/src/graphPath.ts +++ b/query-graphs-js/src/graphPath.ts @@ -42,6 +42,7 @@ import { Vertex, QueryGraph, Edge, RootVertex, isRootVertex, isFederatedGraphRoo import { DownCast, Transition } from "./transition"; import { PathContext, emptyContext, isPathContext } from "./pathContext"; import { v4 as uuidv4 } from 'uuid'; +import { performance } from 'perf_hooks'; const debug = newDebugLogger('path'); @@ -2210,6 +2211,7 @@ function filterNonCollectingPathsForField( // The lists of options can be empty, which has the special meaning that the operation is guaranteed to have no results (it corresponds to unsatisfiable conditions), // meaning that as far as query planning goes, we can just ignore the operation but otherwise continue. export function advanceSimultaneousPathsWithOperation( + deadline: number, supergraphSchema: Schema, subgraphSimultaneousPaths: SimultaneousPathsWithLazyIndirectPaths, operation: OperationElement, @@ -2228,6 +2230,7 @@ export function advanceSimultaneousPathsWithOperation( if (!shouldReenterSubgraph) { debug.group(() => `Direct options`); const { options: advanceOptions, hasOnlyTypeExplodedResults } = advanceWithOperation( + deadline, supergraphSchema, path, operation, @@ -2275,6 +2278,7 @@ export function advanceSimultaneousPathsWithOperation( for (const pathWithNonCollecting of pathsWithNonCollecting.paths) { debug.group(() => `For indirect path ${pathWithNonCollecting}:`); const { options: pathWithOperation } = advanceWithOperation( + deadline, supergraphSchema, pathWithNonCollecting, operation, @@ -2334,6 +2338,7 @@ export function advanceSimultaneousPathsWithOperation( if (options.length === 0 && shouldReenterSubgraph) { debug.group(() => `Cannot defer (no indirect options); falling back to direct options`); const { options: advanceOptions } = advanceWithOperation( + deadline, supergraphSchema, path, operation, @@ -2601,6 +2606,7 @@ function isProvidedEdge(edge: Edge): boolean { // We also actually need to return a set of options of simultaneous paths. Cause when we type explode, we create simultaneous paths, but // as a field might be resolve by multiple subgraphs, we may have options created. function advanceWithOperation( + deadline: number, supergraphSchema: Schema, path: OpGraphPath, operation: OperationElement, @@ -2613,6 +2619,10 @@ function advanceWithOperation( } { debug.group(() => `Trying to advance ${path} directly with ${operation}`); + if (performance.now() > deadline) { + throw new Error('Query plan took too long to calculate'); + } + const currentType = path.tail.type; if (isFederatedGraphRootType(currentType)) { // We cannot advance any operation from there: we need to take the initial non-collecting edges first. @@ -2725,6 +2735,7 @@ function advanceWithOperation( const castOp = new FragmentElement(currentType, implemType.name); debug.group(() => `Handling implementation ${implemType}`); const implemOptions = advanceSimultaneousPathsWithOperation( + deadline, supergraphSchema, new SimultaneousPathsWithLazyIndirectPaths([path], context, conditionResolver, [], [], overrideConditions), castOp, @@ -2748,6 +2759,7 @@ function advanceWithOperation( for (const optPaths of implemOptions) { debug.group(() => `For ${simultaneousPathsToString(optPaths)}`); const withFieldOptions = advanceSimultaneousPathsWithOperation( + deadline, supergraphSchema, optPaths, operation, @@ -2824,6 +2836,7 @@ function advanceWithOperation( debug.group(() => `Trying ${tName}`); const castOp = new FragmentElement(currentType, tName, operation.appliedDirectives); const implemOptions = advanceSimultaneousPathsWithOperation( + deadline, supergraphSchema, new SimultaneousPathsWithLazyIndirectPaths([path], context, conditionResolver, [], [], overrideConditions), castOp, diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index f85c879df..42c724677 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -457,6 +457,7 @@ class QueryPlanningTraversal { let newOptions: SimultaneousPathsWithLazyIndirectPaths[] = []; for (const option of options) { const followupForOption = advanceSimultaneousPathsWithOperation( + this.deadline, this.parameters.supergraphSchema, option, operation, diff --git a/query-planner-js/src/config.ts b/query-planner-js/src/config.ts index 914a399a0..ae7db744d 100644 --- a/query-planner-js/src/config.ts +++ b/query-planner-js/src/config.ts @@ -120,7 +120,7 @@ export type QueryPlannerConfig = { typeConditionedFetching?: boolean, /** - * The maximum time to wait for query planning to complete before giving up and throwing an exception. Defaults to 2 minutes + * The maximum time (in milliseconds) to wait for query planning to complete before giving up and throwing an exception. Defaults to 2 minutes */ maxQueryPlanningTime?: number, }