uppy icon indicating copy to clipboard operation
uppy copied to clipboard

Companion downloads/uploads never canceled

Open mifi opened this issue 2 years ago • 2 comments

Companion: If the user leaves the uppy upload (closes browser or cancels it), the download seems to keep on going forever until companion has downloaded the whole file to disk. This is not a big issue for small files, but if a user tries many times to upload a 100GB file, it’s gonna quickly fill up the hard drive and starve CPU. There is no method in the Provider API for canceling/aborting downloads, so even if the upload stops, the download will keep on going until finished.

  • [x] Also: If the user presses the cancel button in the UI, then only the upload will be canceled. Download still going until the end of the file. This is now solved by #3159

I believe the optimal solution would be that companion server keeps a keep-alive/ping with the client. if the client disappears after X minutes, then cancel the download and the upload

To test this:

  • Start upload of big file
  • Go to dev tools, networking. Set to offline
  • Observe "Upload failed" message, and upload gets canceled
  • In companion, observe that downloads and uploads are still going until they finish

Edit: This may also cover the issue of paused uploads possibly not being garbage collected (if they are paused but never resumed by the user)

Further improvements:

  • [ ] Cancel upload also when canceling download (readStream). now it will just eventually time out due to no more data from the readstream

mifi avatar Sep 01 '21 10:09 mifi

Keep in mind that the continuation of the download is expected behavior for certain people. Example: #3128

Murderlon avatar Sep 01 '21 12:09 Murderlon

Ok, if that's a feature we're supporting, then maybe the client shouldn't say "upload failed" if it loses connection, but rather resume its connection with companion and continue receiving progress events? Then we just need to make sure that the companion server has a lot of available space (terabytes), or rewrite companion to be stream based so we don't need to store anything on the server #3098

mifi avatar Sep 01 '21 12:09 mifi

if that's a feature we're supporting, then maybe the client shouldn't say "upload failed" if it loses connection, but rather resume its connection with companion and continue receiving progress events?

That's already the case, right? The client will try to reconnect and it can continue to receive progress in that case. It's important that clients can "fire and forget", they disconnect to do something else and the upload will finish.

Murderlon avatar Apr 08 '24 10:04 Murderlon