Skip to content

Commit

Permalink
fix(cli): hotswapping is slow for many resources deployed at once (#1…
Browse files Browse the repository at this point in the history
…9081)

In the `LazyListStackResources` class,
we were incorrectly caching the results of the ListStackResources CloudFormation API call.
This resulted in executing redundant calls,
and very visibly impacted the performance of hotswapping when the Stack included many resources.

In manual testing, this change brought down deployment time of a large Stack from 80 to 7 seconds.

Closes #19021

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
skinny85 committed Feb 24, 2022
1 parent e2f6918 commit 040238e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 12 deletions.
24 changes: 12 additions & 12 deletions packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts
Expand Up @@ -7,30 +7,30 @@ export interface ListStackResources {
}

export class LazyListStackResources implements ListStackResources {
private stackResources: AWS.CloudFormation.StackResourceSummary[] | undefined;
private stackResources: Promise<AWS.CloudFormation.StackResourceSummary[]> | undefined;

constructor(private readonly sdk: ISDK, private readonly stackName: string) {
}

public async listStackResources(): Promise<AWS.CloudFormation.StackResourceSummary[]> {
if (this.stackResources === undefined) {
this.stackResources = await this.getStackResources();
this.stackResources = this.getStackResources(undefined);
}
return this.stackResources;
}

private async getStackResources(): Promise<AWS.CloudFormation.StackResourceSummary[]> {
private async getStackResources(nextToken: string | undefined): Promise<AWS.CloudFormation.StackResourceSummary[]> {
const ret = new Array<AWS.CloudFormation.StackResourceSummary>();
let nextToken: string | undefined;
do {
const stackResourcesResponse = await this.sdk.cloudFormation().listStackResources({
StackName: this.stackName,
NextToken: nextToken,
}).promise();
return this.sdk.cloudFormation().listStackResources({
StackName: this.stackName,
NextToken: nextToken,
}).promise().then(async stackResourcesResponse => {
ret.push(...(stackResourcesResponse.StackResourceSummaries ?? []));
nextToken = stackResourcesResponse.NextToken;
} while (nextToken);
return ret;
if (stackResourcesResponse.NextToken) {
ret.push(...await this.getStackResources(stackResourcesResponse.NextToken));
}
return ret;
});
}
}

Expand Down
29 changes: 29 additions & 0 deletions packages/aws-cdk/test/api/lazy-list-stack-resources.test.ts
@@ -0,0 +1,29 @@
import * as AWS from 'aws-sdk';
import { LazyListStackResources } from '../../lib/api/evaluate-cloudformation-template';
import { MockSdk } from '../util/mock-sdk';

describe('Lazy ListStackResources', () => {
test('correctly caches calls to the CloudFormation API', async () => {
// GIVEN
const listStackResMock: jest.Mock<AWS.CloudFormation.ListStackResourcesOutput, AWS.CloudFormation.ListStackResourcesInput[]> = jest.fn();
const mockSdk = new MockSdk();
mockSdk.stubCloudFormation({
listStackResources: listStackResMock,
});
listStackResMock.mockReturnValue({
StackResourceSummaries: [],
NextToken: undefined,
});
const res = new LazyListStackResources(mockSdk, 'StackName');

// WHEN
void res.listStackResources();
void res.listStackResources();
void res.listStackResources();
const result = await res.listStackResources();

// THEN
expect(result.length).toBe(0);
expect(listStackResMock).toHaveBeenCalledTimes(1);
});
});

0 comments on commit 040238e

Please sign in to comment.