Skip to content

Fix download of large files in Console #2773

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

Merged
merged 25 commits into from
May 15, 2023

Conversation

reivaj05
Copy link
Contributor

Fixes: #2760

@dvaldivia
Copy link
Collaborator

I found a memory problem while downloading a large file, to test this I did the following

  1. Generate a 100GiB file via mkfile -n 100g ~/Desktop/LargeTestFile (on macOS)
  2. Upload the file via object browser, this completed just fine
  3. attempt to download the file, then Safari reset the page due to memory pressure, but I saw the download continues even if the download progress component no longer reports it, after a few zombie connections, it stops al together

Screenshot 2023-04-12 at 11 41 37 AM

@dvaldivia
Copy link
Collaborator

on Chrome I was able to download the 100GiB file

@dvaldivia
Copy link
Collaborator

perhaps for Safari we can force a direct download? @reivaj05

@reivaj05
Copy link
Contributor Author

@dvaldivia I believe we won't be able to set anonymous access header if we force a direct download, is that right?

@dvaldivia
Copy link
Collaborator

@reivaj05 when do we need to do that? :S

@reivaj05
Copy link
Contributor Author

@reivaj05
Copy link
Contributor Author

Added way to download directly in safari if file size is more than 4G

@bexsoft bexsoft requested review from pjuarezd, cesnietor and dilverse May 2, 2023 20:22
bexsoft
bexsoft previously requested changes May 9, 2023
Copy link
Collaborator

@bexsoft bexsoft left a comment

Choose a reason for hiding this comment

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

Tested the PR with the following scenarios:

  • Uploaded 5GB File, everything OK
  • Downloaded File in Chrome, Need to find a way to display more constant batches for the progress bar as it updates only every 25%
    2023-05-09 11-43-57 2023-05-09 11_48_00
  • On Safari, the whole application freezes between batches and sometimes crashes once the download has been completed
    2023-05-09 11-52-12 2023-05-09 11_54_24
  • On Firefox Navigator Download manager & MinIO's object manager opens at the same time (not really an issue as current behavior opens Firefox manager after MinIO's object manager completes the download)
  • On Opera same behavior as Firefox is observed

@cesnietor
Copy link
Collaborator

Tested on Chrome with 4G and 100G files ✅
On Safari I also experienced the same behavior raised by @bexsoft where the app crashes and kind of sends me to the Object browser page. This happens even with 4G files. Small files seem to be working fine.

@reivaj05
Copy link
Contributor Author

reivaj05 commented May 11, 2023

@bexsoft i couldn't find a better way to display progress, current approach to get the chunks is fileSize/1.5G, i could reduce the chunk size in the code but smaller chunks will translate in more requests to minio and rate limit errors, with 1.5G we avoid the rate limit error

@bexsoft @cesnietor i'm going to let safari browser handle the downloads directly instead of setting a limit because seems 4G files is still too much for safari to handle

@reivaj05 reivaj05 dismissed bexsoft’s stale review May 11, 2023 16:26

Address safari comment

@reivaj05 reivaj05 requested a review from bexsoft May 11, 2023 16:26
@cesnietor
Copy link
Collaborator

Tested in Safari and is download is now being handled by the browser. ✅
Screenshot 2023-05-11 at 11 44 49 AM

cesnietor
cesnietor previously approved these changes May 11, 2023
Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

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

typo fixed

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

Successfully merging this pull request may close these issues.

"Network error occured" when downloading large file
5 participants