Skip to content

Commit 7ac8345

Browse files
author
Sylvain Lebresne
authored
Query planning performance improvements (#2610)
Implements a few performance improvement for query plan computations: 1. profiling of some slow query planning shows `FetchGroup.isUseless` as one of the hot path. This commit caches the result of this method for a group, and only invalid that cache when we know the result may needs to be recomputed. On the planning of some queries, this is shown to provide a 15% improvement to query planning time. 2. when a type has multiple keys, the query planning was sometimes considering an option where some key `x` was used to get field `y` but then key `y` was used to get that same `y` field from another subgraph. This is obviously not very useful, and we know we can ignore those paths as the 1st part of those path already does what we want. But considering those (useless) options, while harmless for correction, was in some case drastically increasing the number of plans that were evaluated, leading to long query planning times.
1 parent 529ea34 commit 7ac8345

11 files changed

Lines changed: 537 additions & 142 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@apollo/query-planner": patch
3+
"@apollo/query-graphs": patch
4+
"@apollo/federation-internals": patch
5+
---
6+
7+
Improves query planning time in some situations where entities use multiple keys.
8+

composition-js/src/validate.ts

Lines changed: 11 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -43,25 +43,15 @@ import {
4343
RootPath,
4444
advancePathWithTransition,
4545
Transition,
46-
OpGraphPath,
47-
advanceSimultaneousPathsWithOperation,
48-
ExcludedEdges,
4946
QueryGraphState,
50-
ExcludedConditions,
5147
Unadvanceables,
5248
isUnadvanceable,
5349
Unadvanceable,
5450
noConditionsResolution,
55-
ConditionResolution,
56-
unsatisfiedConditionsResolution,
57-
ConditionResolver,
58-
cachingConditionResolver,
59-
PathContext,
60-
addConditionExclusion,
61-
SimultaneousPathsWithLazyIndirectPaths,
62-
advanceOptionsToString,
6351
TransitionPathWithLazyIndirectPaths,
6452
RootVertex,
53+
simpleValidationConditionResolver,
54+
ConditionResolver,
6555
} from "@apollo/query-graphs";
6656
import { CompositionHint, HINTS } from "./hints";
6757
import { ASTNode, GraphQLError, print } from "graphql";
@@ -330,7 +320,7 @@ export function computeSubgraphPaths(
330320
} {
331321
try {
332322
assert(!supergraphPath.hasAnyEdgeConditions(), () => `A supergraph path should not have edge condition paths (as supergraph edges should not have conditions): ${supergraphPath}`);
333-
const conditionResolver = new ConditionValidationResolver(supergraphSchema, subgraphs);
323+
const conditionResolver = simpleValidationConditionResolver({ supergraph: supergraphSchema, queryGraph: subgraphs, withCaching: true });
334324
const initialState = ValidationState.initial({supergraphAPI: supergraphPath.graph, kind: supergraphPath.root.rootKind, subgraphs, conditionResolver});
335325
const context = new ValidationContext(supergraphSchema);
336326
let state = initialState;
@@ -431,11 +421,11 @@ export class ValidationState {
431421
supergraphAPI: QueryGraph,
432422
kind: SchemaRootKind,
433423
subgraphs: QueryGraph,
434-
conditionResolver: ConditionValidationResolver,
424+
conditionResolver: ConditionResolver,
435425
}) {
436426
return new ValidationState(
437427
GraphPath.fromGraphRoot(supergraphAPI, kind)!,
438-
initialSubgraphPaths(kind, subgraphs).map((p) => TransitionPathWithLazyIndirectPaths.initial(p, conditionResolver.resolver)),
428+
initialSubgraphPaths(kind, subgraphs).map((p) => TransitionPathWithLazyIndirectPaths.initial(p, conditionResolver)),
439429
);
440430
}
441431

@@ -586,7 +576,7 @@ function isSupersetOrEqual(maybeSuperset: string[], other: string[]): boolean {
586576
}
587577

588578
class ValidationTraversal {
589-
private readonly conditionResolver: ConditionValidationResolver;
579+
private readonly conditionResolver: ConditionResolver;
590580
// The stack contains all states that aren't terminal.
591581
private readonly stack: ValidationState[] = [];
592582

@@ -604,7 +594,11 @@ class ValidationTraversal {
604594
supergraphAPI: QueryGraph,
605595
subgraphs: QueryGraph
606596
) {
607-
this.conditionResolver = new ConditionValidationResolver(supergraphSchema, subgraphs);
597+
this.conditionResolver = simpleValidationConditionResolver({
598+
supergraph: supergraphSchema,
599+
queryGraph: subgraphs,
600+
withCaching: true,
601+
});
608602
supergraphAPI.rootKinds().forEach((kind) => this.stack.push(ValidationState.initial({
609603
supergraphAPI,
610604
kind,
@@ -680,87 +674,3 @@ class ValidationTraversal {
680674
debug.groupEnd();
681675
}
682676
}
683-
684-
class ConditionValidationState {
685-
constructor(
686-
// Selection that belongs to the condition we're validating.
687-
readonly selection: Selection,
688-
// All the possible "simultaneous paths" we could be in the subgraph when we reach this state selection.
689-
readonly subgraphOptions: SimultaneousPathsWithLazyIndirectPaths[]
690-
) {}
691-
692-
toString(): string {
693-
return `${this.selection} <=> ${advanceOptionsToString(this.subgraphOptions)}`;
694-
}
695-
}
696-
697-
class ConditionValidationResolver {
698-
readonly resolver: ConditionResolver;
699-
700-
constructor(
701-
private readonly supergraphSchema: Schema,
702-
private readonly federatedQueryGraph: QueryGraph
703-
) {
704-
this.resolver = cachingConditionResolver(
705-
federatedQueryGraph,
706-
(
707-
edge: Edge,
708-
context: PathContext,
709-
excludedEdges: ExcludedEdges,
710-
excludedConditions: ExcludedConditions
711-
) => this.validateConditions(edge, context, excludedEdges, excludedConditions)
712-
);
713-
}
714-
715-
private validateConditions(
716-
edge: Edge,
717-
context: PathContext,
718-
excludedEdges: ExcludedEdges,
719-
excludedConditions: ExcludedConditions
720-
): ConditionResolution {
721-
const conditions = edge.conditions!;
722-
excludedConditions = addConditionExclusion(excludedConditions, conditions);
723-
724-
const initialPath: OpGraphPath = GraphPath.create(this.federatedQueryGraph, edge.head);
725-
const initialOptions = [new SimultaneousPathsWithLazyIndirectPaths([initialPath], context, this.resolver, excludedEdges, excludedConditions)];
726-
727-
const stack: ConditionValidationState[] = [];
728-
for (const selection of conditions.selections()) {
729-
stack.push(new ConditionValidationState(selection, initialOptions));
730-
}
731-
732-
while (stack.length > 0) {
733-
const state = stack.pop()!;
734-
const newStates = this.advanceState(state);
735-
if (newStates === null) {
736-
return unsatisfiedConditionsResolution;
737-
}
738-
newStates.forEach(s => stack.push(s));
739-
}
740-
// If we exhaust the stack, it means we've been able to find "some" path for every possible selection in the condition, so the
741-
// condition is validated. Note that we use a cost of 1 for all conditions as we don't care about efficiency.
742-
return { satisfied: true, cost: 1 };
743-
}
744-
745-
private advanceState(state: ConditionValidationState): ConditionValidationState[] | null {
746-
let newOptions: SimultaneousPathsWithLazyIndirectPaths[] = [];
747-
for (const paths of state.subgraphOptions) {
748-
const pathsOptions = advanceSimultaneousPathsWithOperation(
749-
this.supergraphSchema,
750-
paths,
751-
state.selection.element,
752-
);
753-
if (!pathsOptions) {
754-
continue;
755-
}
756-
newOptions = newOptions.concat(pathsOptions);
757-
}
758-
759-
// If we got no options, it means that particular selection of the conditions cannot be satisfied, so the
760-
// overall condition cannot.
761-
if (newOptions.length === 0) {
762-
return null;
763-
}
764-
return state.selection.selectionSet ? state.selection.selectionSet.selections().map(s => new ConditionValidationState(s, newOptions)) : [];
765-
}
766-
}

internals-js/src/operations.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1792,6 +1792,11 @@ export class SelectionSet {
17921792
: ContainsResult.STRICTLY_CONTAINED;
17931793
}
17941794

1795+
containsTopLevelField(field: Field): boolean {
1796+
const selection = this._keyedSelections.get(field.key());
1797+
return !!selection && selection.element.equals(field);
1798+
}
1799+
17951800
/**
17961801
* Returns a selection set that correspond to this selection set but where any of the selections in the
17971802
* provided selection set have been remove.
Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
import {
2+
Field,
3+
FieldDefinition,
4+
Schema,
5+
assert,
6+
buildSupergraphSchema,
7+
} from "@apollo/federation-internals";
8+
import {
9+
GraphPath,
10+
OpGraphPath,
11+
SimultaneousPathsWithLazyIndirectPaths,
12+
advanceSimultaneousPathsWithOperation,
13+
createInitialOptions
14+
} from "../graphPath";
15+
import { QueryGraph, Vertex, buildFederatedQueryGraph } from "../querygraph";
16+
import { emptyContext } from "../pathContext";
17+
import { simpleValidationConditionResolver } from "../conditionsValidation";
18+
19+
function parseSupergraph(subgraphs: number, schema: string): { supergraph: Schema, api: Schema, queryGraph: QueryGraph } {
20+
assert(subgraphs >= 1, 'Should have at least 1 subgraph');
21+
const header = `
22+
schema
23+
@link(url: "https://specs.apollo.dev/link/v1.0")
24+
@link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION)
25+
{
26+
query: Query
27+
}
28+
29+
directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE
30+
directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION
31+
directive @join__graph(name: String!, url: String!) on ENUM_VALUE
32+
directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE
33+
directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR
34+
directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION
35+
directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
36+
37+
scalar join__FieldSet
38+
39+
scalar link__Import
40+
41+
enum link__Purpose {
42+
SECURITY
43+
EXECUTION
44+
}
45+
46+
enum join__Graph {
47+
${[...Array(subgraphs).keys()].map((n) => `S${n+1} @join__graph(name: "S${n+1}", url: "https://S${n+1}")`).join('\n')}
48+
}
49+
50+
`;
51+
52+
try {
53+
const supergraph = buildSupergraphSchema(header + schema)[0];
54+
return {
55+
supergraph,
56+
api: supergraph.toAPISchema(),
57+
queryGraph: buildFederatedQueryGraph(supergraph, true),
58+
};
59+
} catch (e) {
60+
throw new Error('Error parsing supergraph schema:\n' + e.toString());
61+
}
62+
}
63+
64+
function createOptions(supergraph: Schema, queryGraph: QueryGraph): SimultaneousPathsWithLazyIndirectPaths<Vertex>[] {
65+
// We know we only use `Query` in the supergraph, so there is only that as root.
66+
const root = queryGraph.roots()[0];
67+
const initialPath: OpGraphPath<Vertex> = GraphPath.create(queryGraph, root);
68+
return createInitialOptions(
69+
initialPath,
70+
emptyContext,
71+
simpleValidationConditionResolver({ supergraph, queryGraph }),
72+
[],
73+
[],
74+
);
75+
}
76+
77+
function field(schema: Schema, coordinate: string): Field {
78+
const def = schema.elementByCoordinate(coordinate) as FieldDefinition<any>;
79+
return new Field(def);
80+
}
81+
82+
describe("advanceSimultaneousPathsWithOperation", () => {
83+
test("do not use key `x` to fetch `x`", () => {
84+
const { supergraph, api, queryGraph } = parseSupergraph(3, `
85+
type Query
86+
@join__type(graph: S1)
87+
{
88+
t: T @join__field(graph: S1)
89+
}
90+
91+
type T
92+
@join__type(graph: S1)
93+
@join__type(graph: S2, key: "otherId")
94+
@join__type(graph: S2, key: "id")
95+
@join__type(graph: S3, key: "id")
96+
{
97+
otherId: ID! @join__field(graph: S1) @join__field(graph: S2)
98+
id: ID! @join__field(graph: S2) @join__field(graph: S3)
99+
}
100+
`);
101+
102+
// Picking the first initial, the one going to S1
103+
const initial = createOptions(supergraph, queryGraph)[0];
104+
105+
// Then picking `t`, which should be just the one option of picking it in S1 at this point.
106+
const allAfterT = advanceSimultaneousPathsWithOperation(supergraph, initial, field(api, "Query.t"));
107+
assert(allAfterT, 'Should have advanced correctly');
108+
expect(allAfterT).toHaveLength(1);
109+
const afterT = allAfterT[0];
110+
expect(afterT.toString()).toBe(`Query(S1) --[t]--> T(S1) (types: [T])`);
111+
112+
// Checking that, at this point, we technically have 2 options:
113+
// 1. we can go to S2 using `otherId`.
114+
// 2. we can go to S3 using `id`, assuming we first get `id` from S2 (using `otherId`).
115+
const indirect = afterT.indirectOptions(afterT.context, 0);
116+
expect(indirect.paths).toHaveLength(2);
117+
expect(indirect.paths[0].toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ otherId } ⊢ key()]--> T(S2) (types: [T])`);
118+
expect(indirect.paths[1].toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ id } ⊢ key()]--> T(S3) (types: [T])`);
119+
120+
const allForId = advanceSimultaneousPathsWithOperation(supergraph, afterT, field(api, "T.id"));
121+
assert(allForId, 'Should have advanced correctly');
122+
123+
// Here, `id` is a direct path from both of our indirect paths. However, it makes no sense to use the 2nd
124+
// indirect path above, since the condition to get to `S3` was `id`, and this means another indirect path
125+
// is able to get to `id` more directly (the first one in this case).
126+
// So ultimately, we should only keep the 1st option.
127+
expect(allForId).toHaveLength(1);
128+
const forId = allForId[0];
129+
expect(forId.toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ otherId } ⊢ key()]--> T(S2) --[id]--> ID(S2)`);
130+
});
131+
132+
test("do not use key containing `x` to fetch `x`", () => {
133+
// Similar to the previous test, but the key used is not exactly the fetch field, it only contains
134+
// it (but the optimisation should still work).
135+
const { supergraph, api, queryGraph } = parseSupergraph(3, `
136+
type Query
137+
@join__type(graph: S1)
138+
{
139+
t: T @join__field(graph: S1)
140+
}
141+
142+
type T
143+
@join__type(graph: S1)
144+
@join__type(graph: S2, key: "otherId")
145+
@join__type(graph: S2, key: "id1 id2")
146+
@join__type(graph: S3, key: "id1 id2")
147+
{
148+
otherId: ID! @join__field(graph: S1) @join__field(graph: S2)
149+
id1: ID! @join__field(graph: S2) @join__field(graph: S3)
150+
id2: ID! @join__field(graph: S2) @join__field(graph: S3)
151+
}
152+
`);
153+
154+
// Picking the first initial, the one going to S1
155+
const initial = createOptions(supergraph, queryGraph)[0];
156+
157+
// Then picking `t`, which should be just the one option of picking it in S1 at this point.
158+
const allAfterT = advanceSimultaneousPathsWithOperation(supergraph, initial, field(api, "Query.t"));
159+
assert(allAfterT, 'Should have advanced correctly');
160+
expect(allAfterT).toHaveLength(1);
161+
const afterT = allAfterT[0];
162+
expect(afterT.toString()).toBe(`Query(S1) --[t]--> T(S1) (types: [T])`);
163+
164+
// Checking that, at this point, we technically have 2 options:
165+
// 1. we can go to S2 using `otherId`.
166+
// 2. we can go to S3 using `id1 id2`, assuming we first get `id1 id2` from S2 (using `otherId`).
167+
const indirect = afterT.indirectOptions(afterT.context, 0);
168+
expect(indirect.paths).toHaveLength(2);
169+
expect(indirect.paths[0].toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ otherId } ⊢ key()]--> T(S2) (types: [T])`);
170+
expect(indirect.paths[1].toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ id1 id2 } ⊢ key()]--> T(S3) (types: [T])`);
171+
172+
const allForId = advanceSimultaneousPathsWithOperation(supergraph, afterT, field(api, "T.id1"));
173+
assert(allForId, 'Should have advanced correctly');
174+
175+
// Here, `id1` is a direct path from both of our indirect paths. However, it makes no sense to use the 2nd
176+
// indirect path above, since the condition to get to `S3` was `id1 id2`, which includes `id1`.
177+
expect(allForId).toHaveLength(1);
178+
const forId = allForId[0];
179+
expect(forId.toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ otherId } ⊢ key()]--> T(S2) --[id1]--> ID(S2)`);
180+
});
181+
182+
test("avoids indirect path that needs a key to the same subgraph to validate its condition", () => {
183+
const { supergraph, api, queryGraph } = parseSupergraph(2, `
184+
type Query
185+
@join__type(graph: S1)
186+
{
187+
t: T @join__field(graph: S1)
188+
}
189+
190+
type T
191+
@join__type(graph: S1)
192+
@join__type(graph: S2, key: "id1")
193+
@join__type(graph: S2, key: "id2")
194+
{
195+
id1: ID! @join__field(graph: S2)
196+
id2: ID! @join__field(graph: S1) @join__field(graph: S2)
197+
}
198+
`);
199+
200+
// Picking the first initial, the one going to S1
201+
const initial = createOptions(supergraph, queryGraph)[0];
202+
203+
// Then picking `t`, which should be just the one option of picking it in S1 at this point.
204+
const allAfterT = advanceSimultaneousPathsWithOperation(supergraph, initial, field(api, "Query.t"));
205+
assert(allAfterT, 'Should have advanced correctly');
206+
expect(allAfterT).toHaveLength(1);
207+
const afterT = allAfterT[0];
208+
expect(afterT.toString()).toBe(`Query(S1) --[t]--> T(S1) (types: [T])`);
209+
210+
// Technically, the `id1` key could be used to go to S2 by first getting `id1` from S2 using `id2`, but
211+
// that's obviously unecessary to consider since we can just use `id2` to go to S2 in the first place.
212+
const indirect = afterT.indirectOptions(afterT.context, 0);
213+
expect(indirect.paths).toHaveLength(1);
214+
expect(indirect.paths[0].toString()).toBe(`Query(S1) --[t]--> T(S1) --[{ id2 } ⊢ key()]--> T(S2) (types: [T])`);
215+
});
216+
});

0 commit comments

Comments
 (0)