Skip to content

only allow a single concurrent dav write to a file - #9355

Merged
MorrisJobke merged 1 commit into
masterfrom
dav-upload-lock
May 3, 2018
Merged

only allow a single concurrent dav write to a file#9355
MorrisJobke merged 1 commit into
masterfrom
dav-upload-lock

Conversation

@icewind1991

Copy link
Copy Markdown
Member

While concurrent writes wont lead to file corruption it's a waste of bandwith and multiple chunked upload assemblies can lead to the server running out of space

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991icewind1991 added the 3. to review Waiting for reviews label May 1, 2018
@icewind1991icewind1991 added this to the Nextcloud 14 milestone May 1, 2018
@codecov

codecovBot commented May 1, 2018

Copy link
Copy Markdown

Codecov Report

Merging #9355 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@ Coverage Diff @@## master #9355 +/- ##
============================================
+ Coverage 51.93% 51.93% +<.01% 
Complexity 25394 25394 ============================================
Files 1608 1608 Lines 95444 95448 +4 Branches 1393 1393 ============================================
+ Hits 49565 49569 +4 
Misses 45879 45879
Impacted FilesCoverage ΔComplexity Δ
apps/dav/lib/Connector/Sabre/Directory.php66.45% <100%> (+0.85%)65 <0> (ø)⬇️
apps/files_trashbin/lib/Trashbin.php72.46% <0%> (-0.25%)136% <0%> (ø)
apps/files_trashbin/lib/Expiration.php91.93% <0%> (+1.61%)29% <0%> (ø)⬇️

@MorrisJobke

Copy link
Copy Markdown
Member

@icewind1991 Could you explain a bit more the use case and scenario here?

@icewind1991

Copy link
Copy Markdown
MemberAuthor

Something was causing a sync client to have send multiple requests to do chunk assembly for a 100gb file at the same time.
The different part files were filling up the disk, causing the assemblies to fail and the whole thing to start over.

With this we can be sure that only one assembly can run at the same time

@rullzer

Copy link
Copy Markdown
Member

So you just lock some specific file (which doesn't exist at that point) right? Just to prevent multiple trying to do it at the same time right?

@icewind1991

Copy link
Copy Markdown
MemberAuthor

Yes, locking a dummy file allows blocking concurrent access while still allowing read access to the file while writing the part file

@MorrisJobke
MorrisJobke merged commit 7cb467b into masterMay 3, 2018
@MorrisJobke
MorrisJobke deleted the dav-upload-lock branch May 3, 2018 14:28
@MorrisJobke

Copy link
Copy Markdown
Member

@icewind1991@rullzer Should we backport this to stable13?

@rullzer

Copy link
Copy Markdown
Member

Probably!

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

Labels

3. to reviewWaiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@icewind1991@MorrisJobke@rullzer