Skip to content

Auto-recover corrupt packfiles in packfile maintenance - #2052

Open
tyrielv wants to merge 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/packfile-corruption-recovery
Open

Auto-recover corrupt packfiles in packfile maintenance#2052
tyrielv wants to merge 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/packfile-corruption-recovery

Conversation

@tyrielv

@tyrielv tyrielv commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Independent change, based on master. Related to #2048 (both stabilize the same corrupt-packfile scenario) but has no code dependency on it. For maximum robustness, #2048 (which bounds git output) ideally lands first so recovery can't OOM on a very noisy stderr mid-recovery, but the two can merge in either order.

Summary

When a packfile in the shared object cache is corrupt/truncated (e.g. from a past disk-full event), git multi-pack-index write/verify fails with could not load pack N. The existing self-heal only deletes+rewrites the multi-pack-index (MIDX), which does not fix the underlying pack — the rewrite re-scans the same bad pack and keeps failing, so the corruption recurs indefinitely.

Change

PackfileMaintenanceStep routes write/verify failures through a recovery path. When git reports a pack-load failure:

  • Detection (always runs, even with recovery disabled): verifies each pack with git verify-pack and reports every unreadable pack via telemetry (Operation=FoundCorruptPack). The N in could not load pack N is an internal MIDX position, not a filename, so per-pack verification is how we find the actual bad file.
  • Removal (gated — see kill switch): deletes each corrupt pack's files (.pack/.idx/.keep/.rev; Operation=DeletedCorruptPack), then deletes+regenerates the MIDX from the packs that remain (fast path, no full repack). Missing objects are re-fetched on demand.
  • Corrupt prefetch pack (special case): prefetch packs are incremental/ordered by timestamp, so a corrupt one invalidates every later prefetch pack too. Recovery removes the corrupt prefetch pack and every later prefetch pack (Operation=DeletedHealthyPrefetchPack for the healthy ones removed purely due to ordering), then requests a prefetch (via a callback GitMaintenanceScheduler wires to a PrefetchStep, only when using a cache server) to re-download them and rebuild the commit-graph.

Kill switch

The destructive pack removal is gated by a new git config gvfs.enable-packfile-recovery (default true). When false, GVFS still detects and reports corrupt packs (FoundCorruptPackCorruptPackRecoverySkipped) but deletes nothing and requests no prefetch; the non-destructive MIDX rewrite still runs, so behavior degrades to today's. This is a field kill switch — no redeploy needed if the destructive path ever misbehaves.

Telemetry

Distinct, queryable Operation values so rollout can be watched: FoundCorruptPack (even when disabled), CorruptPackRecoverySkipped, DeletedCorruptPack, DeletedHealthyPrefetchPack, PrefetchRestoreUnavailable.

Tests

  • A verify failure reporting a pack-load error removes the corrupt pack and rewrites the MIDX from the remaining good packs (recovery enabled).
  • With recovery disabled, the same failure still verifies each pack and reports the corrupt one but deletes nothing.
  • A corrupt prefetch pack removes it and every later prefetch pack, keeps the earlier healthy one, and requests a prefetch.

Full unit suite: 885 passed, 0 failed (11 pre-existing native-hook skips). The pre-existing PackfileMaintenanceRewriteOnBadVerify (empty-stderr verify failure → MIDX rewrite only, no pack detection) is unchanged.

Follow-up

A functional test that corrupts a real pack and asserts the mount self-heals (rather than looping) is tracked for the streaming follow-up PR.

@tyrielv
tyrielv force-pushed the tyrielv/packfile-corruption-recovery branch from cacf77d to 284fdb5 Compare July 9, 2026 21:22
@tyrielv
tyrielv marked this pull request as ready for review July 9, 2026 22:00
Comment thread GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs Outdated
Comment thread GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs
@tyrielv
tyrielv force-pushed the tyrielv/packfile-corruption-recovery branch from 284fdb5 to bcc2b74 Compare July 14, 2026 22:43
Comment thread GVFS/GVFS.Common/Git/GitProcess.cs
When a packfile in the shared object cache is corrupt or truncated (e.g. from a
past disk-full event), 'git multi-pack-index write/verify' fails with "could not
load pack N". The existing self-heal only deletes and rewrites the
multi-pack-index (MIDX), which does not fix the underlying pack: the rewrite
re-scans the same bad pack and keeps failing. The corruption then recurs
indefinitely.

