search

owncloud / android

:phone: The ownCloud Android App
GNU General Public License v2.0
3.81k stars 3.05k forks source link

Resuming chunked uploads #2070

Open PVince81 opened 6 years ago

PVince81 commented 6 years ago

Steps to reproduce

  1. Prepare a big video files
  2. Upload big video files
  3. Disconnect from network
  4. Wait for error
  5. Connect again
  6. Wait

Expected result

Upload resumes from last starting point (takes less time)

Actual result

Upload does not resume (see https://github.com/owncloud/android/issues/2069) or would restart transfer from scratch.

Versions

Android app 2.5.0 Android 5.1.1

The mobile app should use the new chunking endpoint:

The idea is to just resume uploading the failed chunks and assume the other chunks are still on the server.

Beware that if no chunk upload is happening within more than 2 days, the transfer folder will not exist any more and old chunks are deleted. So if you get a 404 on PUT, assume the transfer was cancelled and start over.

Related: https://github.com/owncloud/android/issues/1400

PVince81 commented 6 years ago

I have some huge files (videos) from holiday to upload and I know they won't survive wifi timeout when phone automatically locks. This leads to me having to actively use the phone because I know that if the upload cancels it will start from the beginning, and without me forcing the WiFi to on actively, the upload will never complete as its resuming starts again at zero.

This is why I think this feature is rather important.

PVince81 commented 6 years ago

I suspect this would make affect users using instant upload. Such users usually expect no interaction at all with the client as everything is uploaded. (are videos included ?) In case of big files, the upload might never complete due to the above reasons (network disconnect), so such user might wonder later why their files still haven't reached the server after so much time.

PVince81 commented 6 years ago

This is getting more annoying over time. I can't comfortably upload my holidays videos on mobile as they keep failing when I forget to manually keep my mobile phone unlocked / connected.

cc @michaelstingl

michaelstingl commented 6 years ago

@davigonz @jesmrec Is some of the annoyance addressed in the 2.5.1 release?

davigonz commented 6 years ago

Hi @PVince81 , I agree with you about this feature, it is pretty annoying for the user to have to be actively using the phone so that the big upload is completed. We will address this problem for coming releases, but not for the 2.5.1 @michaelstingl since it should be ready in January and will include the new camera uploads. To fix the big uploads problem we would need to use the new chunking endpoint as well.

patrickjahns commented 6 years ago

I've also experienced some issues, that might have been easier if we support upload resuming:

During my holidays I was not always having reliably wifi connections. For uploading smaller files this was not a issue - but with uploading several pictures (and videos) the uploads would fail inbetween. But instead of resuming the uploads, they would need to start from the beginning.

Photos would upload eventually since the file size is just around 5-10MB - but with larger files (movies) it was a very painful experience.

Additionally I noticed, that when putting the app in the background (by turning off the phone, or switching context for example to the browser), the uploads would also just stop at one point. Switching back would start the upload process again.

harshitbansal05 commented 6 years ago

@davigonz i would like to work on this issue, can I start?

shashvat-kedia commented 6 years ago

@davigonz I would like to work on this.

davigonz commented 6 years ago

i would like to work on this issue, can I start?

@harshitbansal05 thank you for being interested in this issue but let's start with #2071 and #2080 first and we will talk about this issue then.

davigonz commented 6 years ago

I would like to work on this.

@sd1998 We are thinking about changing our network library for the next release, so I would prefer not to start with this issue before that part is finished.

davigonz commented 6 years ago

@PVince81 just a question about the last step with chunked uploads:

MOVE http://localhost/owncloud/remote.php/dav/uploads/$userId/$transferId/.file to Destination: remote.php/dav/files/$userId/target/path/finalfile.dat

How long does the server take to build the file once all the chunks are uploaded? Is there any way to check that file is completed and ready to be moved to the final destination?

PVince81 commented 6 years ago

@davigonz the assembly depends on the size of the file and number of chunks. The duration is likely similar to the duration of the last PUT when using the old chunking.

There is currently no mechanism to track this. However there's a POC for a possible mechanism to "check later" here: https://github.com/owncloud/core/pull/31851

davigonz commented 6 years ago

@PVince81 how have you handled this scenario in core? Is it possible to know the content of remote.php/dav/uploads/<user> via PROPFIND? If so, which webdav properties could we request?

PVince81 commented 6 years ago

in the web UI we use a transfer id based on the final file path and time stamp: https://github.com/owncloud/core/blob/v10.0.9/apps/files/js/file-upload.js#L45

davigonz commented 6 years ago

in the web UI we use a transfer id based on the final file path and time stamp: https://github.com/owncloud/core/blob/v10.0.9/apps/files/js/file-upload.js#L45

It makes sense, I will follow that path, thanks!

davigonz commented 5 years ago

With the next scenario:

  1. User A shares a folder with user B, with no edit permissions
  2. User B uploads a file to the shared folder

@PVince81 how do you know this is forbidden with the new endpoint? Because I've noticed that with the old chunking endpoint you get an error when trying to upload the first chunk to the final destination while with the new endpoint, since the chunks are saved in a different folder than the final one protected by permissions, there's no 403 error, just a 500 error when moving the whole file to the final destination.

Any clue about this? Thanks

PVince81 commented 5 years ago

A 500 error is likely a bug, please report in core with the matching curl requests for the chunk upload (unless easily reproducible in web UI and cheating the permissions)

Currently the expectation is that the final MOVE tells you whether the operation can be done or not. There is no way to check this upfront because the final destination path is only known during the final MOVE. So expect a 403 during MOVE.

davigonz commented 5 years ago

@PVince81 I have already opened an issue (https://github.com/owncloud/core/issues/33416) and noticed that web UI shows the error You don't have permission to upload or create files here even before starting the upload and the progress is not shown either, how is that possible if as you said, there's no way to check this upfront because the final destination path is only known during the final MOVE?

PVince81 commented 5 years ago

@davigonz this might be because the file list reads the "oc:permissions" of the current folder to determine whether read-only or not, so it does not even start the upload.

You can likely cheat this way:

  1. open two browsers A and B
  2. in A, create a folder "test" and share with another user user2
  3. in B, login as user2
  4. see that "test" is writable, navigate into it
  5. now in A, remove the "edit" permission on "test"
  6. go back to B, the UI still believes that it's read-write, now upload a big file
  7. observe network console in B

you'll likely see that the network process is the same as for mobile now

davigonz commented 5 years ago

@PVince81 I managed to reproduce it in the same browser as well, as you can see in the gif attached in the core issue I created, I was just curious about that error message. Thanks for the explanation!

davigonz commented 5 years ago

This will need to wait a bit more due to architectural changes that are being carried out in the Android app. It makes totally sense to start with chunked upload when the app contains those improvements and avoid additional work. Please follow up https://github.com/owncloud/android/issues/2351 to know the status of the architectural changes.

jesmrec commented 4 years ago

To include in new arch when TUS is finally decided to be included. (https://github.com/owncloud/android/issues/2978)