Skip to content

Limit concurrent downloads & uploads #2313

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 23, 2022
Merged

Limit concurrent downloads & uploads #2313

merged 3 commits into from
Sep 23, 2022

Conversation

bexsoft
Copy link
Collaborator

@bexsoft bexsoft commented Sep 15, 2022 •
edited
Loading

fixes #2163

What does this do?

Limits concurrent downloads & uploads in console Object Manager

To set variables, you need to start console server with the following flags: CONSOLE_MAX_CONCURRENT_UPLOADS
& CONSOLE_MAX_CONCURRENT_DOWNLOADS

if values are set to 0, then no limit is set

Signed-off-by: Benjamin Perez [email protected]

@bexsoft bexsoft self-assigned this Sep 15, 2022
prakashsvmx
prakashsvmx reviewed Sep 15, 2022
View reviewed changes
@prakashsvmx
Copy link
Member

prakashsvmx commented Sep 15, 2022 •
edited
Loading

Observation:
set a limitation of CONSOLE_MAX_CONCURRENT_UPLOADS=4
Drag and drop a set of files and the crash can be observed. ( 4 files are uploaded which can be seen after refresh)

TrafficMonitor.tsx:88 Uncaught TypeError: Cannot read properties of undefined (reading 'send')
    at TrafficMonitor.tsx:88:1
  • could we show a warning/error when there are more files than the limit?

prakashsvmx
prakashsvmx reviewed Sep 15, 2022
View reviewed changes
@bexsoft bexsoft force-pushed the limit-concurrent-uploads branch 3 times, most recently from 4b725ae to 924441b Compare September 15, 2022 20:57
@bexsoft
Copy link
Collaborator Author

bexsoft commented Sep 15, 2022

Observation: set a limitation of CONSOLE_MAX_CONCURRENT_UPLOADS=4 Drag and drop a set of files and the crash can be observed. ( 4 files are uploaded which can be seen after refresh)

TrafficMonitor.tsx:88 Uncaught TypeError: Cannot read properties of undefined (reading 'send')
    at TrafficMonitor.tsx:88:1
  • could we show a warning/error when there are more files than the limit?

fixed

@bexsoft bexsoft force-pushed the limit-concurrent-uploads branch 2 times, most recently from f29f78d to 9216362 Compare September 19, 2022 18:43
@bexsoft bexsoft changed the title [PoC] Limit concurrent downloads & uploads Limit concurrent downloads & uploads Sep 19, 2022
@bexsoft bexsoft requested a review from prakashsvmx September 19, 2022 18:43
@bexsoft bexsoft force-pushed the limit-concurrent-uploads branch from 9216362 to ec5cbf2 Compare September 19, 2022 19:10
dvaldivia
dvaldivia previously requested changes Sep 20, 2022
View reviewed changes
Copy link
Collaborator

@dvaldivia dvaldivia left a comment

Choose a reason for hiding this comment

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

For CONSOLE_MAX_CONCURRENT_UPLOADS=2 I see sets of 2 but others sets of uploads are let through, at some point the desired effects happens

Screen Shot 2022-09-20 at 3 53 33 PM

@bexsoft bexsoft force-pushed the limit-concurrent-uploads branch 2 times, most recently from aaa5497 to 3e8de68 Compare September 21, 2022 19:23
@bexsoft bexsoft added the WIP This PR is WIP and cannot be merged yet label Sep 21, 2022
@bexsoft bexsoft force-pushed the limit-concurrent-uploads branch from cf2d6f3 to 8013bde Compare September 21, 2022 23:30
@bexsoft bexsoft removed the WIP This PR is WIP and cannot be merged yet label Sep 21, 2022
@bexsoft bexsoft requested a review from dvaldivia September 21, 2022 23:48
@bexsoft
Copy link
Collaborator Author

bexsoft commented Sep 22, 2022

For CONSOLE_MAX_CONCURRENT_UPLOADS=2 I see sets of 2 but others sets of uploads are let through, at some point the desired effects happens

Screen Shot 2022-09-20 at 3 53 33 PM

Fixed

prakashsvmx
prakashsvmx approved these changes Sep 22, 2022
View reviewed changes
Copy link
Member

@prakashsvmx prakashsvmx left a comment

Choose a reason for hiding this comment

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

Tested. Changes look good to me 👍

@dvaldivia dvaldivia merged commit d1511c5 into minio:master Sep 23, 2022
@bexsoft bexsoft deleted the limit-concurrent-uploads branch January 23, 2023 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reivaj05 reivaj05 reivaj05 approved these changes

@dvaldivia dvaldivia dvaldivia approved these changes

@prakashsvmx prakashsvmx prakashsvmx approved these changes

@harshavardhana harshavardhana Awaiting requested review from harshavardhana

@Alevsk Alevsk Awaiting requested review from Alevsk

@cniackz cniackz Awaiting requested review from cniackz

@jinapurapu jinapurapu Awaiting requested review from jinapurapu

Assignees

@bexsoft bexsoft

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Limit concurrent files upload
4 participants
@bexsoft @prakashsvmx @reivaj05 @dvaldivia