PackfileMaintenanceStep now routes write/verify failures through a recovery path
that, when git reports a pack-load failure:

  - Detection (always runs, even with recovery disabled): verifies each pack in
    the object cache with 'git verify-pack' and reports every unreadable pack via
    telemetry (Operation=FoundCorruptPack). The "could not load pack N" ordinal is
    an internal MIDX position, not a filename, so per-pack verification is how we
    find the actual bad file.
  - Removal (gated, see kill switch below): deletes each corrupt pack's files
    (.pack/.idx/.keep/.rev; Operation=DeletedCorruptPack), then deletes and
    regenerates the MIDX from the packs that remain (fast path, no full repack).
    Missing objects are re-fetched on demand.
  - Corrupt prefetch pack (special case): prefetch packs are incremental and
    ordered by timestamp, so a corrupt one invalidates every later prefetch pack
    too - leaving a hole would let the newest surviving timestamp advance past it
    so a later prefetch never backfills the gap. Recovery removes the corrupt
    prefetch pack and every later prefetch pack
    (Operation=DeletedHealthyPrefetchPack for the healthy ones removed purely due
    to ordering), then requests a prefetch (via a callback GitMaintenanceScheduler
    wires to a PrefetchStep, only when using a cache server) to re-download them
    and rebuild the commit-graph.

Kill switch: the destructive pack removal is gated by a new git config,
gvfs.enable-packfile-recovery (default true). When false, GVFS still detects and
reports corrupt packs (Operation=FoundCorruptPack, then
CorruptPackRecoverySkipped) but deletes nothing and does not request a prefetch;
the non-destructive MIDX rewrite still runs, so behavior degrades to today's.
This gives a field kill switch without a redeploy if the destructive path ever
misbehaves.

This is stacked on the git-output bounding change: recovery runs additional git
commands (verify-pack, MIDX rewrites) against the corrupt repo, so it relies on
that change to keep a noisy stderr from OOM-ing the mount mid-recovery.

Review follow-ups:
  - prefetchRestoreNeeded is now set only after a corrupt prefetch pack is
    actually removed (RemovePackFileSet returns whether the .pack file was
    deleted), instead of as soon as one is detected. If deletion is blocked,
    the restore no longer runs while the corrupt pack is still present.
  - DetectAndRemoveCorruptPacks now remembers, for the lifetime of a single
    maintenance run, that it already reported corrupt packs with recovery
    disabled, and skips the redundant per-pack verify-pack rescan on later
    MIDX failures in that same run.
  - DetectAndRemoveCorruptPacks now parses the corrupt pack's filename directly
    out of the write/verify failure's stderr when git includes it (e.g.
    "packfile pack-1234.pack does not match index" / "wrong index v2 file size
    in pack-1234.idx"), and verifies only that candidate instead of every pack
    in the object cache. This only helps when git actually names the file,
    which it does for the verify-triggered failures this code mostly handles
    (not for the rarer write-path "could not load pack N", which is genuinely
    an unresolvable internal ordinal - confirmed by reading git's midx-write.c).
    Falls back to verifying every pack whenever no candidate can be parsed, or
    the parsed candidate turns out to be healthy, so detection is never less
    thorough than before.

Tests:
  - A verify failure reporting a pack-load error removes the corrupt pack and
    rewrites the MIDX from the remaining good packs (recovery enabled).
  - With recovery disabled, the same failure still verifies each pack and reports
    the corrupt one but deletes nothing.
  - A corrupt prefetch pack removes it and every later prefetch pack, keeps the
    earlier healthy one, and requests a prefetch.
  - A verify failure that names the corrupt pack directly verifies only that
    pack (fast path).
  - A verify failure that names a pack which turns out to be healthy falls back
    to verifying every pack (fallback path).

Assisted-by: Claude Sonnet 5
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv
tyrielv force-pushed the tyrielv/packfile-corruption-recovery branch from 490f99e to 9ec7abf Compare August 7, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants