diff --git a/Annex/Drop.hs b/Annex/Drop.hs index 791273d8e8..aabc1c31fd 100644 --- a/Annex/Drop.hs +++ b/Annex/Drop.hs @@ -30,7 +30,10 @@ type Reason = String - The UUIDs are ones where the content is believed to be present. - The Remote list can include other remotes that do not have the content; - only ones that match the UUIDs will be dropped from. - - If allowed to drop fromhere, that drop will be tried first. + - + - If allowed to drop fromhere, that drop will be done last. This is done + - because local drops do not need any LockedCopy evidence, and so dropping + - from local last allows the content to be removed from more remotes. - - A VerifiedCopy can be provided as an optimisation when eg, a key - has just been uploaded to a remote. @@ -52,8 +55,8 @@ handleDropsFrom locs rs reason fromhere key afile preverified runner = do , return $ maybeToList afile ) n <- getcopies fs - if fromhere && checkcopies n Nothing - then go fs rs =<< dropl fs n + void $ if fromhere && checkcopies n Nothing + then go fs rs n >>= dropl fs else go fs rs n where getcopies fs = do @@ -78,12 +81,12 @@ handleDropsFrom locs rs reason fromhere key afile preverified runner = do | S.member u untrusted = v | otherwise = decrcopies v Nothing - go _ [] _ = noop + go _ [] n = pure n go fs (r:rest) n | uuid r `S.notMember` slocs = go fs rest n | checkcopies n (Just $ Remote.uuid r) = dropr fs r n >>= go fs rest - | otherwise = noop + | otherwise = pure n checkdrop fs n u a | null fs = check $ -- no associated files; unused content diff --git a/debian/changelog b/debian/changelog index 1f336b9ec8..a0011687db 100644 --- a/debian/changelog +++ b/debian/changelog @@ -14,6 +14,9 @@ git-annex (5.20150931) UNRELEASED; urgency=medium - If a git remote has too old a version of git-annex-shell installed, git-annex won't trust it to hold onto a copy of a file when dropping that file from some other remote. + * Changed drop ordering when using git annex sync --content or the + assistant, to drop from remotes first and from the local repo last. + This works better with the behavior changes to drop in many cases. * Do verification of checksums of annex objects downloaded from remotes. * When annex objects are received into git repositories from other git repos, their checksums are verified then too. @@ -36,6 +39,7 @@ git-annex (5.20150931) UNRELEASED; urgency=medium and stop recommending bittornado | bittorrent. * Debian: Remove dependency on transformers library, as it is now included in ghc. + * -- Joey Hess Thu, 01 Oct 2015 12:42:56 -0400 diff --git a/doc/bugs/concurrent_drop--from_presence_checking_failures.mdwn b/doc/bugs/concurrent_drop--from_presence_checking_failures.mdwn index 44d73b1dfc..cb3d34055d 100644 --- a/doc/bugs/concurrent_drop--from_presence_checking_failures.mdwn +++ b/doc/bugs/concurrent_drop--from_presence_checking_failures.mdwn @@ -367,3 +367,32 @@ for 1 locked copy when dropping from a remote, and so is sufficent to fix the bug. > done + +# drop ordering + +Consider the network: `T -- C --- (B1,B2)` + +When numcopies is 2, a file could start on T, get copied to C, and then on +the B1 and B2. At which point, it can be dropped from C and T (if +unwanted). + +Currently, the assistant and sync --content drop from the local repo 1st, +and then from remotes. Before the changes to fix this bug, that worked; +the content got removed from T and then from C, using the copies on B1 and +B2 as evidence. Now though, if B1 and B2 are special remotes, once the copy +is dropped from C, there is no locked copy available on (B1,B2), so the +subsequent drop from T fails. + +Changing the drop order lets C lock its own copy in order to +drop from T. then the local drop from C proceeds successfully without locking, +as local drops don't need locking. + +Course, there is a behavior change here.. Before, if B2 didn't exist, +the content would reach B2 and then be dropped from C, and then with only 2 +copies, it could not also be dropped from T. If the drop order changes, the +content is instead dropped from T and left on C. + +The new behavior seems better when T is a transfer remote, but perhaps not in +other cases. + +> implemented