fsck: Detect and correct stale or missing inode caches for object files

An easy way to see this in action is to have an unlocked file, and touch the
object file.

While all code that compares inode caches for object files needs to be
prepared for this kind of problem and fall back to verification, having
fsck notice it and correct it is cheap (as long as fsck is being run
anyway) and ensures that if it happens for some unusual reason, there's a
way for the user to notice that it's happening.

Not that, when annex.thin is in use, the earlier call to isUnmodified
(and also potentially earlier calls to inAnnex in eg, verifyLocationLog)
will fix up the same problem silently. That might prevent the warning
being displayed, although probably it still will be, because the
Database.Keys write of the InodeCache will be queued but will not have
happened yet. I can't see a way to improve this, but it's not great.

Sponsored-by: Dartmouth College's Datalad project

diff --git a/CHANGELOG b/CHANGELOG
index 62083aa26c0ca264040e25beca0013f9cc156996..aa0e3a79b846c75415eec252770ad0ab988345d7 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -21,6 +21,7 @@ git-annex (8.20210715) UNRELEASED; urgency=medium
     changed.
   * Fix bug that could prevent pointer files from being populated,
     in a repository that was upgraded from v7.
+  * fsck: Detect and correct stale or missing inode caches.
 
  -- Joey Hess <id@joeyh.name>  Wed, 14 Jul 2021 14:26:36 -0400
 

diff --git a/Command/Fsck.hs b/Command/Fsck.hs
index c735b7a4975e493b16b44f406b78934e16edd5ae..1e2ea3801cd7e146775f8682c8800f6f621aaacd 100644 (file)
--- a/Command/Fsck.hs
+++ b/Command/Fsck.hs
@@ -1,6 +1,6 @@
 {- git-annex command
  -
- - Copyright 2010-2020 Joey Hess <id@joeyh.name>
+ - Copyright 2010-2021 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU AGPL version 3 or higher.
  -}
@@ -16,6 +16,7 @@ import qualified Remote
 import qualified Types.Backend
 import qualified Backend
 import Annex.Content
+import Annex.Content.Presence.LowLevel
 import Annex.Perms
 import Annex.Link
 import Logs.Location
@@ -31,6 +32,8 @@ import Utility.HumanTime
 import Utility.CopyFile
 import Git.FilePath
 import Utility.PID
+import Utility.InodeCache
+import Annex.InodeSentinal
 import qualified Database.Keys
 import qualified Database.Fsck as FsckDb
 import Types.CleanupActions
@@ -446,7 +449,14 @@ checkBackend backend key keystatus afile = do
        content <- calcRepo (gitAnnexLocation key)
        ifM (pure (isKeyUnlockedThin keystatus) <&&> (not <$> isUnmodified key content))
                ( nocheck
-               , checkBackendOr badContent backend key content ai
+               , do
+                       mic <- withTSDelta (liftIO . genInodeCache content)
+                       ifM (checkBackendOr badContent backend key content ai)
+                               ( do
+                                       checkInodeCache key content mic ai
+                                       return True
+                               , return False
+                               )
                )
   where
        nocheck = return True
@@ -472,6 +482,32 @@ checkBackendOr bad backend key file ai =
                        return ok
                Nothing -> return True
 
+{- Check, if there are InodeCaches recorded for a key, that one of them
+ - matches the object file. There are situations where the InodeCache
+ - of the object file does not get recorded, including a v8 upgrade.
+ - There may also be situations where the wrong InodeCache is recorded,
+ - if inodes are not stable.
+ -
+ - This must be called after the content of the object file has been
+ - verified to be correct. The InodeCache is generated again to detect if
+ - the object file was changed while the content was being verified.
+ -}
+checkInodeCache :: Key -> RawFilePath -> Maybe InodeCache -> ActionItem -> Annex ()
+checkInodeCache key content mic ai = case mic of
+       Nothing -> noop
+       Just ic -> do
+               ics <- Database.Keys.getInodeCaches key
+               unless (null ics) $
+                       unlessM (isUnmodifiedCheapLowLevel ic ics) $ do
+                               withTSDelta (liftIO . genInodeCache content) >>= \case
+                                       Nothing -> noop
+                                       Just ic' -> whenM (compareInodeCaches ic ic') $ do
+                                               warning $ concat
+                                                       [ decodeBS' (actionItemDesc ai)
+                                                       , ": Stale or missing inode cache; updating."
+                                                       ]
+                                               Database.Keys.addInodeCaches key [ic]
+
 checkKeyNumCopies :: Key -> AssociatedFile -> NumCopies -> Annex Bool
 checkKeyNumCopies key afile numcopies = do
        let (desc, hasafile) = case afile of

diff --git a/doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_24_4345e2d7f30318dc7565e90ac8578086._comment b/doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_24_4345e2d7f30318dc7565e90ac8578086._comment
new file mode 100644 (file)
index 0000000..3457a8d
--- /dev/null
+++ b/doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_24_4345e2d7f30318dc7565e90ac8578086._comment
@@ -0,0 +1,19 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 24"""
+ date="2021-07-29T17:00:57Z"
+ content="""
+I agree, fsck should notice and correct this problem. The current fix
+papers over the problem it in a way that will prevent users noticing it.
+Fsck being noisy about it will help avoid sweeping it under the rug and
+forgetting about it. Implemented that.
+
+The performance impact here is relatively small. Aside from
+whenever this problem occurs, the extra checksumming will only happen when
+files are unlocked and the object file is modified. So annex.thin
+would need to be set and a file modified in a way that affects the object
+file. Several other parts of git-annex also checksum in that situation;
+that's a perf price you pay for using annex.thin. When this problem does
+occur, it will only checksum the object file once, and then will get its
+inode information cached.
+"""]]