#32217 Change the file locking on large uploads #32226
Conversation
micbar
added
Type:Bug
3 - To Review
blue-ticket
backport-request
p3-medium
micbar
force-pushed
the
file-locking-large-uploads
branch
from
ddfdcfd
to
08503ab
Compare
Codecov Report
@@ Coverage Diff @@
## master #32226 +/- ##
============================================
- Coverage 64.01% 64% -0.01%
- Complexity 18560 18563 +3
============================================
Files 1171 1171
Lines 69837 69845 +8
Branches 1267 1267
============================================
+ Hits 44706 44707 +1
- Misses 24761 24768 +7
Partials 370 370
Continue to review full report at Codecov.
|
| @@ -188,9 +188,25 @@ public function put($data) { | |||
| // because we have no clue about the cause we can only throw back a 500/Internal Server Error | |||
| throw new Exception('Could not write file contents'); | |||
| } | |||
|
|
|||
| try { | |||
| $this->changeLock(ILockingProvider::LOCK_EXCLUSIVE); | |||
There was a problem hiding this comment.
please move this before fopen to avoid having two concurrent processes opening the file at the same time
There was a problem hiding this comment.
are you sure? If i do this, tho whole operations fails.
Is it possible to lock a file before opening?
There was a problem hiding this comment.
how exactly does it fail ? it shouldn't fail as the storage fopen shouldn't lock anything. Maybe there is something else that is suspicious there...
in general if we leave it like you did then a concurrent process would be able to lock the target file, but would fail after fopen and before starting stream copy. so the error handler would need to close the file again, which I think will happen anyway.
would still be good to investigate why moving the lock before makes the operation fail, in case there are some ghosts hidden somewhere...
micbar
force-pushed
the
file-locking-large-uploads
branch
2 times, most recently
from
42dbf9b
to
c9d30d6
Compare
|
@DeepDiver1975 @PVince81 As seen in the put() is executed with a shared lock. Please review and ping back, i get the impression, that this is a very sensitive part :-) |
micbar
force-pushed
the
file-locking-large-uploads
branch
from
c9d30d6
to
57cd53a
Compare
Changed locking to avoid exclusive lock during hooks
micbar
force-pushed
the
file-locking-large-uploads
branch
from
57cd53a
to
8ee1d3f
Compare
|
Backport |
|
@micbar do you remember where exactly you put the |
| $target = $partStorage->fopen($internalPartPath, 'wb'); | ||
| if (!\is_resource($target)) { | ||
| \OCP\Util::writeLog('webdav', '\OC\Files\Filesystem::fopen() failed', \OCP\Util::ERROR); | ||
| // because we have no clue about the cause we can only throw back a 500/Internal Server Error | ||
| throw new Exception('Could not write file contents'); | ||
| } | ||
|
|
There was a problem hiding this comment.
I had a sleep() statement after setting the exclusive lock and before the final move operation.
There was a problem hiding this comment.
@micbar which value did you use ? how high to set it to have the desktop client time out on its own ?
There was a problem hiding this comment.
`sleep(300)' in both locations
There was a problem hiding this comment.
The desktop client resends the MOVE request after 300sec.
Description
When uploading large files >10GB, the assembly process on the server runs very long (>5minutes). This causes a timeout on the client side (Version 2.4.2). The setup is a clustered web server behind a F5 load balancer.
New move Requests by the client, let the running assembly processes fail.
Related Issue
Motivation and Context
Created PR to test if we can make the file locking process more robust
How Has This Been Tested?
Types of changes
Checklist:
Open tasks: