#2640 closed defect (fixed)

Magic Folder: upload/download loop

Reported by: meejah Owned by: daira
Priority: normal Milestone: undecided
Component: code-frontend-magic-folder Version: 1.10.2
Keywords: magic-folder integration-test Cc:
Launchpad Bug:


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

Change History (13)

comment:1 Changed at 2015-12-15T08:45:49Z by 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.

comment:2 Changed at 2015-12-15T16:30:19Z by 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.)

comment:3 Changed at 2015-12-15T16:32:08Z by 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.

comment:4 Changed at 2015-12-15T16:38:47Z by 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.

comment:5 Changed at 2015-12-15T20:08:23Z by 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 ...

comment:6 Changed at 2015-12-15T23:48:45Z by 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

comment:7 follow-up: Changed at 2015-12-17T09:16:56Z by dawuud

Yes that sounds right; remove the UNIQUE constraint.

comment:8 in reply to: ↑ 7 Changed at 2015-12-18T14:48:00Z by daira

Replying to dawuud:

Yes that sounds right; remove the UNIQUE constraint.


comment:9 Changed at 2015-12-28T20:41:22Z by daira

  • Status changed from new to assigned
  • Summary changed from upload/download loop to Magic Folder: upload/download loop

comment:10 Changed at 2016-01-01T00:45:12Z by daira

  • Owner changed from daira to dawuud
  • Status changed from assigned to new

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

comment:11 Changed at 2016-01-14T12:39:38Z by 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

comment:12 Changed at 2016-01-14T12:40:05Z by dawuud

  • Owner changed from dawuud to daira

comment:13 Changed at 2016-01-15T18:43:59Z by daira

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.