Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(aws-service-catalog): SIGKILL: 9 CustomCDKBucketDeployment Lambda runs out of memory #29862

Closed
Labels
@aws-cdk/aws-servicecatalog Related to AWS Service Catalog effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@schnipseljagd
Copy link

schnipseljagd commented Apr 17, 2024

Describe the bug

Received response status [FAILED] from custom resource. Message returned: Command '['/opt/awscli/aws', 's3', 'cp', 's3://cdk-accel-assets-XXXX-eu-central-1/d91fe0e02571a1639b83572917ebc528352265bd305ff570f75cae8419dae522.zip', '/tmp/tmpd41vn6fr/contents']' died with <Signals.SIGKILL: 9>. (RequestId: 90349d17-82a5-4ed1-b285-aad7469cb046)

Expected Behavior

I should be able to configure the memory limit for the bucket deployment Lambda used by the ProductStackSynthesizer.

Current Behavior

It is not configurable since it is hard-coded deep in the synthesizer:

?? new BucketDeployment(deploymentScope, deploymentCid, {

Reproduction Steps

You need to try to deploy a big enough CDK Stack using service catalog product stacks.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.134.0 (build 265d769)

Framework Version

No response

Node.js Version

v18.20.0

OS

Linux

Language

TypeScript

Language Version

No response

Other information

No response

@schnipseljagd schnipseljagd added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 17, 2024
@github-actions github-actions bot added the @aws-cdk/aws-servicecatalog Related to AWS Service Catalog label Apr 17, 2024
@schnipseljagd
Copy link
Author

Btw. Increasing the memory limit of the Lambda manually fixes the problem for the time being.

@schnipseljagd schnipseljagd changed the title (aws-service-catalog): memory limit for Buc (aws-service-catalog): SIGKILL: 9 CustomCDKBucketDeployment Lambda runs out of memory Apr 17, 2024
@nmussy
Copy link
Contributor

nmussy commented Apr 17, 2024

I don't think increasing the memory allocated to the Lambda is the correct solution here. It's a waste of resources and would not scale with larger file sizes. The file downloaded from S3 should not be fully stored in memory until its download is completed, it should be progressively saved to disk and assembled.

I don't think this is possible with the AWS CLI, but it might be doable with boto3, see Multipart transfers. If not, it could be done with the SDK, see Upload or download large files to and from Amazon S3 using an AWS SDK

EDIT: boto3 should be good to go, and a lot simpler than re-implementing the same behavior with the SDK, see docs

multipart_threshold – The transfer size threshold for which multipart uploads, downloads, and copies will automatically be triggered.

@pahud
Copy link
Contributor

pahud commented Apr 18, 2024

@nmussy Thank you and I agree with you.

But it makes sense to expose the memoryLimit as well. Feel free and welcome to submit a PR to expose the memoryLimit for this.

Please help us prioritize with 👍 .

@pahud pahud added feature-request A feature should be added or improved. p2 effort/small Small work item – less than a day of effort and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 18, 2024
@nmussy
Copy link
Contributor

nmussy commented Apr 19, 2024

I've opened another issue for my solution, so we can keep track of it after this one is fixed by the memoryLimit prop: #29898

@mergify mergify bot closed this as completed in #30105 May 15, 2024
mergify bot pushed a commit that referenced this issue May 15, 2024
### Issue # (if applicable)

Closes #29862

### Reason for this change

Exposes `BucketDeployment`'s `memoryLimit` prop in the `ProductStack` class. While not an ideal solution, this allows users to handle larger files

### Description of changes

* Added `memoryLimit` to `ProductStackProps` and the internal `ProductStackSynthesizerProps`

### Description of how you validated changes

Updated unit and integ tests

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@aws-cdk-automation
Copy link
Collaborator

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.