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

Do not check all chunks of a chunked upload if we do not need to #22602

Merged
merged 2 commits into from
Mar 9, 2016

Conversation

rullzer
Copy link
Contributor

@rullzer rullzer commented Feb 23, 2016

Fixes #22601

Before we did a full test on all chunks to verify if a chunked upload
was completed. This is unneeded since if we are missing one chunk we can
already fail.

Also we look from back to front since it is much more likely that we
find a missing chunk thus can error out early.

FYI: @dragotin @DeepDiver1975 @jturcotte @ogoffart

On my system this saves ~4 seconds on a 1GB upload. But the slower the storage the bigger the gain.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @bartv2, @DeepDiver1975 and @PVince81 to be potential reviewers

@guruz
Copy link
Contributor

guruz commented Mar 1, 2016

👍

@rullzer rullzer force-pushed the fix_slow_chunkcheck branch from 4d353ef to 47acc59 Compare March 2, 2016 07:06
@rullzer rullzer added this to the 9.0.1-next-maintenance milestone Mar 2, 2016
@rullzer
Copy link
Contributor Author

rullzer commented Mar 2, 2016

Please review @PVince81 @nickvergessen @DeepDiver1975

@rullzer rullzer modified the milestones: 9.1-next, 9.0.1-next-maintenance Mar 7, 2016
@rullzer
Copy link
Contributor Author

rullzer commented Mar 7, 2016

Set milestone to 9.1 as this PR is against master.

@rullzer
Copy link
Contributor Author

rullzer commented Mar 7, 2016

To test, upload a 'large' file using the desktop client.

@PVince81
Copy link
Contributor

PVince81 commented Mar 7, 2016

Unit test would be nice, if possible, for consistent results.
The test should test storing chunks with different orders.

rullzer added 2 commits March 7, 2016 19:52
Fixes #22601

Before we did a full test on all chunks to verify if a chunked upload
was completed. This is unneeded since if we are missing one chunk we can
already fail.

Also we look from back to front since it is much more likely that we
find a missing chunk thus can error out early.
* Unit tests for OC_Filechunking to verify the isComplete function
* Intergration tests to show that shuffling chunks is all fine
@rullzer rullzer force-pushed the fix_slow_chunkcheck branch from 47acc59 to 7301b43 Compare March 7, 2016 20:23
@rullzer
Copy link
Contributor Author

rullzer commented Mar 7, 2016

@PVince81 some unit tests and intergration tests :)

@PVince81
Copy link
Contributor

PVince81 commented Mar 7, 2016

Looks better 👍

@rullzer
Copy link
Contributor Author

rullzer commented Mar 9, 2016

@karlitschek would be good to backport this to 9.0.1 since it helps with a blue ticket on the client side. (Issue is also against 9.0.1).

@icewind1991
Copy link
Contributor

Looks good 👍

@karlitschek
Copy link
Contributor

backport makes sense 👍

@rullzer
Copy link
Contributor Author

rullzer commented Mar 9, 2016

stable9: #23022

DeepDiver1975 added a commit that referenced this pull request Mar 9, 2016
Do not check all chunks of a chunked upload if we do not need to
@DeepDiver1975 DeepDiver1975 merged commit 0cc53ee into master Mar 9, 2016
@DeepDiver1975 DeepDiver1975 deleted the fix_slow_chunkcheck branch March 9, 2016 14:06
@lock
Copy link

lock bot commented Aug 7, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chunked Files: Optimize checking if all chunks are there
9 participants