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

Detect chunked upload timeouts and retry #30770

Closed
pmaier1 opened this issue Mar 13, 2018 · 16 comments · Fixed by #30776
Closed

Detect chunked upload timeouts and retry #30770

pmaier1 opened this issue Mar 13, 2018 · 16 comments · Fixed by #30776
Assignees
Labels
blue-ticket p2-high Escalation, on top of current planning, release blocker server sev2-high Type:Bug
Milestone

Comments

@pmaier1
Copy link
Contributor

pmaier1 commented Mar 13, 2018

With IE uploads sometimes get stuck. Very important issue for the next server release.

@pmaier1 pmaier1 added enhancement p1-urgent Critical issue, need to consider hotfix with just that issue labels Mar 13, 2018
@pmaier1 pmaier1 added this to the development milestone Mar 13, 2018
@butonic
Copy link
Member

butonic commented Mar 13, 2018

uploads can get stuck with any browser. The IE case is a little bit different because IE sometimes just stops uploading. In the middle of a chunk. The ui freezes, but we already tested that the upload can be aborted using js. Afterwards the browser works fine again. So, the idea is to retry stalled chunks in general. It makes sense for all browsers, but for IE it even allows recovering from a UI freeze.

@ownclouders
Copy link
Contributor

GitMate.io thinks possibly related issues are #25480 (Detect DB connection timeout and retry connection), #16278 (Problems with Chunked upload), #5326 (web dav chunked upload tasks), #14603 (Disable / detect mod_reqtimeout to avoid upload problems), and #17996 (Prevent uploading the same chunk twice).

@butonic
Copy link
Member

butonic commented Mar 13, 2018

hm https://github.com/blueimp/jQuery-File-Upload/wiki/Chunked-file-uploads#resuming-file-uploads & https://github.com/blueimp/jQuery-File-Upload/wiki/Chunked-file-uploads#automatic-resume look promising. I already have the test code that aborts the stalled upload. It should be possible to customize these snippets to automatically resume the upload at the right chunk if we can get the current chunk or the number of uploaded bytes from the server.

@PVince81 IIRC you wanted to get rid of blueimp ...

@PVince81
Copy link
Contributor

blueimp is not fully compatible with our way of doing chunking, so it might be rather tricky to make it resume. still worth a try ?

@felixboehm
Copy link
Contributor

If a upload breaks for any reason, ideally we can just continue that upload.
Yet, on client side the state for "currently uploading fileXYZ" is lost, when the page is reloaded.
Does it make sense to store this state on the server to retry on stalled uploads?
@PVince81 @butonic

@PVince81
Copy link
Contributor

At some point in the future we can consider saving upload state on the server, out of scope for this fix as it involves a lot of changes in the server to make the upload stateful. Additionally we'd need to make sure that the file being resume is exactly the same as the one we just started.

For the scope of this fix the goal is to make it resume without reloading the page as the state information is still there in some form.

@PVince81 PVince81 self-assigned this Mar 27, 2018
@felixboehm
Copy link
Contributor

ok, when the reason for the failing uploads is not needing a reload.

@PVince81
Copy link
Contributor

In Phoenix let's experiment with resumable.js: owncloud/web#67.

If it turns out that it works well we could consider backporting...

@butonic
Copy link
Member

butonic commented Apr 4, 2018

  • disable trace

@PVince81
Copy link
Contributor

the fix is causing trouble now on stable10 / v10.0.8RC2, reverting #31185 for the release

@butonic butonic added p2-high Escalation, on top of current planning, release blocker sev2-high blue-ticket and removed p1-urgent Critical issue, need to consider hotfix with just that issue green-ticket labels Apr 20, 2018
@PVince81 PVince81 modified the milestones: development, backlog May 28, 2018
@PVince81 PVince81 removed their assignment May 28, 2018
@PVince81
Copy link
Contributor

needs rescheduling

@PVince81 PVince81 self-assigned this Jul 23, 2018
@PVince81 PVince81 modified the milestones: backlog, development Jul 24, 2018
@PVince81
Copy link
Contributor

Estimate: 1md to get this sorted out.

this time let's also try and add JS tests for chunking, even though it's difficult gymnastics because of the jquery.fileupload object and its various event handlers

@PVince81
Copy link
Contributor

bringing back the old PR where work will continue: #32170

@PVince81
Copy link
Contributor

from the changelog, this is what did not work while testing:

when simulating chunk failure, the upload finishes with error about sum of chunks sizes not matching

@PVince81
Copy link
Contributor

ok, seems the problem is solved: the PR itself was ok but the code that simulated failure wasn't behaving correctly and was returning success instead of an error code on timeout.

I've adjusted the "patch that simulates timeout" here: https://gist.github.com/PVince81/2978ed02a21876fd9e58f74beda7e5c0

In the future it would be nice to have something in the testing app so one would only need to set some oc_appconfig settings, see the idea here: #32183

@PVince81
Copy link
Contributor

PVince81 commented Aug 1, 2018

PR was merged, will be in 10.0.10. closing

@PVince81 PVince81 closed this as completed Aug 1, 2018
@PVince81 PVince81 modified the milestones: development, QA Jan 11, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blue-ticket p2-high Escalation, on top of current planning, release blocker server sev2-high Type:Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants