fsck: handle packs removed by concurrent repacks - #2221
Open
newren wants to merge 3 commits into
Open
Conversation
"git fsck" enumerates packs up front and later verifies each pack's reverse index. If a concurrent "git repack" removes one of those packs in the meantime, load_pack_revindex_from_disk() fails and fsck reports "unable to load rev-index", implying corruption. That is misleading: the objects are safe in the replacement pack and a quiescent retry succeeds. When the load fails because the ".pack" itself is gone (ENOENT), say so and suggest retrying once maintenance completes, rather than blaming the rev-index. A failure with the pack still present is reported as before, so genuine corruption still surfaces. This can be provoked by looping "git repack -adq" in one process while another loops "git fsck --connectivity-only": occasionally fsck trips over a pack removed after it was enumerated, and now says the pack disappeared instead of reporting a bad rev-index. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
"git multi-pack-index verify" (which "git fsck" always spawns) walks the midx's packs, closing and reopening each by name as it checks object offsets. A concurrent "git repack" that unlinks a redundant pack in that window makes the reopen fail, and verify reports "failed to load pack in position N", "failed to load pack entry for oid[N]", or "failed to load pack-index for packfile ..." -- all of which read like midx corruption, though the objects are safe in the replacement pack and a quiescent retry succeeds. When such a failure is explained by a midx-referenced ".pack" having vanished (repack unlinks a redundant pack's ".idx" before its ".pack", so a missing ".pack" is the tell-tale), add a one-time hint to retry when quiescent. The existing per-failure messages are kept, so genuine corruption is still reported as before. To provoke it, "git multi-pack-index write" a repo with several packs and run "git multi-pack-index verify" while "git repack -adq" loops in the background; verify occasionally fails with one of the messages above, now followed by the retry hint. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
MIDX verification closes packs between object groups and later reopens them by name. A concurrent repack can remove one in that interval, causing verification to fail on an otherwise benign race. When the referenced packs fit within a conservative fd budget, open them before verifying offsets and keep them open through the walk. Most geometrically maintained MIDXes have only O(log N) packs; larger MIDX chains retain the close-as-we-go path and its race-aware diagnostic. Do not use do_not_close for this: find_lru_pack() ignores it, so close_one_pack() may still reclaim the fd under pressure. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
git fsckandgit multi-pack-index verifycan race with a concurrentrepack that removes a pack after it has been enumerated. The resulting
errors currently look like corrupt reverse indexes or MIDX entries even
though the objects have safely moved to a replacement pack and a
quiescent retry succeeds.
This series distinguishes pack removal from genuine index corruption
and adds a retry hint when a concurrent repack explains the failure. It
also pins MIDX-referenced packs during verification when they fit within
a conservative file-descriptor budget, eliminating the usual race for
geometrically maintained repositories while preserving the existing
close-as-we-go behavior for unusually large pack sets.