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

Rename a folder during upload race condition #11795

Closed
PVince81 opened this issue Oct 27, 2014 · 10 comments
Closed

Rename a folder during upload race condition #11795

PVince81 opened this issue Oct 27, 2014 · 10 comments

Comments

@PVince81
Copy link
Contributor

Steps to reproduce

  1. Create a folder "A" in the web UI
  2. Setup the sync client, let it sync
  3. Close the sync client
  4. Copy lots of files inside of the local "A" folder (in my case pictures about 3-7 MB)
  5. Start the sync client and let it upload the files
  6. During the upload, rename "A" to "A2"
  7. Wait until the sync client does its second try

Expected result

All files uploaded to "A2"

Actual result

About 90% of the time, there will be two folders "A" and "A2" where part of the files go to "A" and the other part to "A2". Sometimes some files are even missing.

See owncloud/client#2296 for LOTS of details about my debugging results.

Basically there seems to be a time window during which the FS update doesn't match the cache (because it hasn't been updated yet). If in that time window a concurrent call runs scanFile(), it will find the mtime different on disk and interpret the folder "A2" as a new folder and it will make a new entry for it in the cache, even though "A" is still there.

@PVince81
Copy link
Contributor Author

CC @moscicki @icewind1991

@PVince81
Copy link
Contributor Author

Setting to showstopper/gold as per original, but I don't think this should block the release.
This is an old issue that eventually needs to be looked into.

Ideally a high-level file operation locking mechanism needs to be designed and developed to avoid such concurrency issues.

@PVince81
Copy link
Contributor Author

@karlitschek
Copy link
Contributor

A showstopper is something that blocks the release. It it doesn´t block the release than it´s not gold/showstopper. Please make up your mind :-)

@PVince81
Copy link
Contributor Author

Yes, I just wasn't sure whether I could simply remove the gold label as I didn't get any feedback so far.
Thanks for removing it.

@PVince81
Copy link
Contributor Author

CC @jnfrmarks for an "interesting" test case. The symtoms/results change based on the timing.

@PVince81
Copy link
Contributor Author

Tested with high-level file locking #16237

Let's think about the locking case from #16237: since we're uploading multiple files, there are two possible conditions, either:

  1. We are still uploading (or doing the final rename), so the parent folder is locked and cannot be renamed.
    or
  2. We are between two uploads (or still gathering chunks) and the parent folder is not locked. In this case the parent folder can be renamed. When renaming it, the folder should be locked during the rename operation + cache update so any uploads (actually their final rename) should fail / be blocked.

So far I only managed to meet condition 2) with the steps above, which doesn't seem to work properly yet with high-level file locking.

It currently doesn't work, probably because small files (I used files < 20 MB) are going directly to the storage.

@PVince81
Copy link
Contributor Author

@cmonteroluque we probably also want this assigned to 8.1 (because I don't see why #13391 is but not this one here)

@MorrisJobke
Copy link
Contributor

@cmonteroluque Ping

@ghost ghost added this to the 8.1-current milestone Jun 15, 2015
@PVince81
Copy link
Contributor Author

I retested with #17017 and #16963 combined and couldn't reproduce any loss.

Either the rename tells me the folder is locked, which means a file operation is pending. Or the client cannot find the target folder since I renamed it.
After the second sync run it properly recovers and all files are still there.

However, it might happen that a second folder got created with some files in it from the first try. This is not something we can avoid at this level of locking. But I think the most important thing is that no file is going missing. They all still had their copy available in the original folder, which the sync client simply reuploaded.

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

No branches or pull requests

3 participants