Skip to content

Throttle getstoragestats.php calls and allow simultaneous uploads - #13855

Merged
ChristophWurst merged 1 commit into
nextcloud:masterfrom
tomasz-grobelny:faster_file_upload
Jan 29, 2019
Merged

Throttle getstoragestats.php calls and allow simultaneous uploads#13855
ChristophWurst merged 1 commit into
nextcloud:masterfrom
tomasz-grobelny:faster_file_upload

Conversation

@tomasz-grobelny

@tomasz-grobelnytomasz-grobelny commented Jan 27, 2019

Copy link
Copy Markdown
Contributor

Improves speed on #13854 and #13696. Does not address reliability in any way.

Signed-off-by: Tomasz Grobelny <tomasz@grobelny.net>

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and works nicely 👍 Also the getStorageStats call on every upload is not there anymore. Nice finding 💯

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good 👍

@kesselb

kesselb commented Jan 29, 2019

Copy link
Copy Markdown
Contributor

Cool 👍

limitConcurrentUploads: 10 if you select 10 files a 50mb they are uploaded in parallel. not sure if this is a sensible default. i would vote for a lower value (e.g. 4).

@ChristophWurst

Copy link
Copy Markdown
Member

@danielkesselberg but but but what if I upload 100 tiny files? Then 10 concurrent will probably be faster 😉

@tomasz-grobelny

tomasz-grobelny commented Jan 29, 2019

Copy link
Copy Markdown
ContributorAuthor

Cool 👍

limitConcurrentUploads: 10 if you select 10 files a 50mb they are uploaded in parallel. not sure if this is a sensible default. i would vote for a lower value (e.g. 4).

This heavily depends on your uplink, server speed and filesize. I believe one could justify just about any value. I did my tests on a pcengines apu2c4 with about 5MB files and I was still far from reaching the link speed (got 10Mbit/s).

@kesselb

Copy link
Copy Markdown
Contributor

Ok if this is a reasonable standard that works for most of your users.

@BloodyIron

Copy link
Copy Markdown

Why not make this an admin setting that we can set in the webGUI as an admin to tune it for our setup? I mean, hiding the setting seems like asking for trouble. Yes? :)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsfeature: files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@tomasz-grobelny@kesselb@ChristophWurst@BloodyIron@MorrisJobke