Skip to content

Commit

Permalink
chore(cli): print detected cycle if work graph stalls
Browse files Browse the repository at this point in the history
To help with diagnosing #25806, if the work graph can't make any
progress anymore because of a dependency cycle, print the cycle that was
found instead of all the remaining nodes.
  • Loading branch information
rix0rrr committed Jun 2, 2023
1 parent f9c656f commit b614635
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1284,16 +1284,5 @@
},
"internetmonitor": {
"name": "InternetMonitor"
},
"ivsrealtime": {
"prefix": "ivs-realtime",
"name": "IVSRealTime"
},
"vpclattice": {
"prefix": "vpc-lattice",
"name": "VPCLattice"
},
"osis": {
"name": "OSIS"
}
}
29 changes: 27 additions & 2 deletions packages/aws-cdk/lib/util/work-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ export class WorkGraph {
this.readyPool.sort((a, b) => (b.priority ?? 0) - (a.priority ?? 0));

if (this.readyPool.length === 0 && activeCount === 0 && pendingCount > 0) {
throw new Error(`Unable to make progress anymore among: ${this}`);
throw new Error(`Unable to make progress anymore, dependency cycle between remaining artifacts: ${this.findCycle().join(' -> ')}`);
}
}

Expand All @@ -283,6 +283,31 @@ export class WorkGraph {
}
}
}

/**
* Find cycles in a graph
*
* Not the fastest, but effective and should be rare
*/
private findCycle(): string[] {
const self = this;
for (const nodeId of Object.keys(this.nodes)) {
const cycle = recurse(nodeId, [nodeId]);
if (cycle) { return cycle; }
}
return ['No cycle found!'];

function recurse(nodeId: string, path: string[]): string[] | undefined {
for (const dep of self.nodes[nodeId].dependencies ?? []) {
if (dep === path[0]) { return [...path, dep]; }

const cycle = recurse(dep, [...path, dep]);
if (cycle) { return cycle; }
}

return undefined;
}
}
}

export interface WorkGraphActions {
Expand All @@ -297,4 +322,4 @@ function sum(xs: number[]) {
ret += x;
}
return ret;
}
}
7 changes: 5 additions & 2 deletions packages/aws-cdk/test/work-graph.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,13 +328,15 @@ describe('WorkGraph', () => {
toDeploy: createArtifacts([
{ id: 'A', type: 'stack', stackDependencies: ['A'] },
]),
expectedError: 'A -> A',
},
{
scenario: 'A -> B, B -> A',
toDeploy: createArtifacts([
{ id: 'A', type: 'stack', stackDependencies: ['B'] },
{ id: 'B', type: 'stack', stackDependencies: ['A'] },
]),
expectedError: 'A -> B -> A',
},
{
scenario: 'A, B -> C, C -> D, D -> B',
Expand All @@ -344,12 +346,13 @@ describe('WorkGraph', () => {
{ id: 'C', type: 'stack', stackDependencies: ['D'] },
{ id: 'D', type: 'stack', stackDependencies: ['B'] },
]),
expectedError: 'B -> C -> D -> B',
},
])('Failure - Graph Circular Dependencies - $scenario', async ({ toDeploy }) => {
])('Failure - Graph Circular Dependencies - $scenario', async ({ toDeploy, expectedError }) => {
const graph = new WorkGraph();
addTestArtifactsToGraph(toDeploy, graph);

await expect(graph.doParallel(1, callbacks)).rejects.toThrowError(/Unable to make progress anymore/);
await expect(graph.doParallel(1, callbacks)).rejects.toThrowError(new RegExp(`Unable to make progress.*${expectedError}`));
});
});

Expand Down

0 comments on commit b614635

Please sign in to comment.