Skip to content

Commit

Permalink
fix(core): many nested stacks make NodeJS run out of memory (#11250)
Browse files Browse the repository at this point in the history
Because of a confluence of factors (but principally the poor design
behind how cross-stack references are detected and rendered), having
a lot of nested stacks leads to a wasteful amount of work being done
and the NodeJS program running out of memory.

An internal project has 26 nested stacks, and was taking `2m20` to
synth and ran out of the default NodeJS heap (having to increase it to
`8G`).

The reason is because the entire construct tree is being resynthesized
for every nested stack, and because this particular application was
using CloudWatch Dashboards, a lot of temporary tokens were being
created (and retained in the global encoded token map) on each
of the 26 synthesis operations.

With the current patch applied, that same project synthesizes in `51s`
and takes `~1G` of memory (this can be brought down further to about 60%
of this number by caching of `Lazy`s, but that change will be introduced
in a different PR).

The problem
-----------

(Legacy) assets work by exploiting cross-stack
references: they add `CfnParameter`s at the top-level, and then
reference them inside a nested template. Cross-stack reference detection
picks this up and adds `CfnParameters` and `AWS::CloudFormation::Stack`
parameters to all stacks on the root path to "transport" the values to
the right nested stack.

To define a nested stack asset, we need to know the template hash, so we
must have already resolved stack references to their final form.
However, the process of defining the nested stack assets may add new
references (as mentioned before) which need to be resolved, leading to
our existing implementation of calling `resolveReferences()` once for
every nested stack in the application.

Calling `resolveReferences()` leads to all Tokens being resolved. It
happens that the Tokens are `Lazy`s that resolve to other `Lazy`s,
perhaps indirectly in the following form:

```ts
Lazy.stringValue({ produce: () => someFunction() })

function someFunction() {
  return Lazy.stringValue({ produce: () => /* something else */ });
}
```

For every resolution for every nested stack, the *inner* `Lazy` would be
reconstructed anew, and subsequently registered in the global token
map (along with its stack trace). A wasteful amount of temporary
Token objects are created and infinitely retained in this process.

The fix
-------

We resolve references twice, instead of once per nested stack.
Once at the start, to resolve all cross-stack references put there
by the user to their final form. Then we add nested stack assets
in the right order, and then we finally do one more "resolve
references" to make sure the stack asset parameters are forwarded
to the right nested stacks.

The nested stack asset hash will now be calculated over a template
that is not yet the final template, but the hash still includes the
user-mutable parts and will change if the user introduces changes
(compare it to a `sourceHash` we use for other assets).


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Nov 3, 2020
1 parent db205da commit c124886
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/// !cdk-integ pragma:ignore-assets
import * as sns from '@aws-cdk/aws-sns';
import { App, Fn, Stack } from '@aws-cdk/core';
import { NestedStack } from '../lib';
Expand Down Expand Up @@ -28,4 +29,4 @@ app.synth();
// Stack1-NestedUnderStack1NestedStackNestedUnderStack1NestedStackResourceF616305B-EM64TEGA04J9-TopicInNestedUnderStack115E329C4-HEO7NLYC1AFL
function shortName(topicName: string) {
return Fn.select(1, Fn.split('-', topicName));
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/// !cdk-integ pragma:ignore-assets
import * as sns from '@aws-cdk/aws-sns';
import { App, Construct, Stack } from '@aws-cdk/core';
import * as cfn from '../lib';
Expand Down
41 changes: 37 additions & 4 deletions packages/@aws-cdk/aws-cloudformation/test/test.nested-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as path from 'path';
import { expect, haveResource, matchTemplate, SynthUtils } from '@aws-cdk/assert';
import * as s3_assets from '@aws-cdk/aws-s3-assets';
import * as sns from '@aws-cdk/aws-sns';
import { App, CfnParameter, CfnResource, Construct, ContextProvider, Stack } from '@aws-cdk/core';
import { App, CfnParameter, CfnResource, Construct, ContextProvider, LegacyStackSynthesizer, Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { NestedStack } from '../lib/nested-stack';

Expand Down Expand Up @@ -714,6 +714,8 @@ export = {
},
});

const middleStackHash = 'b2670b4c0c3fdf1d8fd9b9272bb8bf8173d18c0f67a888ba165cc569a248a84f';

// nested1 wires the nested2 template through parameters, so we expect those
expect(nested1).to(haveResource('Resource::1'));
const nested2Template = SynthUtils.toCloudFormation(nested1);
Expand All @@ -729,9 +731,9 @@ export = {
AssetParameters8169c6f8aaeaf5e2e8620f5f895ffe2099202ccb4b6889df48fe0967a894235cS3BucketDE3B88D6: { Type: 'String', Description: 'S3 bucket for asset "8169c6f8aaeaf5e2e8620f5f895ffe2099202ccb4b6889df48fe0967a894235c"' },
AssetParameters8169c6f8aaeaf5e2e8620f5f895ffe2099202ccb4b6889df48fe0967a894235cS3VersionKey3A62EFEA: { Type: 'String', Description: 'S3 key for asset version "8169c6f8aaeaf5e2e8620f5f895ffe2099202ccb4b6889df48fe0967a894235c"' },
AssetParameters8169c6f8aaeaf5e2e8620f5f895ffe2099202ccb4b6889df48fe0967a894235cArtifactHash7DC546E0: { Type: 'String', Description: 'Artifact hash for asset "8169c6f8aaeaf5e2e8620f5f895ffe2099202ccb4b6889df48fe0967a894235c"' },
AssetParameters8b50795a950cca6b01352f162c45d9d274dee6bc409f2f2b2ed029ad6828b3bfS3Bucket76ACFB38: { Type: 'String', Description: 'S3 bucket for asset "8b50795a950cca6b01352f162c45d9d274dee6bc409f2f2b2ed029ad6828b3bf"' },
AssetParameters8b50795a950cca6b01352f162c45d9d274dee6bc409f2f2b2ed029ad6828b3bfS3VersionKey04162EF1: { Type: 'String', Description: 'S3 key for asset version "8b50795a950cca6b01352f162c45d9d274dee6bc409f2f2b2ed029ad6828b3bf"' },
AssetParameters8b50795a950cca6b01352f162c45d9d274dee6bc409f2f2b2ed029ad6828b3bfArtifactHashF227ADD3: { Type: 'String', Description: 'Artifact hash for asset "8b50795a950cca6b01352f162c45d9d274dee6bc409f2f2b2ed029ad6828b3bf"' },
[`AssetParameters${middleStackHash}S3Bucket3DB431CB`]: { Type: 'String', Description: `S3 bucket for asset "${middleStackHash}"` },
[`AssetParameters${middleStackHash}S3VersionKeyBFFDABE9`]: { Type: 'String', Description: `S3 key for asset version "${middleStackHash}"` },
[`AssetParameters${middleStackHash}ArtifactHash8EA52875`]: { Type: 'String', Description: `Artifact hash for asset "${middleStackHash}"` },
});

