Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/beige-suits-occur.md
Original file line number Diff line number Diff line change
@@ -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.
96 changes: 96 additions & 0 deletions query-planner-js/src/__tests__/buildPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10721,3 +10721,99 @@ 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 (10 microseconds)', () => {
const config = {
debug: { maxEvaluatedPlans: undefined },
maxQueryPlanningTime: 0.01,
};
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',
);
});
});
24 changes: 19 additions & 5 deletions query-planner-js/src/buildPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
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');
Expand Down Expand Up @@ -394,6 +395,9 @@
readonly costFunction: CostFunction,
initialContext: PathContext,
typeConditionedFetching: boolean,

// time (calculated relative from performance.now) after which query plan calculation may be aborted
readonly deadline: number,
excludedDestinations: ExcludedDestinations = [],
excludedConditions: ExcludedConditions = [],
) {
Expand Down Expand Up @@ -445,6 +449,9 @@
}

private handleOpenBranch(selection: Selection, options: SimultaneousPathsWithLazyIndirectPaths<RV>[]) {
if (performance.now() > this.deadline) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We ended up adding deadline checks to some other parts in Rust QP (see this commit), can you also add those deadline checks to the corresponding parts in gateway's QP?

throw new Error('Query plan took too long to calculate');

Check warning on line 453 in query-planner-js/src/buildPlan.ts

View check run for this annotation

Codecov / codecov/patch

query-planner-js/src/buildPlan.ts#L453

Added line #L453 was not covered by tests
}
const operation = selection.element;
debug.group(() => `Handling open branch: ${operation}`);
let newOptions: SimultaneousPathsWithLazyIndirectPaths<RV>[] = [];
Expand Down Expand Up @@ -771,6 +778,7 @@
this.costFunction,
context,
this.typeConditionedFetching,
this.deadline,
excludedDestinations,
addConditionExclusion(excludedConditions, edge.conditions),
).findBestPlan();
Expand Down Expand Up @@ -3531,10 +3539,10 @@
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.
Expand All @@ -3554,6 +3562,7 @@
parameters,
0,
hasDefers,
deadline,
);
({ main, deferred } = dependencyGraph.process(processor, operation.rootKind));
primarySelection = dependencyGraph.deferTracking.primarySelection;
Expand Down Expand Up @@ -3675,12 +3684,14 @@
parameters: PlanningParameters<RootVertex>,
startFetchIdGen: number,
hasDefer: boolean,
deadline: number,
): FetchDependencyGraph {
return computeRootParallelBestPlan(
parameters,
parameters.operation.selectionSet,
startFetchIdGen,
hasDefer,
deadline,
)[0];
}

Expand All @@ -3689,6 +3700,7 @@
selection: SelectionSet,
startFetchIdGen: number,
hasDefers: boolean,
deadline: number,
): [FetchDependencyGraph, OpPathTree<RootVertex>, number] {
const planningTraversal = new QueryPlanningTraversal(
parameters,
Expand All @@ -3699,6 +3711,7 @@
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),
Expand Down Expand Up @@ -3726,17 +3739,18 @@
function computeRootSerialDependencyGraph(
parameters: PlanningParameters<RootVertex>,
hasDefers: boolean,
deadline: number,
): FetchDependencyGraph[] {
const { supergraphSchema, federatedQueryGraph, operation, root } = parameters;
const rootType = hasDefers ? supergraphSchema.schemaDefinition.rootType(root.rootKind) : undefined;
// We have to serially compute a plan for each top-level selection.
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);

Check warning on line 3753 in query-planner-js/src/buildPlan.ts

View check run for this annotation

Apollo SecOps / Static App Security Check

rules.providers.gitlab.security.eslint.detect-object-injection

Bracket object notation with user input is present, this might allow an attacker to access all properties of the object and even it's prototype, leading to possible code execution.
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
Expand Down
6 changes: 6 additions & 0 deletions query-planner-js/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ export type QueryPlannerConfig = {
* If you aren't aware of this flag, you probably don't need it.
*/
typeConditionedFetching?: boolean,

/**
* The maximum time to wait for query planning to complete before giving up and throwing an exception. Defaults to 2 minutes
*/
maxQueryPlanningTime?: number,
}

export function enforceQueryPlannerConfigDefaults(
Expand All @@ -128,6 +133,7 @@ export function enforceQueryPlannerConfigDefaults(
reuseQueryFragments: true,
generateQueryFragments: false,
cache: new InMemoryLRUCache<QueryPlan>({maxSize: Math.pow(2, 20) * 50 }),
maxQueryPlanningTime: 2 * 60 * 1000, // two minutes
...config,
incrementalDelivery: {
enableDefer: false,
Expand Down