Magic Folder: implement "Water Dragons" section of design doc #1710

Closed
opened 2012-04-06 03:00:12 +00:00 by davidsarah · 45 comments
davidsarah commented 2012-04-06 03:00:12 +00:00
Owner

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:

  • some users might not want files to be immediately unlinked from the upload directory, in case the local deletion was accidental.
  • the space for unlinked files would not be reclaimed unless and until garbage collection is performed. (This applies already to old versions of changed files.)
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: * some users might not want files to be immediately unlinked from the upload directory, in case the local deletion was accidental. * the space for unlinked files would not be reclaimed unless and until garbage collection is performed. (This applies already to old versions of changed files.)
tahoe-lafs added the
code-frontend
normal
enhancement
1.9.1
labels 2012-04-06 03:00:12 +00:00
tahoe-lafs added this to the undecided milestone 2012-04-06 03:00:12 +00:00
davidsarah commented 2012-04-06 04:17:13 +00:00
Author
Owner

On 06/04/12 03:20, erpo4 wrote on the vg2 list:

davidsarah wrote:

erpo4 wrote:

I think the drop_upload feature is what I want. Files I put in the
local directory seem to be uploaded to the grid. However, when I
delete the files out of my local directory, the files are not
automatically deleted from the grid. Is this the expected behavior?

Yes, it is. I've just filed
<https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1710> to allow the
automatic deletion behaviour. (This isn't likely to be a priority
to support in the next couple of releases, though, unless someone
volunteers to implement it.)