// proxy asset params to nested stack
Expand Down Expand Up @@ -935,6 +937,37 @@ export = {
test.done();
},

'3-level stacks: legacy synthesizer parameters are added to the middle-level stack'(test: Test) {
// GIVEN
const app = new App();
const top = new Stack(app, 'stack', {
synthesizer: new LegacyStackSynthesizer(),
});
const middle = new NestedStack(top, 'nested1');
const bottom = new NestedStack(middle, 'nested2');

// WHEN
new CfnResource(bottom, 'Something', {
type: 'BottomLevel',
});

// THEN
const asm = app.synth();
const middleTemplate = JSON.parse(fs.readFileSync(path.join(asm.directory, middle.templateFile), { encoding: 'utf-8' }));

const hash = 'bc3c51e4d3545ee0a0069401e5a32c37b66d044b983f12de416ba1576ecaf0a4';
test.deepEqual(middleTemplate.Parameters ?? {}, {
[`referencetostackAssetParameters${hash}S3BucketD7C30435Ref`]: {
Type: 'String',
},
[`referencetostackAssetParameters${hash}S3VersionKeyB667DBE1Ref`]: {
Type: 'String',
},
});

test.done();
},

'references to a resource from a deeply nested stack'(test: Test) {
// GIVEN
const app = new App();
Expand Down
34 changes: 20 additions & 14 deletions packages/@aws-cdk/cloudformation-include/test/nested-stacks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,9 @@ describe('CDK Include for nested stacks', () => {
let child: inc.IncludedNestedStack;
let grandChild: inc.IncludedNestedStack;

let hash1: string;
let hash2: string;

let parentBucketParam: string;
let parentKeyParam: string;
let grandChildBucketParam: string;
Expand Down Expand Up @@ -471,41 +474,44 @@ describe('CDK Include for nested stacks', () => {
child = parentTemplate.getNestedStack('ChildStack');
grandChild = child.includedTemplate.getNestedStack('GrandChildStack');

parentBucketParam = 'AssetParameters5dc7d4a99cfe2979687dc74f2db9fd75f253b5505a1912b5ceecf70c9aefba50S3BucketEAA24F0C';
parentKeyParam = 'AssetParameters5dc7d4a99cfe2979687dc74f2db9fd75f253b5505a1912b5ceecf70c9aefba50S3VersionKey1194CAB2';
grandChildBucketParam = 'referencetoAssetParameters5dc7d4a99cfe2979687dc74f2db9fd75f253b5505a1912b5ceecf70c9aefba50S3BucketEAA24F0CRef';
grandChildKeyParam = 'referencetoAssetParameters5dc7d4a99cfe2979687dc74f2db9fd75f253b5505a1912b5ceecf70c9aefba50S3VersionKey1194CAB2Ref';
hash1 = '5dc7d4a99cfe2979687dc74f2db9fd75f253b5505a1912b5ceecf70c9aefba50';
hash2 = '7775730164edb5faae717ac1d2e90d9c0d0fdbeafe48763e5c1b7fb5e39e00a5';

parentBucketParam = `AssetParameters${hash1}S3BucketEAA24F0C`;
parentKeyParam = `AssetParameters${hash1}S3VersionKey1194CAB2`;
grandChildBucketParam = `referencetoAssetParameters${hash1}S3BucketEAA24F0CRef`;
grandChildKeyParam = `referencetoAssetParameters${hash1}S3VersionKey1194CAB2Ref`;

childBucketParam = 'AssetParameters891fd3ec75dc881b0fe40dc9fd1b433672637585c015265a5f0dab6bf79818d5S3Bucket23278F13';
childKeyParam = 'AssetParameters891fd3ec75dc881b0fe40dc9fd1b433672637585c015265a5f0dab6bf79818d5S3VersionKey7316205A';
childBucketParam = `AssetParameters${hash2}S3BucketDEB194C6`;
childKeyParam = `AssetParameters${hash2}S3VersionKey8B342ED1`;
});

test('correctly creates parameters in the parent stack, and passes them to the child stack', () => {
expect(assetStack).toMatchTemplate({
"Parameters": {
[parentBucketParam]: {
"Type": "String",
"Description": "S3 bucket for asset \"5dc7d4a99cfe2979687dc74f2db9fd75f253b5505a1912b5ceecf70c9aefba50\"",
"Description": `S3 bucket for asset \"${hash1}\"`,
},
[parentKeyParam]: {
"Type": "String",
"Description": "S3 key for asset version \"5dc7d4a99cfe2979687dc74f2db9fd75f253b5505a1912b5ceecf70c9aefba50\"",
"Description": `S3 key for asset version \"${hash1}\"`,
},
"AssetParameters5dc7d4a99cfe2979687dc74f2db9fd75f253b5505a1912b5ceecf70c9aefba50ArtifactHash9C417847": {
[`AssetParameters${hash1}ArtifactHash9C417847`]: {
"Type": "String",
"Description": "Artifact hash for asset \"5dc7d4a99cfe2979687dc74f2db9fd75f253b5505a1912b5ceecf70c9aefba50\"",
"Description": `Artifact hash for asset \"${hash1}\"`,
},
[childBucketParam]: {
"Type": "String",
"Description": "S3 bucket for asset \"891fd3ec75dc881b0fe40dc9fd1b433672637585c015265a5f0dab6bf79818d5\"",
"Description": `S3 bucket for asset \"${hash2}\"`,
},
[childKeyParam]: {
"Type": "String",
"Description": "S3 key for asset version \"891fd3ec75dc881b0fe40dc9fd1b433672637585c015265a5f0dab6bf79818d5\"",
"Description": `S3 key for asset version \"${hash2}\"`,
},
"AssetParameters891fd3ec75dc881b0fe40dc9fd1b433672637585c015265a5f0dab6bf79818d5ArtifactHashA1DE5198": {
[`AssetParameters${hash2}ArtifactHashAA82D4CC`]: {
"Type": "String",
"Description": "Artifact hash for asset \"891fd3ec75dc881b0fe40dc9fd1b433672637585c015265a5f0dab6bf79818d5\"",
"Description": `Artifact hash for asset \"${hash2}\"`,
},
},
"Resources": {
Expand Down
27 changes: 20 additions & 7 deletions packages/@aws-cdk/core/lib/private/prepare-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,32 @@ export function prepareApp(root: IConstruct) {
}
}

resolveReferences(root);

// depth-first (children first) queue of nested stacks. We will pop a stack
// from the head of this queue to prepare its template asset.
//
// Depth-first since the a nested stack's template hash will be reflected in
// its parent's template, which then changes the parent's hash, etc.
const queue = findAllNestedStacks(root);

while (true) {
resolveReferences(root);

const nested = queue.shift();
if (!nested) {
break;
if (queue.length > 0) {
while (queue.length > 0) {
const nested = queue.shift()!;
defineNestedStackAsset(nested);
}

defineNestedStackAsset(nested);
// ▷[ Given the legacy synthesizer and a 3-or-deeper nesting of nested stacks ]
//
// Adding nested stack assets may haved added CfnParameters to the top-level
// stack which are referenced in a deeper-level stack. The values of these
// parameters need to be carried through to the right location via Nested
// Stack parameters, which `resolveReferences()` will do.
//
// Yes, this may add `Parameter` elements to a template whose hash has
// already been calculated, but the invariant that if the functional part
// of the template changes its hash will change is still upheld.
resolveReferences(root);
}
}

Expand Down

0 comments on commit c124886

Please sign in to comment.