Skip to content

Commit

Permalink
fix: reduce memory pressure while uploading files (#13718)
Browse files Browse the repository at this point in the history
* fix: reduce memory pressure while uploading files

* fix: reduce memory pressure while uploading files

* fix: undo that

* fix: fix that

* chore: comments
  • Loading branch information
sobolk authored Apr 16, 2024
1 parent 044156c commit 2691a0a
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 9 deletions.
2 changes: 1 addition & 1 deletion packages/amplify-provider-awscloudformation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"@types/deep-diff": "^1.0.0",
"@types/folder-hash": "^4.0.1",
"@types/lodash.throttle": "^4.1.6",
"@types/node": "^12.12.6",
"@types/node": "^18.16.0",
"@types/uuid": "^8.0.0",
"jest": "^29.5.0",
"typescript": "^4.9.5"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import { $TSAny, $TSContext, AmplifyError, AmplifyFault, stateManager } from '@a

import _ from 'lodash';

import fs from 'fs-extra';
import ora from 'ora';
import { ListObjectVersionsOutput, ListObjectVersionsRequest, ObjectIdentifier } from 'aws-sdk/clients/s3';
import { pagedAWSCall } from './paged-call';
import { loadConfiguration } from '../configuration-manager';
import aws from './aws';

const providerName = require('../constants').ProviderName;
const consumers = require('stream/consumers');

const { fileLogger } = require('../utils/aws-logger');

Expand Down Expand Up @@ -126,6 +128,16 @@ export class S3 {
spinner.start('Uploading files.');
}
logger('uploadFile.s3.upload', [others])();
const minChunkSize = 5 * 1024 * 1024; // 5 MB
if (augmentedS3Params.Body instanceof fs.ReadStream && fs.statSync(augmentedS3Params.Body.path).size <= minChunkSize) {
// Buffer small files to avoid memory leak.
// Previous implementation used s3.putObject for small uploads, but it didn't have retries, see https://github.com/aws-amplify/amplify-cli/pull/13493.
// On the other hand uploading small streams leads to memory leak, see https://github.com/aws/aws-sdk-js/issues/2552.
// Therefore, buffering small files ourselves seems to be middle ground between memory leak and loosing retries.
// Buffering small files brings back balance between leaking and non-leaking uploads that is matching
// the ratio from before https://github.com/aws-amplify/amplify-cli/pull/13493.
augmentedS3Params.Body = await consumers.buffer(augmentedS3Params.Body);
}
uploadTask = this.s3.upload(augmentedS3Params);
if (showSpinner) {
uploadTask.on('httpUploadProgress', (max) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ const extractAPIModel = async (context: $TSContext, resource: $TSObject, framewo

fs.ensureDirSync(tempDir);

// After updating node types from 12.x to 18.x the objectResult
// became not directly assignable to Buffer.from parameter type.
// However, this code has been running fine since 2022 which means that
// runtime types are compatible.
// The alternative would require multiple logical branches to handle type mismatch
// that doesn't seem to exist in runtime.
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const buff = Buffer.from(data.body);
fs.writeFileSync(`${tempDir}/${apiName}.zip`, buff);
await extract(`${tempDir}/${apiName}.zip`, { dir: tempDir });
Expand Down
9 changes: 9 additions & 0 deletions packages/amplify-provider-awscloudformation/src/zip-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ export const downloadZip = async (s3: S3, tempDir: string, zipFileName: string,
log();
const objectResult = await s3.getFile({ Key: zipFileName }, envName);
fs.ensureDirSync(tempDir);

// After updating node types from 12.x to 18.x the objectResult
// became not directly assignable to Buffer.from parameter type.
// However, this code has been running fine since 2022 which means that
// runtime types are compatible.
// The alternative would require multiple logical branches to handle type mismatch
// that doesn't seem to exist in runtime.
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const buff = Buffer.from(objectResult);
const tempFile = `${tempDir}/${zipFileName}`;
await fs.writeFile(tempFile, buff);
Expand Down
25 changes: 17 additions & 8 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ __metadata:
"@types/deep-diff": ^1.0.0
"@types/folder-hash": ^4.0.1
"@types/lodash.throttle": ^4.1.6
"@types/node": ^12.12.6
"@types/node": ^18.16.0
"@types/uuid": ^8.0.0
amplify-codegen: 4.8.0
archiver: ^5.3.0
Expand Down Expand Up @@ -12488,10 +12488,12 @@ __metadata:
languageName: node
linkType: hard

"@types/node@npm:*, @types/node@npm:^18.16.1":
version: 18.16.1
resolution: "@types/node@npm:18.16.1"
checksum: bd43d9e1df253955d73348ae9c8bc73f328ad3bd5481a134743142a04b0d1b74e39b90369aa0d361fd0b86a6a5c98035a226c1e1a4f67ba98e71b06734cc4f7d
"@types/node@npm:*, @types/node@npm:^18.16.0, @types/node@npm:^18.16.1":
version: 18.19.31
resolution: "@types/node@npm:18.19.31"
dependencies:
undici-types: ~5.26.4
checksum: bfebae8389220c0188492c82eaf328f4ba15e6e9b4abee33d6bf36d3b13f188c2f53eb695d427feb882fff09834f467405e2ed9be6aeb6ad4705509822d2ea08
languageName: node
linkType: hard

Expand All @@ -12503,9 +12505,9 @@ __metadata:
linkType: hard

"@types/node@npm:^16.9.2":
version: 16.11.14
resolution: "@types/node@npm:16.11.14"
checksum: 017037da8387c85ea92c4ecb06dfb1f511e398ec14504a9395bc92948d58840755066eba991e308b39cf1117648be20cd3c43d184f1ec1339bb60b836dd5e4e7
version: 16.18.96
resolution: "@types/node@npm:16.18.96"
checksum: 05ac1c80c8d075086863f7640fd3e75c3912c4ed067bb38bb8fd5377f4e64de7a81d5be3ceae5448dc90d9802a0c7b0d3376538759b91ea652d16cc6dc7de767
languageName: node
linkType: hard

Expand Down Expand Up @@ -31492,6 +31494,13 @@ node-pty@beta:
languageName: node
linkType: hard

"undici-types@npm:~5.26.4":
version: 5.26.5
resolution: "undici-types@npm:5.26.5"
checksum: bb673d7876c2d411b6eb6c560e0c571eef4a01c1c19925175d16e3a30c4c428181fb8d7ae802a261f283e4166a0ac435e2f505743aa9e45d893f9a3df017b501
languageName: node
linkType: hard

"unfetch@npm:^4.2.0":
version: 4.2.0
resolution: "unfetch@npm:4.2.0"
Expand Down

0 comments on commit 2691a0a

Please sign in to comment.