Opened at 2015-10-26T09:04:43Z
Closed at 2020-06-30T13:40:47Z
#2551 closed enhancement (somebody else's problem)
Magic Folder: implement "Fire Dragons" section of design doc
Reported by: | dawuud | Owned by: | daira |
---|---|---|---|
Priority: | normal | Milestone: | undecided |
Component: | code-frontend-magic-folder | Version: | 1.10.1 |
Keywords: | remote conflict detection | Cc: | |
Launchpad Bug: |
Description
Implement remote conflict detection as documented in the "Fire Dragons" section of the design doc: https://tahoe-lafs.org/trac/tahoe-lafs/browser/docs/proposed/magic-folder/remote-to-local-sync.rst
Change History (24)
comment:1 Changed at 2015-10-26T09:14:06Z by dawuud
comment:2 Changed at 2015-10-26T09:30:53Z by dawuud
"there are pending notifications of changes to foo;" I think this means that if there are local pending notification for foo from the Downloader then it's a conflict because these local changes didn't account for the latest changes in the DMD. Correct?
"the last-uploaded statinfo is either absent, or different from the current statinfo;" means that we compare the statinfo from the DMD metadata of the last uploaded file to the last downloaded statinfo in the magic-folder db?
comment:3 Changed at 2015-10-26T09:54:48Z by daira
Yes (but I think you mean from the Uploader), and yes.
comment:4 Changed at 2015-10-26T11:05:14Z by daira
Reviewed https://github.com/david415/tahoe-lafs/commit/1893110d11a4bf9669cda3431ebd5d72f5a4d9f6, -1 for now.
comment:5 Changed at 2015-10-26T14:20:49Z by dawuud
I've read the code review and made some of the corrections here:
https://github.com/david415/tahoe-lafs/tree/2551.remote-conflict-detection.0
comment:6 Changed at 2015-10-26T16:09:44Z by daira
Reviewing that now.
comment:7 Changed at 2015-10-26T16:13:46Z by daira
Created https://github.com/tahoe-lafs/tahoe-lafs/pull/198 to facilitate review.
comment:8 Changed at 2015-10-26T17:52:23Z by dawuud
Thanks for the code reviews. I've made all the requested changes and pushed them to my dev branch 2551.remote-conflict-detection.0... however it doesn't look like the changes showed up in the github pull request.
I think the next step is to write a unit test that produces a conflict and test that it gets handled properly...
After that we can add timestamp comparison etc.
comment:9 Changed at 2015-10-26T19:08:04Z by daira
Reviewed, +1. Please unindent is_conflict = True by 4 spaces, and then squash the branch relative to 2438.magic-folder-stable.4, and open a new PR. Thanks!
comment:10 Changed at 2015-10-26T23:26:37Z by dawuud
I fixed the indentation... and taught it to count the conflicts as well. I started working on a conflict test... but failed to teach bob to upload a file... latest commits here: https://github.com/david415/tahoe-lafs/tree/2551.remote-conflict-detection.0
comment:11 Changed at 2015-10-27T10:02:04Z by dawuud
Added a basic conflict unit test that works... it correctly exercises the conflict detection... however we should make a couple more unit tests to test the other remote conflict cases; this one only triggers to failure of the remote ancestry comparison.
comment:12 Changed at 2015-10-27T23:04:59Z by daira
Current branch is https://github.com/tahoe-lafs/tahoe-lafs/tree/2438.magic-folder-stable.5.
comment:13 Changed at 2015-10-28T16:08:26Z by dawuud
my current dev branch is forked from that stable.5 branch: https://github.com/david415/tahoe-lafs/tree/2551.remote-conflict-detection.3
this now includes timestamp comparison... however i did not yet write a unit test for this new feature.
comment:14 Changed at 2015-10-29T11:19:12Z by dawuud
i pushed another commit that checks for pending uploads to detect remote conflicts... but i didn't write a test for it yet.
comment:15 Changed at 2015-10-29T15:17:09Z by dawuud
I wrote a test for the last uploaded uri comparison... however i noticed that both Alice and Bob will both receive the conflict if the uploader doesn't update the magic-folder db... which the test currently does not.
comment:16 follow-up: ↓ 17 Changed at 2015-11-03T19:39:28Z by dawuud
i've merged 2553.do-not-read-from-own-dmd.3 into my dev branch... and i made the changes to the test_alice_bob` test to make it pass... namely to not account for a conflict propagation between alice and bob.
https://github.com/david415/tahoe-lafs/tree/2551.remote-conflict-detection.3
comment:17 in reply to: ↑ 16 Changed at 2015-11-03T22:54:23Z by daira
- Keywords review-needed added
- Status changed from new to assigned
Replying to dawuud:
https://github.com/david415/tahoe-lafs/tree/2551.remote-conflict-detection.3
I rebased this to https://github.com/tahoe-lafs/tahoe-lafs/tree/2551.remote-conflict-detection.4 so that it's a fast forward of 2553.do-not-read-from-own-dmd.3 (and 2438.magic-folder-stable.5).
I will review it and manually test it on Windows, then push it onto 2438.magic-folder-stable.5 if everything checks out.
comment:18 Changed at 2015-11-04T10:44:21Z by dawuud
I started to write a test for the pending upload conflict case... but didn't finish getting it to work; the test is made to pass. I'll come back to it later and fix it.
https://github.com/david415/tahoe-lafs/tree/2551.remote-conflict-detection.3 https://github.com/david415/tahoe-lafs/commit/6e13f802dd6fbef2225c0bd40808c69e658f8dcb
comment:19 Changed at 2015-11-04T22:51:00Z by dawuud
after pairing with Daira today we fixed the conflict detection... this here dev branch is a continuation of that effort; here i've fixed the remote conflict detection:
- teach Uploader to set the dmd metadata's last_downloaded_uri to db_entry.last_uploaded_uri
- teach Downloader to call did_upload_version properly and set last_uploaded_uri to filecap and set last_downloaded_uri to last_uploaded_uri
- fix Downloader remote conflict detection:
- conflict "2a" is triggered appropriately for deletion versus modification
- conflict "2cii" and "2ciii" shall not be triggered when the version is zero
here: https://github.com/david415/tahoe-lafs/tree/2551.wip.2
comment:20 Changed at 2015-11-05T00:32:04Z by daira
Reviewing.
comment:21 Changed at 2015-11-05T02:03:03Z by daira
Reviewed, -1. (See comments on https://github.com/david415/tahoe-lafs/commit/23e76b0dfb5aae23d05458747f2bbd1845831f07.)
comment:22 Changed at 2015-11-05T11:06:35Z by dawuud
ok... well I've again reviewed the Fire Dragon section of the design doc and I gotta -1 it because it doesn't correctly detect conflicts with parties of three or more:
Alice downloads version x from Bob, then uploads versions x+1 followed by x+2. The metadata last_downloaded_uri will be set to URI of version x. Ada downloads version x+2 from Alice. Ada uploads version x+3, setting the last_downloaded_uri in the metadata to URI of x+2. Bob downloads version x+3 and sees it as a conflict because last_downloaded_uri is set to URI of x+2 and is not equal to Bob's last_uploaded_uri which is the URI of version x.
We should be able to solve this if we change our data model to include file history. This would simultaneously resolve #2565
comment:23 Changed at 2015-11-23T22:53:47Z by zooko
Here's my description of this issue on the tahoe-dev mailing list: pipermail/tahoe-dev/2015-November/009615.html
comment:24 Changed at 2020-06-30T13:40:47Z by exarkun
- Keywords review-needed removed
- Resolution set to somebody else's problem
- Status changed from assigned to closed
magic-folder has been split out into a separate project - https://github.com/leastauthority/magic-folder
The design doc states:
I've attempted to provide basic remote conflict detection by comparison of ancestor URIs as described in the last point of section 2d.
Here's my dev branch: https://github.com/david415/tahoe-lafs/commits/2506.enforce-paths.5