AWS S3 v6 sends chunks in serial instead of parallel + possibly memory leak
Initial checklist
- I understand this is a bug report and questions should be posted in the Community Forum
- I searched issues and couldn’t find anything (or linked relevant results below)
Link to runnable example
No response
Steps to reproduce
I'm using Uppy with AWS S3 plugin in my React Native/Expo app and I run into some issues after trying to upgrade to v6. To make it work with v5 I actually had to make a patch to solve some problems but it's nothing complex:
diff --git a/node_modules/@uppy/aws-s3/lib/HTTPCommunicationQueue.js b/node_modules/@uppy/aws-s3/lib/HTTPCommunicationQueue.js
index 2fdb973..0326348 100644
--- a/node_modules/@uppy/aws-s3/lib/HTTPCommunicationQueue.js
+++ b/node_modules/@uppy/aws-s3/lib/HTTPCommunicationQueue.js
@@ -1,5 +1,6 @@
import { pausingUploadReason } from './MultipartUploader.js';
import { throwIfAborted } from './utils.js';
+import pLimit from "p-limit";
function removeMetadataFromURL(urlString) {
const urlObject = new URL(urlString);
urlObject.search = '';
@@ -187,7 +188,7 @@ export class HTTPCommunicationQueue {
signal,
}).abortOn(signal);
let body;
- const data = chunk.getData();
+ const data = await chunk.getData();
if (method.toUpperCase() === 'POST') {
const formData = new FormData();
Object.entries(fields).forEach(([key, value]) => formData.set(key, value));
@@ -225,7 +226,8 @@ export class HTTPCommunicationQueue {
const { uploadId, key } = await this.getUploadId(file, signal);
throwIfAborted(signal);
try {
- const parts = await Promise.all(chunks.map((chunk, i) => this.uploadChunk(file, i + 1, chunk, signal)));
+ const limit = pLimit(6);
+ const parts = await Promise.all(chunks.map((chunk, i) => limit(() => this.uploadChunk(file, i + 1, chunk, signal))));
throwIfAborted(signal);
return await this.#sendCompletionRequest(this.#getFile(file), { key, uploadId, parts, signal }, signal).abortOn(signal);
}
@@ -282,7 +284,7 @@ export class HTTPCommunicationQueue {
};
for (;;) {
throwIfAborted(signal);
- const chunkData = chunk.getData();
+ let chunkData = await chunk.getData();
const { onProgress, onComplete } = chunk;
let signature;
try {
@@ -305,7 +307,7 @@ export class HTTPCommunicationQueue {
}
throwIfAborted(signal);
try {
- return {
+ let data = {
PartNumber: partNumber,
...(await this.#uploadPartBytes({
signature,
@@ -316,6 +318,8 @@ export class HTTPCommunicationQueue {
signal,
}).abortOn(signal)),
};
+ chunkData = null;
+ return data;
}
catch (err) {
if (!(await this.#shouldRetry(err, chunkRetryIterator)))
diff --git a/node_modules/@uppy/aws-s3/lib/MultipartUploader.js b/node_modules/@uppy/aws-s3/lib/MultipartUploader.js
index 19fea1a..5e812d9 100644
--- a/node_modules/@uppy/aws-s3/lib/MultipartUploader.js
+++ b/node_modules/@uppy/aws-s3/lib/MultipartUploader.js
@@ -88,9 +88,9 @@ class MultipartUploader {
for (let offset = 0, j = 0; offset < fileSize; offset += chunkSize, j++) {
const end = Math.min(fileSize, offset + chunkSize);
// Defer data fetching/slicing until we actually need the data, because it's slow if we have a lot of files
- const getData = () => {
+ const getData = async () => {
const i2 = offset;
- return this.#data.slice(i2, end);
+ return await this.#data.slice(i2, end);
};
this.#chunks[j] = {
getData,
@@ -112,7 +112,7 @@ class MultipartUploader {
else {
this.#chunks = [
{
- getData: () => this.#data,
+ getData: async () => this.#data,
onProgress: this.#onPartProgress(0),
onComplete: this.#onPartComplete(0),
shouldUseMultipart,
diff --git a/node_modules/@uppy/aws-s3/lib/createSignedURL.js b/node_modules/@uppy/aws-s3/lib/createSignedURL.js
index f9fc2d5..52e549e 100644
--- a/node_modules/@uppy/aws-s3/lib/createSignedURL.js
+++ b/node_modules/@uppy/aws-s3/lib/createSignedURL.js
@@ -68,9 +68,9 @@ async function hash(key, data) {
/**
* @see https://docs.aws.amazon.com/IAM/latest/UserGuide/create-signed-request.html
*/
-export default async function createSignedURL({ accountKey, accountSecret, sessionToken, bucketName, Key, Region, expires, uploadId, partNumber, }) {
+export default async function createSignedURL({ accountKey, accountSecret, sessionToken, bucketName, Key, Region, expires, uploadId, partNumber, endpoint }) {
const Service = 's3';
- const host = `${Service}.${Region}.amazonaws.com`;
+ const host = endpoint || `${Service}.${Region}.amazonaws.com`;
/**
* List of char out of `encodeURI()` is taken from ECMAScript spec.
* Note that the `/` character is purposefully not included in list below.
diff --git a/node_modules/@uppy/aws-s3/lib/index.js b/node_modules/@uppy/aws-s3/lib/index.js
index 123fa57..1d5e5e1 100644
--- a/node_modules/@uppy/aws-s3/lib/index.js
+++ b/node_modules/@uppy/aws-s3/lib/index.js
@@ -250,6 +250,7 @@ export default class AwsS3Multipart extends BasePlugin {
Key: key ?? `${crypto.randomUUID()}-${file.name}`,
uploadId,
partNumber,
+ endpoint: this.opts.endpoint,
})}`,
// Provide content type header required by S3
headers: {
Most important change is adding await to slice method because RN app runs into OOM error when creating a blob from big file so I had to create my own "async blob" to prevent loading entire file to memory. I also limited amount of chunks uploaded at once and cleared variable containing their's data after upload. There is also a fix for using endpoint as host because I use Cloudflare R2 but it's not relevant for this issue.
After upgrading Uppy to v6 I also made a patch for adding await to slice method but after a couple of chunks there is error:
Call to function 'FileSystemFileHandle.readBytes' has been rejected.
→ Caused by: Unable to read from a file handle: 'Bad file descriptor' what looks like is error connected to my async blob implementation for reading chunks but it work's perfectly fine with v5 and if I use it to just read chunks and skip only the upload there is also no error so it looks like it's connected to Uppy. I suspect it may be some kind of memory leak keeping all the chunks in memory and system rejects file operation because it's running out of memory. I tried to clear variables containing chunks data after upload like in my v5 patch but it doesn't help.
Separate issue is that chunks are uploaded in serial instead of parallel like in v5 (in my v5 patch there's actually added a limit to amount of chunks uploaded at once to prevent OOM), it makes upload significantly slower.
Here is my new patch for v6, it works for smaller files but bigger ones crash after a couple of chunks:
diff --git a/node_modules/@uppy/aws-s3/lib/S3Uploader.js b/node_modules/@uppy/aws-s3/lib/S3Uploader.js
index 240af15..3b18146 100644
--- a/node_modules/@uppy/aws-s3/lib/S3Uploader.js
+++ b/node_modules/@uppy/aws-s3/lib/S3Uploader.js
@@ -208,7 +208,7 @@ export default class S3Uploader {
continue; // already uploaded
const chunk = this.#chunks[i];
const partNumber = i + 1;
- const chunkData = this.#data.slice(chunk.start, chunk.end);
+ let chunkData = await this.#data.slice(chunk.start, chunk.end);
const chunkIndex = i; // Capture for closure (cannot use for-loop variable i directly in a closure)
if (this.#key == null) {
throw new Error('Missing S3 object key for uploading part');
@@ -224,6 +224,7 @@ export default class S3Uploader {
},
signal,
});
+ chunkData = null;
// after part finished uploading, update chunk state
this.#chunkState[i].uploaded = chunk.size;
this.#chunkState[i].etag = etag;Expected behavior
Upload with paralell chunks like with v5
Actual behavior
Chunks uploading in serial and crash after a couple of chunks
Source: transloadit/uppy