Magic Folder: upload/download loop

[meejah]

I can "fairly often" get the smoketest case into a situation where Bob (e.g.) decides to upload a file that he only ever got from Alice, which then causes Alice to re-download it

The way I have repeated it is as follows: I let the smoketest case run through, and immediately copy a large (1.2GiB in this case, the Tails ISO image) file into Alice's magic-folder. Sometimes I need to copy a second or third one (or simply re-copy the first, or "touch" it) causing new versions to be created (and subsequently downloaded by Bob). Then, sometimes, Bob's uploader will decide to upload that same file -- so then when this "new" version is posted to his DMD, Alice downloads it (giving her uploader a chance at hitting this, too).

I don't yet have "a way" to repeat this reliably...

[meejah]

Okay, I believe this is happening because the time we set the downloaded file to is sometimes slightly different than the timestamp that's immediately written in the database, causing the uploader's "is_new_file" check to return true -- a "maybe upload" gets queued from the inotify we get after renaming the downloaded file.

[daira]

Ah ok. That's not supposed to happen because we're supposed to ​set the `mtime` of the downloaded temporary file and record the same mtime in the database:

Note that, if there is no conflict, the entry for foo recorded in the magic folder db will reflect the mtime set in step 3 [of the overwrite algorithm]. The move operation in step 4b will cause a MOVED_FROM event for foo, and the link operation in step 4c will cause an IN_CREATE event for foo. However, these events will not trigger an upload, because they are guaranteed to be processed only after the file replacement has finished, at which point the metadata recorded in the database entry will exactly match the metadata for the file's inode on disk. (The two hard links — foo and, while it still exists, .foo.tmp — share the same inode and therefore the same metadata.)

[daira]

Actually, there might be a problem with the specified algorithm if the filesystem is imprecise in setting/storing the mtime. So we might need to stat the temporary file after setting that time, and store the result in the database.

[meejah]

Well, that's more-or-less what it looks like the code is doing -- we set the times (minus some FUDGE_SECONDS value, whose use I don't understand) in _write_downloaded_file and then re-read it (in do_updated_db) but it seems that at least "sometimes" these don't match.

Note that we record both mtime *and* ctime, and if either of these don't match we consider the file new...in the logs I'm looking at it appears it's the ctime mismatching that is causing this.

[meejah]

00:03 < meejah> hmm, okay i think i've got it 00:03 < meejah> i was simply copying the same large file to different names in the magic-folder to get multiple uploads 00:07 < meejah> however, both last_downloaded_uri and last_uploaded_uri are marked UNIQUE in the database, but it seems if you

have the very same content, the CHK uri is the same if the content was the same

00:08 < meejah> so, i *think* we just want to remove UNIQUE for those. but, i don't know why they were marked unique in the

first place ...

[meejah]

There is a good way to repeat this, using the setup from the smoketest:

1. copy a file X into Alice's magic-folder as file0 (wait for sync to bob)
2. copy the same file X into Alice's magic-folder as file1 (wait for sync to bob)
3. touch or re-copy X over top of file0

[dawuud]

Yes that sounds right; remove the UNIQUE constraint.

[daira]

Replying to dawuud:

Yes that sounds right; remove the UNIQUE constraint.

+1

[daira]

I won't have time to do this before going to Berlin.

[dawuud]

fixed here: ​https://github.com/david415/tahoe-lafs/tree/2640.fix-upload-download-loop.0

please merge it into a stable branch after reviewing this super simple code change