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

Make S3 MultipartUpload Concurrency Configurable - Address S3 Timeouts #24330

Closed
wants to merge 2 commits into from

Conversation

J-Light
Copy link

@J-Light J-Light commented Nov 24, 2020

Created a new service with the latest Nextcloud docker image with S3 backend. File 4GB and larger fail to upload. Issue discussed #20519 and in forums

https://help.nextcloud.com/t/an-exception-occurred-while-uploading-parts-to-a-multipart-upload/47163
https://help.nextcloud.com/t/primary-storage-s3-files-large-more-than-4gb/59797
https://help.nextcloud.com/t/s3-primary-object-store-large-uploads-2gb-fail-with-exception-and-timeout-using-webdav-or-nextcloud-client-or-web-ui/67185
https://help.nextcloud.com/t/s3-random-storage-problem-on-large-files/72897

No concrete solution proposed.

The Socket Timeout error by trial and error is down to Concurrency level used by AWS's S3 SDK. The default is 5 and it may be stressing the container or server environments.

In my case setting this to a value of 3 explicitly resolved the issue of Socket Timeouts.

The PR make this configurable with a default value the value used as default in the SDK.

New to project to suggestions welcome.

@solracsf
Copy link
Member

solracsf commented Nov 24, 2020

Interesting.

I wonder if setting it to 1 could not solve issues like #21958 or #23781 and kind-of simulate a non-multipart upload... 🤔

So my question is: shouldn't this be set to 1 as default, then document it so administrators can raise it if needed?

@J-Light
Copy link
Author

J-Light commented Nov 25, 2020

@acsfer I think the idea is good. Since setting it to 3 I when from 100% failure to about 20 to 30% failure. I am going to keep this for a few more days to assess. Setting to 1 would also be a good default, its something I will be testing the coming days.

@J-Light
Copy link
Author

J-Light commented Jan 11, 2021

Any progress on this front? Upgraded my instance and noticed errors again and remembered I needed to include this.

@skjnldsv skjnldsv requested a review from juliusknorr August 18, 2021 12:06
@skjnldsv

This comment was marked as resolved.

@@ -96,9 +96,10 @@ public function writeObject($urn, $stream, string $mimetype = null) {
'bucket' => $this->bucket,
'key' => $urn,
'part_size' => $this->uploadPartSize,
'concurrency' => $this->config->getSystemValue('objectstore.arguments.concurrency', 5),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should rather be passed as a param of the existing object store configuration which are parsed in the S3ConnectionTrait, similar to how it is done for the uploadPartSize already:

$this->uploadPartSize = !isset($params['uploadPartSize']) ? 524288000 : $params['uploadPartSize'];

@PVince81
Copy link
Member

@juliushaertl does this clash somehow with the chunked upload PR or is still relevant and useful in its own way ?

@juliusknorr
Copy link
Member

Still relevant I'd say, for cases where chunked upload is not working, e.g. using a random webdav client or Windows webdav mount.

@sebastiansterk
Copy link
Member

Yes I agree as well. It is still relevant.

@timstoop
Copy link

timstoop commented Mar 6, 2023

I'd like to point out that this is still relevant in 25.0.4.

@frittentheke
Copy link

frittentheke commented Apr 4, 2023

With #27034 merged for Nextcloud 26, what does this PR then still improve / fix?

@juliusknorr
Copy link
Member

See #24330 (comment)

@skjnldsv
Copy link
Member

@J-Light would you be alright to address Julius's comment :)

If not, I will close this PR for inactivity unless someone else is willing to take over 🙏

@skjnldsv skjnldsv added 2. developing Work in progress stale Ticket or PR with no recent activity and removed 3. to review Waiting for reviews labels Feb 27, 2024
@frittentheke
Copy link

Still relevant I'd say, for cases where chunked upload is not working, e.g. using a random webdav client or Windows webdav mount.

@juliushaertl can you explain which components communicate via which protocol and where your proposed setting for concurrency come into play?

From what you said I imagine a WebDAV client uploading a large file to Nextcloud. That then uploads this large file to an S3 storage and uses multipart due to the file size to do so.

@J-Light
Copy link
Author

J-Light commented Feb 28, 2024

Not actively using or working on this anymore in large part due to this issue. Have moved to OCIS. Happy to close this given how old this is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress feature: object storage stale Ticket or PR with no recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants