git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* git fsck does not check the packed-refs file
@ 2024-01-18  8:02 R. Diez
  2024-01-18 11:15 ` Patrick Steinhardt
  0 siblings, 1 reply; 3+ messages in thread
From: R. Diez @ 2024-01-18  8:02 UTC (permalink / raw)
  To: git

Hi all:

I have been hit by an unfortunate system problem, and as a result, a few files in my Git repository got corrupted on my last git push. Some random blocks of bytes were overwritten with binary zeros, so I started getting weird unpacking errors etc.

It took a while to realise what the problem was. During my investigation, I ran "git fsck", which reported no problems, and then "git push" failed.

One of the very few corrupted files was packed-refs. This is a text file, so it was easy to compare it and see the corrupting binary zeros. But that made me wonder what "git fsck" checks.

I am guessing that "git fsck" does not check file packed-refs at all. I mean, it does not even attempt to parse it, in order to check whether at least the format makes any sense. Only "git push" does it.

What other parts of the repository does "git fsck" not check then?

The repository check is suspiciously fast. Is there a slow way to check that a repository is fine? I mean, something along the lines of checking whether every commit can be checked out without problems.

Best regards,
   rdiez

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: git fsck does not check the packed-refs file
  2024-01-18  8:02 git fsck does not check the packed-refs file R. Diez
@ 2024-01-18 11:15 ` Patrick Steinhardt
  2024-01-20  1:00   ` Jeff King
  0 siblings, 1 reply; 3+ messages in thread
From: Patrick Steinhardt @ 2024-01-18 11:15 UTC (permalink / raw)
  To: R. Diez; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 3164 bytes --]

On Thu, Jan 18, 2024 at 09:02:30AM +0100, R. Diez wrote:
> Hi all:
> 
> I have been hit by an unfortunate system problem, and as a result, a
> few files in my Git repository got corrupted on my last git push. Some
> random blocks of bytes were overwritten with binary zeros, so I
> started getting weird unpacking errors etc.
> 
> It took a while to realise what the problem was. During my
> investigation, I ran "git fsck", which reported no problems, and then
> "git push" failed.
> 
> One of the very few corrupted files was packed-refs. This is a text
> file, so it was easy to compare it and see the corrupting binary
> zeros. But that made me wonder what "git fsck" checks.

Can you maybe expand a bit on how you arrived at this bug? Was this a
hard crash of the system that corrupted the repository or rather
something like actual disk corruption?

I'm mostly asking because I have been fixing some sources of refdb
corruption:

  - bc22d845c4 (core.fsync: new option to harden references, 2022-03-11)
    started to fsync loose refs to disk before renaming them into place,
    released with Git v2.36.

  - ce54672f9b (refs: fix corruption by not correctly syncing
    packed-refs to disk, 2022-12-20) started to sync packed-refs to disk
    before renaming them into place, released with Git v2.40 and
    backported to Git v2.39.3.

So if:

  - you use a journaling filesystem,

  - you didn't disable `core.fsync`,

  - you use Git v2.40 or newer,

then you should in theory not run into any refdb corruption anymore. At
least we didn't experience corruption anymore at GitLab.com, whereas
before we encountered corruption every so often.

> I am guessing that "git fsck" does not check file packed-refs at all.
> I mean, it does not even attempt to parse it, in order to check
> whether at least the format makes any sense. Only "git push" does it.

Indeed it doesn't. While the issue is comparatively easy to spot by
manually inspecting the `packed-refs` file, I agree that it would be
great if git-fsck(1) knew how to check the refdb for consistency. This
problem is only going to get worse once the upcoming reftable backend
lands -- it is a binary format, and just opening it with a text editor
to check whether it looks sane-ish stops being a viable option here.

In fact, I already planned to introduce such consistency checks for the
refdb soonish. Once the reftable backend is upstream I will focus more
on additional tooling to support it, and extending our consistency
checks is one of the first items on my todo list here.

> What other parts of the repository does "git fsck" not check then?

There may be some metadata and cache-like data structures that we don't
check, but the object database is checked by default.

> The repository check is suspiciously fast. Is there a slow way to
> check that a repository is fine? I mean, something along the lines of
> checking whether every commit can be checked out without problems.

Other than running `git fsck --full --strict`: not that I'm aware of.
And `--full` isn't even needed because it's the default.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: git fsck does not check the packed-refs file
  2024-01-18 11:15 ` Patrick Steinhardt
@ 2024-01-20  1:00   ` Jeff King
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff King @ 2024-01-20  1:00 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: R. Diez, git

On Thu, Jan 18, 2024 at 12:15:08PM +0100, Patrick Steinhardt wrote:

> > I am guessing that "git fsck" does not check file packed-refs at all.
> > I mean, it does not even attempt to parse it, in order to check
> > whether at least the format makes any sense. Only "git push" does it.
> 
> Indeed it doesn't. While the issue is comparatively easy to spot by
> manually inspecting the `packed-refs` file, I agree that it would be
> great if git-fsck(1) knew how to check the refdb for consistency. This
> problem is only going to get worse once the upcoming reftable backend
> lands -- it is a binary format, and just opening it with a text editor
> to check whether it looks sane-ish stops being a viable option here.

We don't check the packed-refs file explicitly, but we do open and parse
it to iterate over the refs it contains. E.g.:

  $ git init
  $ echo foo >.git/packed-refs
  $ git fsck
  Checking object directories: 100% (256/256), done.
  fatal: unexpected line in .git/packed-refs: foo

It's quite possible that the reading code could be more careful. I'd
have to see the exact corruption that "git fsck" didn't complain about
to say more.  If there's a page full of NUL bytes at the end of the
file, I wouldn't be surprised if the reading code gently ignores that,
which obviously is not ideal.

Fundamentally we cannot catch all cases here; a simple truncation, for
example, might yield a valid file that is simply missing some entries.
Unlike objects (which make promises about reachability and so on), there
is no real "consistency" for the state of the refs. But probably warning
if saw a bunch of garbage in the file is a good thing.

I also agree that a specific refdb consistency check would be valuable.
There are some things that the regular reading code will not check, but
which an fsck should (e.g., if the packed-refs file claims to have the
"sorted" trait, we should confirm that).

-Peff

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-01-20  1:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-18  8:02 git fsck does not check the packed-refs file R. Diez
2024-01-18 11:15 ` Patrick Steinhardt
2024-01-20  1:00   ` Jeff King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).