Magic Folder: implement "Water Dragons" section of design doc #1710
Labels
No Label
0.2.0
0.3.0
0.4.0
0.5.0
0.5.1
0.6.0
0.6.1
0.7.0
0.8.0
0.9.0
1.0.0
1.1.0
1.10.0
1.10.1
1.10.2
1.10a2
1.11.0
1.12.0
1.12.1
1.13.0
1.14.0
1.15.0
1.15.1
1.2.0
1.3.0
1.4.1
1.5.0
1.6.0
1.6.1
1.7.0
1.7.1
1.7β
1.8.0
1.8.1
1.8.2
1.8.3
1.8β
1.9.0
1.9.0-s3branch
1.9.0a1
1.9.0a2
1.9.0b1
1.9.1
1.9.2
1.9.2a1
LeastAuthority.com automation
blocker
cannot reproduce
cloud-branch
code
code-dirnodes
code-encoding
code-frontend
code-frontend-cli
code-frontend-ftp-sftp
code-frontend-magic-folder
code-frontend-web
code-mutable
code-network
code-nodeadmin
code-peerselection
code-storage
contrib
critical
defect
dev-infrastructure
documentation
duplicate
enhancement
fixed
invalid
major
minor
n/a
normal
operational
packaging
somebody else's problem
supercritical
task
trivial
unknown
was already fixed
website
wontfix
worksforme
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Reference: tahoe-lafs/trac-2024-07-25#1710
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The prototype implementation of drop-upload in #1429 adds new or changed files in the local directory to the upload directory, but it does not unlink files in the upload directory when they are deleted locally.
Note that:
On 06/04/12 03:20, erpo4 wrote on the vg2 list:
It would indeed default to off, since we don't want the behaviour to change surprisingly from the current behaviour on a Tahoe upgrade.
Add magic-folder keyword to all drop-upload tickets.
The intended behaviour for Magic Folder is described at source:docs/proposed/magic-folder/remote-to-local-sync.rst#deletion-of-a-file.
drop-upload: optionally unlink locally deleted files from upload directoryto Magic Folder: implement local deletion of files as described in "Air Dragons" section of design docMagic Folder: implement local deletion of files as described in "Air Dragons" section of design docto Magic Folder: implement local deletion of files/directories as described in "Air Dragons" section of design docMagic Folder: implement local deletion of files/directories as described in "Air Dragons" section of design docto Magic Folder: implement local deletion of files/directories as described in "Water Dragons" section of design docI wrote some rough draft code that handles the local file deletion case:
this temporary dev branch:
https://github.com/david415/tahoe-lafs/tree/scan-parent
this commit:
https://github.com/david415/tahoe-lafs/commit/a256ddb50d384aa18ac241dcdb9d5757725ad9d2
but we should finish our rebase and cherry-pick this commit onto the new dev branch when it is available...
i put those changes here:
https://github.com/david415/tahoe-lafs/tree/2438.otf-objective-4.2-local-file-deletion.1
but we first need to get the tests to pass before working more on this...
my latest using an added api call in backupdb.py specifically for checking if a given filename has an entry in the db:
https://github.com/david415/tahoe-lafs/commit/594de028e81ce8afdc536fca4844b4c14fe9f6d0
in preparation for actually completing this task i've merged in the magic-folder CLI tools dev branch... and thus have been able to prepare tahoe magic-folder test grids using the CLI. Here we've written rough draft versions of test_persistence and test_move_tree that use the magic-folder CLI.
https://github.com/david415/tahoe-lafs/tree/1710.local-deletion-water-dragon.1
https://github.com/david415/tahoe-lafs/commit/0054798c9095466eea8446b66146156c9f2f4dc0
https://github.com/david415/tahoe-lafs/commit/ae9c492b79472fe71a30ee15dece87e29d18b7c1
we've introduced code duplication... next we should make sure all the magic-folder tests share this code instead of duplicating it.
we could benefit from a review but we are not currently blocked
cleaned up duplicate code here...
https://github.com/david415/tahoe-lafs/commit/6775c38f3eb40c494d7a06e5fa9dfee97f286507
i'm building up Alice-Bob tests here so we can have multiple writers to a magic-folder in a test:
https://github.com/david415/tahoe-lafs/tree/1710.local-deletion-water-dragon.1.test-alice-and-bob.1
I had to change the magic-folder cli unit test api when i added the Alice + Bob test. in this commit i fixed the non-Alice+Bob magic-folder CLI tests:
https://github.com/david415/tahoe-lafs/commit/e2ce372cbf02dc2ad56a87b505e157d576b7101b
I've added a naive downloader and remote scan mechanism... The basic "sharing" between Alice and Bob is working in so far as the unit test passes now.
this dev branch:
https://github.com/david415/tahoe-lafs/tree/1710.local-deletion-water-dragon.1.test-alice-and-bob.1
this commit:
https://github.com/david415/tahoe-lafs/commit/d214c7c2143e75a67a23b47d80220ff4f9837fbd
this test:
./bin/tahoe debug trial allmydata.test.test_magic_folder.RealTest.test_alice_bob
just now added "version" to the magic folder db schema... creating a version 3 of tahoe backupdb schema... which is unutilized by the existing tahoe backup command but is used by only magic folders thus far.
here we also roughly schetch out the addition and deletion behavior with the local version number being incremented by file modifications and deletions...
https://github.com/david415/tahoe-lafs/commit/209c0285be35aaa587cd71aee6bcc1bac4d3e452
added deletion/version propagation... and a basic test for that however i am having trouble getting the
test_persistence
test to pass... here's my latest:https://github.com/david415/tahoe-lafs/tree/1710.local-deletion-water-dragon.1.test-alice-and-bob.1
https://github.com/david415/tahoe-lafs/commit/ccdba44e8a51787ca39a9db41b975e9290727831
Alice and Bob mock unit test fixed in my dev branch:
https://github.com/david415/tahoe-lafs/tree/1710.local-deletion-water-dragon.1.test-alice-and-bob.1
Rebased to https://github.com/tahoe-lafs/tahoe-lafs/commits/1710.test-alice-and-bob.3.
slight progress with breaking work in progress commits on my dev branch here:
https://github.com/david415/tahoe-lafs/tree/2477.refactor-all-the-things.0
here the alice + bob test checks for proper version numbers... and we can identitify that the current difficulty happens right after alice deletes the file... her magic-folder db entry for that file should be updated with the new version number but it is not... and so the test fails because version number mismatch.
more progress in my dev branch... i got rid of the live-lock... however the test exits with a dirty reactor.
Merged David's changes and mine (to use
deferredutil.HookMixin
) into https://github.com/tahoe-lafs/tahoe-lafs/tree/2477.refactor-all-the-things.1.found and fixed two more dropped deferreds but the alice bob unit test still errs:
https://github.com/david415/tahoe-lafs/tree/2477.refactor-all-the-things.1
After our Tuesday pairing, tests now pass on that branch.
I do not believe we've implemented this Water Dragon feature:
"""When a Magic Folder client restarts, we can detect files that had been downloaded but were deleted while it was not running, because their paths will have last-downloaded records in the magic folder db without any corresponding local file."""
Yes, I believe that's correct (that it is not implemented). As well as scanning for existing files/directories at startup, we should simulate change events for paths in the magic folder db at which no file/directory exists.
We can do that by obtaining the set of all entries in the
local_files
table before doing the scan, removing paths from that set as we process them, and then processing all those remaining in the set.i added a get_all_files as a magic-folder db util function...
but this test_persistence unit test seems broken as the startup code path does not run twice like it should...
https://github.com/david415/tahoe-lafs/tree/2489.write_downloaded_file.1
I'm trying to fix the
test_alice_bob
test... and I've made some progress here... with this simple commit that teaches the uploader to ignore temp suffix files we create: ".tmp" and ".backup" :https://github.com/david415/tahoe-lafs/tree/1710.ignore-temp-files.0
Magic Folder: implement local deletion of files/directories as described in "Water Dragons" section of design docto Magic Folder: implement "Water Dragons" section of design docDaira pointed out yet another bug due to our Uploader / Downloader code refactor;
i made a fix commit here:
https://github.com/david415/tahoe-lafs/tree/2438.magic-folder-stable.1
"""
Add RemoteScanMixin to share our
_get_collective_latest_file
methodBoth the Uploader and Downloader need to know the latest collective
remote file...
"""
https://github.com/david415/tahoe-lafs/commit/6e8728ca0fd6a0186648bb64e187e999255f7bf3
Replying to dawuud:
The https://github.com/tahoe-lafs/tahoe-lafs/commits/2438.magic-folder-stable.3 branch fixes this in a different way; the 2438.magic-folder-stable.1 branches are now obsolete.
I think we can close this ticket now that all of the deletion propagation features have been implemented.
Can we close it?
We're close, but I need to review whether all of the Water Dragons features have been implemented.
I think we did not finish implementing deletion i did part of it; and meejah mentioned he added that feature. Please post the dev branch location so we can rebase it into our new dev/stable branches.
(https://github.com/tahoe-lafs/tahoe-lafs/commits/2438.magic-folder-stable.4) is the current stable branch (on which we did the demos).
https://github.com/tahoe-lafs/tahoe-lafs/commits/2521.magic-db-schema.1 is the current, broken, development branch.
There's also https://github.com/meejah/tahoe-lafs/commits/1710.actually-unlink-files.0 which I will do a version of rebased onto magic-db-schema.1 (I guess?)
I'm reviewing that branch now.
My current work-in-progress is at 2521.magic-db-schema.1.wip .
I have a https://github.com/meejah/tahoe-lafs/commits/1710.actually-unlink-files.1 branch now also, and another unit-test and further smoke test logic.
There's still a case which doesn't work:
If alice creates a file, then bob deletes it, and then alice re-creates it bob never sees it. (It does however, work, if bob is the one to re-create it). I'm still investigating this.
I mean: I have more things to push to the .1 branch, including some as-yet-unwritten code to fix the issue I mention above :)
daira: dawuud meejah: the deletion bug is an omission in the design doc. in the case of existent files, the event when the file is replaced by the downloader will be ignored because the entry for that path in the magic folder db has the same size/mtime/ctime as the new file
daira: the same should be the case for a deleted file. so the (size, mtime, ctime) in the db needs to be the same as returned by
fileutil.get_pathinfo
. the latter is (None, now, now) in the current implementation, which I think is probably wrong...daira: because the current time at the point of the
get_pathinfo
call won't/can't match the time in the databasedaira: the right fix is probably to change
get_pathinfo
to returnPathInfo(isdir=False, isfile=False, islink=False, exists=False, size=None, mtime=None, ctime=None)
for theENOENT
casedaira: and make sure that's also what is written to the db when the downloader detects a deleted file
(
None
is converted to an SQL NULL.)We also need to think about what happens when a file is replaced by a directory or vice versa. Those are treated as different paths (the directory path ends in
/
), so it's a bit different from the deleted file case.the case I have a fix for is: on say "bob's" side if "alice" deletes: the downloader downloads a "delete", so moves the file to "whatever.tmp" but immediately bob's uploader gets an inotify for the moved file, and when that gets processed it uploads another version (with delete=True) -- but so then if "alice" now replaces the file, bob doesn't download it because the versions match.
Replying to meejah:
Do you mean "whatever.backup"? Nothing should ever be moved to "whatever.tmp".
In any case, the IN_MOVED_FROM event for "whatever" should be ignored as described in comment:88084. (The IN_MOVED_TO event should also be ignored because it's for an ignorable filename pattern.)
Yes, this sounds like the same case that I was talking about. It isn't a problem that Bob's uploader gets an inotify event, but that event should be ignored because Bob's db should already have been updated with metadata showing that the file has been deleted.
I will update the design doc tomorrow to clarify what should happen.
Replying to daira:
See https://github.com/tahoe-lafs/tahoe-lafs/commit/5d2365f6c4cc388c2d74e43b2c1b7e8fc4c4ab86 (the previous three commits to the design doc are also worth reading).
So, I believe the fix is just to add this before the last line of
MagicFolderDB.is_new_file
:Replying to daira:
This fix is on the 2438.magic-folder-stable.5 branch, and there are tests for deletion. So I believe this is fixed, at least until we have a new design that handles the >2-client case.
Milestone renamed