From d2aead67bd17faac753ccf41964789a3a62ddaf3 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 29 Jul 2021 14:06:13 -0400 Subject: [PATCH] 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 --- CHANGELOG | 1 + Command/Fsck.hs | 40 ++++++++++++++++++- ..._4345e2d7f30318dc7565e90ac8578086._comment | 19 +++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_24_4345e2d7f30318dc7565e90ac8578086._comment diff --git a/CHANGELOG b/CHANGELOG index 62083aa26c..aa0e3a79b8 100644 --- 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 Wed, 14 Jul 2021 14:26:36 -0400 diff --git a/Command/Fsck.hs b/Command/Fsck.hs index c735b7a497..1e2ea3801c 100644 --- a/Command/Fsck.hs +++ b/Command/Fsck.hs @@ -1,6 +1,6 @@ {- git-annex command - - - Copyright 2010-2020 Joey Hess + - Copyright 2010-2021 Joey Hess - - 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 index 0000000000..3457a8d729 --- /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. +"""]]