Skip to content
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

Create a Way to Disable Parallel Uploads #2743

Closed
dragotin opened this issue Jan 23, 2015 · 20 comments
Closed

Create a Way to Disable Parallel Uploads #2743

dragotin opened this issue Jan 23, 2015 · 20 comments
Assignees
Labels
Needs info p2-high Escalation, on top of current planning, release blocker

Comments

@dragotin
Copy link
Contributor

For various reasons we can not enable the parallel upload by default. We need to disable the parallel upload and only switch it on by a flag if admins decide that. Even if that results in a performance drop.

e.g. #2724 #1866 (comment) #2401

@dragotin dragotin added this to the 1.8 - UI Enhancements milestone Jan 23, 2015
@dragotin dragotin added the p2-high Escalation, on top of current planning, release blocker label Jan 23, 2015
@dragotin
Copy link
Contributor Author

This is not only about uploads but also all other operations. Disable parallelism by default.

@guruz
Copy link
Contributor

guruz commented Jan 23, 2015

OK... check for status.php version to be >= 8.1 ?

Or check the capabilities API for a maxparallel value.
curl -X GET 'http://root:admin@localhost/owncloud7/ocs/v1.php/cloud/capabilities'

@PVince81
Copy link
Contributor

As discussed on IRC we could add a new value "maxparallel" to that capabilities call with the following logic: if SQLite is used, "maxparallel" would be 1, else 0 (infinite)

@PVince81
Copy link
Contributor

In general I think we should start using this "capabilities" endpoint more often to share server info with clients.

@dragotin
Copy link
Contributor Author

agree with @PVince81

@guruz
Copy link
Contributor

guruz commented Jan 23, 2015

We also had the idea to check all error messages for SQLSTATE and then force paralleism down to 1

@PeterPablo
Copy link

Would disabling parallelism also prevent wrong/missing ctimes and mtimes (confer the problem described in the referenced issue #2401).

@ogoffart
Copy link
Contributor

@PeterPablo: no, it would just make it less likely.

sqlite should be avoided on the server if possible.

@PeterPablo
Copy link

In my opinion users should be made aware of this in a more pressing alert than described in the current documentation (confer #2401 (comment)). I still consider this loss of data and as such a bug that should be tackled.

@radry
Copy link

radry commented Jan 25, 2015

I agree with this issue! On my low memory owncloud php runs out of memory when uploading many files with the windows client because of parallel uploads!! Uploading via a browser works perfectly fine. This is a major issue since there are no (free) alternative clients which can sync (instead of "mounting" the webdav which I don't want).

ogoffart added a commit that referenced this issue Jan 26, 2015
If the server can't cope with parallel upload, it can advertise it
in the capabilities secion

Issue #2743
Need owncloud/core#13628
@dragotin
Copy link
Contributor Author

dragotin commented Feb 5, 2015

We need to know to which extend the server fixed the underlying problems to be able to decide what to do.

@PVince81
Copy link
Contributor

PVince81 commented Feb 5, 2015

Parallel upload should work now on OC 8 RC 2, the major database is locked issues were fixed.
It worked fine in my various tests (and I had a slow HDD / VM where the issue could be reproduced consistently before the fix)

@PVince81
Copy link
Contributor

PVince81 commented Feb 5, 2015

ogoffart added a commit that referenced this issue Feb 5, 2015
If the server can't cope with parallel upload, it can advertise it
in the capabilities secion

Issue #2743
Need owncloud/core#13628
@radry
Copy link

radry commented Feb 5, 2015

@PVince81

Great but what about php running out of memory on low end servers? I realize that is not the problem of owncloud per se but the parallel upload is still to blame, so there MUST be an option to disable parallel upload in the client or the server manually.

@guruz
Copy link
Contributor

guruz commented Feb 5, 2015

@radry I believe that problem should be fixed because the 1.8 client only uses a chunk size of 5MB (vs 20MB in 1.7.x and 10 MB in 10.6.x)

@ogoffart
Copy link
Contributor

ogoffart commented Feb 6, 2015

So according @PVince81 in #2724 (comment)
recommand disabling parallelism for anything less than owncloud 8.0

Should we disable only parallel chunk upload (which is new in 1.8), or any kind of parallelism?
Don't we risk users complaining about preformence regression in 1.8?

Another option we could take is to disable the parallelism once we detect the 500 error.

@moscicki
Copy link
Contributor

moscicki commented Feb 6, 2015

I would like to say it again: what about that the server tells the client what kind of features it supports? In this way you may avoid any hardcoded assumptions and also avoid to manually re-adjust the client when the new versions of the server develop new capabilities (which is anyway impossible because you still have old versions of the servers around).

The same also for the size of the chunk. Changing from 10MB -> 20MB -> 5MB looks a bit like a random walk for me. I KNOW what is the averge file size in my service, so why would not you let me configure the server with the correct number for MY service.

And above all it all may be achieved by adding extending simple JSON reponse of status.php. Why make it any more complicated?

kuba

On Feb 6, 2015, at 1:00 PM, Olivier Goffart [email protected] wrote:

So according @PVince81 in #2724 (comment)
recommand disabling parallelism for anything less than owncloud 8.0

Should we disable only parallel chunk upload (which is new in 1.8), or any kind of parallelism?
Don't we risk users complaining about preformence regression in 1.8?

Another option we could take is to disable the parallelism once we detect the 500 error.


Reply to this email directly or view it on GitHub.

@ogoffart
Copy link
Contributor

ogoffart commented Feb 6, 2015

@moscicki I agree with you, and that is already implemented by d7c2b66 for the amount of parallelism, but the server do not expose this after all.

@PVince81
Copy link
Contributor

PVince81 commented Feb 6, 2015

I'm fine with adding this back or making this configurable, but not sure if others agree: @karlitschek @DeepDiver1975
The ticket for "max_parallels" was this one: owncloud/core#13628

@ogoffart ogoffart added the ReadyToTest QA, please validate the fix/enhancement label Feb 12, 2015
@luciamaestro
Copy link

It works fine version 1.8.0-beta1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs info p2-high Escalation, on top of current planning, release blocker
Projects
None yet
Development

No branches or pull requests

8 participants