Just to be clear: I wasn't complaining. :) The only reason I asked was
that it's not the behavior I'm used to from other cloud storage
services (e.g. Ubuntu One). It may actually be preferable not to
autodelete files that have been removed from the drop_upload
directory. In the case that I accidentally rm -rf /drop_upload_dir/*,
I wouldn't want that to wipe out my online backups when tahoe gets
inotify'd. If this is to be a feature, perhaps it should default to
off?

It would indeed default to off, since we don't want the behaviour to change surprisingly from the current behaviour on a Tahoe upgrade.

On 06/04/12 03:20, erpo4 wrote on the vg2 list: > davidsarah wrote: > > erpo4 wrote: > > > I think the drop_upload feature is what I want. Files I put in the > > > local directory seem to be uploaded to the grid. However, when I > > > delete the files out of my local directory, the files are not > > > automatically deleted from the grid. Is this the expected behavior? > > > > Yes, it is. I've just filed > > <<https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1710>> to allow the > > automatic deletion behaviour. (This isn't likely to be a priority > > to support in the next couple of releases, though, unless someone > > volunteers to implement it.) > > Just to be clear: I wasn't complaining. :) The only reason I asked was > that it's not the behavior I'm used to from other cloud storage > services (e.g. Ubuntu One). It may actually be preferable not to > autodelete files that have been removed from the drop_upload > directory. In the case that I accidentally rm -rf /drop_upload_dir/*, > I wouldn't want that to wipe out my online backups when tahoe gets > inotify'd. If this is to be a feature, perhaps it should default to > off? It would indeed default to off, since we don't want the behaviour to change surprisingly from the current behaviour on a Tahoe upgrade.
warner removed the
code-frontend
label 2014-12-02 19:49:06 +00:00
daira commented 2015-06-01 16:11:09 +00:00
Author
Owner

Add magic-folder keyword to all drop-upload tickets.

Add magic-folder keyword to all drop-upload tickets.
daira commented 2015-06-01 16:21:02 +00:00
Author
Owner

The intended behaviour for Magic Folder is described at source:docs/proposed/magic-folder/remote-to-local-sync.rst#deletion-of-a-file.

The intended behaviour for Magic Folder is described at source:docs/proposed/magic-folder/remote-to-local-sync.rst#deletion-of-a-file.
tahoe-lafs modified the milestone from undecided to 1.11.0 2015-06-01 16:21:02 +00:00
tahoe-lafs changed title from drop-upload: optionally unlink locally deleted files from upload directory to Magic Folder: implement local deletion of files as described in "Air Dragons" section of design doc 2015-06-01 16:21:02 +00:00
tahoe-lafs changed title from Magic Folder: implement local deletion of files as described in "Air Dragons" section of design doc to Magic Folder: implement local deletion of files/directories as described in "Air Dragons" section of design doc 2015-06-01 16:29:39 +00:00
tahoe-lafs changed title from Magic Folder: implement local deletion of files/directories as described in "Air Dragons" section of design doc to Magic Folder: implement local deletion of files/directories as described in "Water Dragons" section of design doc 2015-06-01 16:30:12 +00:00
dawuud commented 2015-06-10 02:04:15 +00:00
Author
Owner

I 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 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...
dawuud commented 2015-06-10 21:49:32 +00:00
Author
Owner

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...

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...
dawuud commented 2015-06-11 22:03:00 +00:00
Author
Owner

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

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>
dawuud commented 2015-06-24 23:53:07 +00:00
Author
Owner

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

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
dawuud commented 2015-06-25 21:35:59 +00:00
Author
Owner
cleaned up duplicate code here... <https://github.com/david415/tahoe-lafs/commit/6775c38f3eb40c494d7a06e5fa9dfee97f286507>
dawuud commented 2015-06-27 03:27:08 +00:00
Author
Owner

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'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>
dawuud commented 2015-06-29 21:03:49 +00:00
Author
Owner

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 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>
dawuud commented 2015-07-02 04:24:47 +00:00
Author
Owner

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

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
dawuud commented 2015-07-02 18:55:35 +00:00
Author
Owner

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

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>
dawuud commented 2015-07-03 01:23:13 +00:00
Author
Owner

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

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>
dawuud commented 2015-07-06 20:50:21 +00:00
Author
Owner
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>
daira commented 2015-07-20 23:54:44 +00:00
Author
Owner
Rebased to <https://github.com/tahoe-lafs/tahoe-lafs/commits/1710.test-alice-and-bob.3>.
dawuud commented 2015-07-31 23:23:39 +00:00
Author
Owner

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.

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.
dawuud commented 2015-08-20 11:11:46 +00:00
Author
Owner

more progress in my dev branch... i got rid of the live-lock... however the test exits with a dirty reactor.

more progress in my dev branch... i got rid of the live-lock... however the test exits with a dirty reactor.
daira commented 2015-08-20 16:10:25 +00:00
Author
Owner

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.

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>.
dawuud commented 2015-08-24 11:29:54 +00:00
Author
Owner

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

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>
daira commented 2015-08-25 20:16:01 +00:00
Author
Owner

After our Tuesday pairing, tests now pass on that branch.

After our Tuesday pairing, tests now pass on that branch.
dawuud commented 2015-08-28 12:47:03 +00:00
Author
Owner

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."""

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."""
daira commented 2015-08-31 12:19:33 +00:00
Author
Owner

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.

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.
dawuud commented 2015-08-31 21:41:01 +00:00
Author
Owner

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 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>
dawuud commented 2015-09-02 18:15:43 +00:00
Author
Owner

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

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>
tahoe-lafs changed title from Magic Folder: implement local deletion of files/directories as described in "Water Dragons" section of design doc to Magic Folder: implement "Water Dragons" section of design doc 2015-09-16 14:18:24 +00:00
dawuud commented 2015-09-21 08:52:46 +00:00
Author
Owner

Daira 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 method

Both the Uploader and Downloader need to know the latest collective
remote file...
"""

https://github.com/david415/tahoe-lafs/commit/6e8728ca0fd6a0186648bb64e187e999255f7bf3

Daira 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` method Both the Uploader and Downloader need to know the latest collective remote file... """ <https://github.com/david415/tahoe-lafs/commit/6e8728ca0fd6a0186648bb64e187e999255f7bf3>
daira commented 2015-10-02 21:15:06 +00:00
Author
Owner

Replying to dawuud:

Daira pointed out yet another bug due to our Uploader / Downloader code refactor;
i made a fix commit here:
https://github.com/david415/tahoe-lafs/commit/6e8728ca0fd6a0186648bb64e187e999255f7bf3

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.

Replying to [dawuud](/tahoe-lafs/trac-2024-07-25/issues/1710#issuecomment-88073): > Daira pointed out yet another bug due to our Uploader / Downloader code refactor; > i made a fix commit here: > <https://github.com/david415/tahoe-lafs/commit/6e8728ca0fd6a0186648bb64e187e999255f7bf3> 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.
dawuud commented 2015-10-05 12:02:57 +00:00
Author
Owner

I think we can close this ticket now that all of the deletion propagation features have been implemented.

Can we close it?

I think we can close this ticket now that all of the deletion propagation features have been implemented. Can we close it?
daira commented 2015-10-06 00:11:07 +00:00
Author
Owner

We're close, but I need to review whether all of the Water Dragons features have been implemented.

We're close, but I need to review whether all of the Water Dragons features have been implemented.
dawuud commented 2015-10-15 10:58:19 +00:00
Author
Owner

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.

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.
daira commented 2015-10-15 13:08:33 +00:00
Author
Owner

(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.

(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.
Owner

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?)

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?)
daira commented 2015-10-19 13:39:10 +00:00
Author
Owner

I'm reviewing that branch now.

I'm reviewing that branch now.
daira commented 2015-10-19 17:31:34 +00:00
Author
Owner

My current work-in-progress is at 2521.magic-db-schema.1.wip .

My current work-in-progress is at 2521.magic-db-schema.1.wip .
Owner

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 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.
Owner

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 :)

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 commented 2015-10-24 22:21:53 +00:00
Author
Owner

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 database
daira: the right fix is probably to change get_pathinfo to return PathInfo(isdir=False, isfile=False, islink=False, exists=False, size=None, mtime=None, ctime=None) for the ENOENT case
daira: and make sure that's also what is written to the db when the downloader detects a deleted file

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 database daira: the right fix is probably to change `get_pathinfo` to return `PathInfo(isdir=False, isfile=False, islink=False, exists=False, size=None, mtime=None, ctime=None)` for the `ENOENT` case daira: and make sure that's also what is written to the db when the downloader detects a deleted file
daira commented 2015-10-24 22:22:21 +00:00
Author
Owner

(None is converted to an SQL NULL.)

(`None` is converted to an SQL NULL.)
daira commented 2015-10-24 22:25:21 +00:00
Author
Owner

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.

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.
Owner

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.

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.
daira commented 2015-10-24 23:04:46 +00:00
Author
Owner

Replying to meejah:

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"

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.)

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.

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.

Replying to [meejah](/tahoe-lafs/trac-2024-07-25/issues/1710#issuecomment-88087): > 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" 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](/tahoe-lafs/trac-2024-07-25/issues/1710#issuecomment-88084). (The IN_MOVED_TO event should also be ignored because it's for an ignorable filename pattern.) > 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. 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.
daira commented 2015-10-24 23:23:34 +00:00
Author
Owner

I will update the design doc tomorrow to clarify what should happen.

I will update the design doc tomorrow to clarify what should happen.
daira commented 2015-10-25 13:23:05 +00:00
Author
Owner

Replying to daira:

I will update the design doc tomorrow to clarify what should happen.

See https://github.com/tahoe-lafs/tahoe-lafs/commit/5d2365f6c4cc388c2d74e43b2c1b7e8fc4c4ab86 (the previous three commits to the design doc are also worth reading).

Replying to [daira](/tahoe-lafs/trac-2024-07-25/issues/1710#issuecomment-88089): > I will update the design doc tomorrow to clarify what should happen. See <https://github.com/tahoe-lafs/tahoe-lafs/commit/5d2365f6c4cc388c2d74e43b2c1b7e8fc4c4ab86> (the previous three commits to the design doc are also worth reading).
daira commented 2015-10-25 13:33:33 +00:00
Author
Owner

So, I believe the fix is just to add this before the last line of MagicFolderDB.is_new_file:

if not pathinfo.exists and row[0] is None:
    return False
So, I believe the fix is just to add this before the last line of `MagicFolderDB.is_new_file`: ``` if not pathinfo.exists and row[0] is None: return False ```
daira commented 2015-12-08 15:13:56 +00:00
Author
Owner

Replying to daira:

So, I believe the fix is just to add this before the last line of MagicFolderDB.is_new_file:

if not pathinfo.exists and row[0] is None:
    return False

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.

Replying to [daira](/tahoe-lafs/trac-2024-07-25/issues/1710#issuecomment-88091): > So, I believe the fix is just to add this before the last line of `MagicFolderDB.is_new_file`: > ``` > if not pathinfo.exists and row[0] is None: > return False > ``` 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.
tahoe-lafs added the
fixed
label 2015-12-08 15:13:56 +00:00
daira closed this issue 2015-12-08 15:13:56 +00:00

Milestone renamed

Milestone renamed
warner modified the milestone from 1.11.0 to 1.12.0 2016-03-22 05:02:52 +00:00
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Reference: tahoe-lafs/trac-2024-07-25#1710
No description provided.