feat(s3): upload file bigger than 50GB #7067
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fbeauchamp
force-pushed
the
feat_s3_stream
branch
from
6b9b129
to
676851e
Compare
also Put object part with a prevalculated md5 and size doesn'c consume additionnal memory against presigned + raw upload
julien-f
reviewed
Oct 10, 2023
@xen-orchestra/fs/src/s3.js
Outdated
Comment on lines
232
to
234
| if (this._readCounter === undefined) { | ||
| this._readCounter = 0 | ||
| } |
There was a problem hiding this comment.
Use a closure (variable outside of the function) and initialize it to 0.
| region = guessAwsRegion(host), | ||
| } = parse(url) | ||
| const client = new S3Client({ | ||
| apiVersion: '2006-03-01', |
| keepAlive: true, | ||
| }), | ||
| }), | ||
| }) |
There was a problem hiding this comment.
I see we are not passing tls option, is this necessary in our S3 implementation?
@xen-orchestra/fs/testupload.mjs
Outdated
Comment on lines
111
to
113
| while((data = await read(inputStream, Math.min(remainingBytes, maxChunkPartSize))) !== null){ | ||
| remainingBytes = await write(data, chunkStream, remainingBytes) | ||
| } |
There was a problem hiding this comment.
Why a loop? Doesn't the first read give you everything?
There was a problem hiding this comment.
This is for padding, but we should use a TransformStream instead for better perf (less buffering).
fbeauchamp
force-pushed
the
feat_s3_stream
branch
from
552951b
to
4d10e26
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
chunk size computation
Chunk size is (length ?? maxLength ) / 10 000 . (min 5MB, max 500MB).
can be computed from disk utilization (for an xva export) or disk size (for a mirror backup)
Transfer strategy
lib-storageI think we should use 2 on non object locked, non encrypted backup repository, and 5 in other. A file configuration should be present to force the fallback to lib-storage
To do
Checklist
Fixes #007,See xoa-support#42,See https://...)Introduced byCHANGELOG.unreleased.md