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

Increase chunked upload finalization timeout #6527

Closed
ckamm opened this issue May 15, 2018 · 6 comments
Closed

Increase chunked upload finalization timeout #6527

ckamm opened this issue May 15, 2018 · 6 comments
Assignees
Milestone

Comments

@ckamm
Copy link
Contributor

ckamm commented May 15, 2018

See https://github.com/owncloud/enterprise/issues/2480

Some server setups regularly hit the 5min timeout on the final MOVE or PUT. We don't like the large timeouts, but increasing it based on the size of the uploaded document seems like a required workaround until the processing is sped up on the server or, preferably, an asynchronous API is introduced.

Concretely: 5min + 3min/gb ? This is entirely made up and should be checked against data (@SamuAlfageme)

@ckamm ckamm added this to the 2.4.2-maybe milestone May 15, 2018
@ckamm ckamm self-assigned this May 15, 2018
@SamuAlfageme
Copy link
Contributor

@ckamm looks like a plan; as for the exact figures; I might poke some server devs. to benchmark the assembly step and determine some sort of estimate.

@guruz
Copy link
Contributor

guruz commented May 15, 2018

Maybe @butonic @PVince81 @DeepDiver1975 have some insight on a good formula.
if not let's just get the initial suggestion of 5min + 3min/gb into 2.4.2 as soon as possible..

@SamuAlfageme
Copy link
Contributor

I was just having a discussion with @butonic and he's also trying to come up with a solution for this at some level. Let's hold our horses - his solution might make way more sense wo/ going full owncloud/core#12097

@guruz
Copy link
Contributor

guruz commented May 15, 2018

@SamuAlfageme Increasing the timeout in the client side is a (quick and small) workaround for the CURRENT situation for all servers. We can still implement the dynamic timeout and remove it again in some years.

@ogoffart
Copy link
Contributor

I'd do qBound(normalTimeout, 3 minutes * size in GB , normalTimeout * 10 )

3 minutes per gigabyte is about 5.5 MB/s. For comparison, a google search for hard drive speed on google gives results in the range of 75 - 140 MB/s But i have no idea how slow an anti-virus can be.

The reason to use bound is to keep the timout reasonable. If it takes one hour to detect a timeout, that means it will also take one hour to recover when the connection drops and the timeout is real. (So normalTimeout*10 is maybe a bit high even)

ckamm added a commit that referenced this issue May 16, 2018
Some servers have virus scanners and the like that can delay the
response of the final chunked upload assembly significantly, often
breaking the current 5min (!) timeout. See owncloud/enterprise#2480
for details.
ckamm added a commit that referenced this issue May 17, 2018
Some servers have virus scanners and the like that can delay the
response of the final chunked upload assembly significantly, often
breaking the current 5min (!) timeout. See owncloud/enterprise#2480
for details.
@ckamm ckamm added ReadyToTest QA, please validate the fix/enhancement and removed PR available labels May 17, 2018
ckamm added a commit that referenced this issue May 17, 2018
Some servers have virus scanners and the like that can delay the
response of the final chunked upload assembly significantly, often
breaking the current 5min (!) timeout. See owncloud/enterprise#2480
for details.

(cherry picked from commit 2638332)
@SamuAlfageme
Copy link
Contributor

Re-labeled since the patch has proved to be useful for the large file file scenario - but far from a complete compromise solution.

Like @ogoffart points out in #6527 (comment), it's sane to try keep the timeout reasonable. Thus the 30 min limit; if there's additional assembly delays (i.e. I/O load on the server, antivirus processing the assembled file before MOVE gets replied, etc.).

Server support for server-to-client communication (e.g. #6195) for async. ops. is required to get a reliable solution. I'll close here as the workaround does its job 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants