diff --git a/GVFS/GVFS.Common/GVFSConstants.cs b/GVFS/GVFS.Common/GVFSConstants.cs index 8f135786a..6463fcd81 100644 --- a/GVFS/GVFS.Common/GVFSConstants.cs +++ b/GVFS/GVFS.Common/GVFSConstants.cs @@ -44,6 +44,13 @@ public static class GitConfig public const string TrustPackIndexes = GVFSPrefix + "trust-pack-indexes"; public const bool TrustPackIndexesDefault = true; + /* Kill switch for the destructive part of packfile-maintenance corruption recovery: when + * false, GVFS still detects and reports corrupt packs but does not delete them (or later + * prefetch packs) and does not request a restoring prefetch. Detection/telemetry is + * unaffected; the non-destructive multi-pack-index rewrite still runs. */ + public const string EnablePackfileRecovery = GVFSPrefix + "enable-packfile-recovery"; + public const bool EnablePackfileRecoveryDefault = true; + public const string ShowHydrationStatus = GVFSPrefix + "show-hydration-status"; public const bool ShowHydrationStatusDefault = false; diff --git a/GVFS/GVFS.Common/Git/GitProcess.cs b/GVFS/GVFS.Common/Git/GitProcess.cs index cf666bc64..d27d7a749 100644 --- a/GVFS/GVFS.Common/Git/GitProcess.cs +++ b/GVFS/GVFS.Common/Git/GitProcess.cs @@ -784,6 +784,17 @@ public Result VerifyMultiPackIndex(string objectDir) return this.InvokeGitAgainstDotGitFolder("-c core.multiPackIndex=true multi-pack-index verify --object-dir=\"" + objectDir + "\" --no-progress"); } + /// + /// Verifies the integrity of a single packfile via its .idx. Returns a failure exit code if the + /// pack is truncated or otherwise unreadable. Used by pack maintenance recovery to determine + /// which pack is corrupt - the "could not load pack N" ordinal reported by the multi-pack-index + /// is an internal position, not a filename, so it cannot be mapped to a file directly. + /// + public Result VerifyPack(string packIndexPath) + { + return this.InvokeGitAgainstDotGitFolder("verify-pack \"" + packIndexPath + "\""); + } + public Result RemoteAdd(string remoteName, string url) { return this.InvokeGitAgainstDotGitFolder("remote add " + remoteName + " " + url); diff --git a/GVFS/GVFS.Common/Maintenance/GitMaintenanceScheduler.cs b/GVFS/GVFS.Common/Maintenance/GitMaintenanceScheduler.cs index 760f80329..2759306ff 100644 --- a/GVFS/GVFS.Common/Maintenance/GitMaintenanceScheduler.cs +++ b/GVFS/GVFS.Common/Maintenance/GitMaintenanceScheduler.cs @@ -54,7 +54,9 @@ private void ScheduleRecurringSteps() return; } - if (this.gitObjects.IsUsingCacheServer()) + bool usingCacheServer = this.gitObjects.IsUsingCacheServer(); + + if (usingCacheServer) { TimeSpan prefetchPeriod = TimeSpan.FromMinutes(15); this.stepTimers.Add(new Timer( @@ -70,8 +72,16 @@ private void ScheduleRecurringSteps() dueTime: this.looseObjectsDueTime, period: this.looseObjectsPeriod)); + // When packfile-maintenance recovery removes a corrupt prefetch pack (and the later prefetch + // packs that depend on it), it needs a prefetch to re-download them and rebuild the + // commit-graph. This is only meaningful when a cache server is in use; otherwise the objects + // are restored on demand. + Action requestPrefetch = usingCacheServer + ? () => this.queue.TryEnqueue(new PrefetchStep(this.context, this.gitObjects)) + : (Action)null; + this.stepTimers.Add(new Timer( - (state) => this.queue.TryEnqueue(new PackfileMaintenanceStep(this.context)), + (state) => this.queue.TryEnqueue(new PackfileMaintenanceStep(this.context, requestPrefetch: requestPrefetch)), state: null, dueTime: this.packfileDueTime, period: this.packfilePeriod)); diff --git a/GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs b/GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs index a5fc5b54a..e51f11a2a 100644 --- a/GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs +++ b/GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Text.RegularExpressions; namespace GVFS.Common.Maintenance { @@ -27,21 +28,30 @@ namespace GVFS.Common.Maintenance public class PackfileMaintenanceStep : GitMaintenanceStep { public const string PackfileLastRunFileName = "pack-maintenance.time"; - public const string DefaultBatchSize = "2g"; + public const string DefaultBatchSize = "2g"; private const string MultiPackIndexLock = "multi-pack-index.lock"; private readonly bool forceRun; private readonly string batchSize; + private readonly Action requestPrefetch; + + // Set once corrupt packs have been detected and reported with recovery disabled. Recovery leaves + // the corrupt packs in place, so 'git multi-pack-index write/verify' keeps failing on them for + // the rest of this maintenance run - once reported, skip re-verifying every pack on each + // subsequent failure in the same run rather than repeating an identical, already-known result. + private bool reportedCorruptPacksWithRecoveryDisabled; public PackfileMaintenanceStep( GVFSContext context, bool requireObjectCacheLock = true, bool forceRun = false, string batchSize = DefaultBatchSize, - GitProcessChecker gitProcessChecker = null) + GitProcessChecker gitProcessChecker = null, + Action requestPrefetch = null) : base(context, requireObjectCacheLock, gitProcessChecker) { this.forceRun = forceRun; this.batchSize = batchSize; + this.requestPrefetch = requestPrefetch; } public override string Area => nameof(PackfileMaintenanceStep); @@ -116,41 +126,54 @@ protected override void PerformMaintenance() return; } - string multiPackIndexLockPath = Path.Combine(this.Context.Enlistment.GitPackRoot, MultiPackIndexLock); - this.Context.FileSystem.TryDeleteFile(multiPackIndexLockPath); - - this.RunGitCommand((process) => process.WriteMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.WriteMultiPackIndex)); - - // If a LibGit2Repo is active, then it may hold handles to the .idx and .pack files we want - // to delete during the 'git multi-pack-index expire' step. If one starts during the step, - // then it can still block those deletions, but we will clean them up in the next run. By - // running CloseActiveRepos() here, we ensure that we do not run twice with the same - // LibGit2Repo active across two calls. A "new" repo should not hold handles to .idx files - // that do not have corresponding .pack files, so we will clean them up in CleanStaleIdxFiles(). - this.Context.Repository.CloseActiveRepo(); - - GitProcess.Result expireResult = this.RunGitCommand((process) => process.MultiPackIndexExpire(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.MultiPackIndexExpire)); - - this.Context.Repository.OpenRepo(); - + string multiPackIndexLockPath = Path.Combine(this.Context.Enlistment.GitPackRoot, MultiPackIndexLock); + this.Context.FileSystem.TryDeleteFile(multiPackIndexLockPath); + + // Read the recovery kill switch while the repo is open. When disabled, we still detect and + // report corrupt packs but do not delete anything. + bool recoveryEnabled = this.IsPackfileRecoveryEnabled(); + + // A corrupt or truncated packfile in the shared object cache (e.g. introduced by a + // disk-full event) makes 'git multi-pack-index write' fail with "could not load pack N". + // The existing self-heal only ran after a later verify failed - but the write is first, + // so recover on the write path too rather than pressing on with a broken cache. + GitProcess.Result writeResult = this.RunGitCommand((process) => process.WriteMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.WriteMultiPackIndex)); + + if (!this.Stopping && writeResult.ExitCodeIsFailure) + { + this.RepairMultiPackIndex(activity, writeResult, recoveryEnabled); + } + + // If a LibGit2Repo is active, then it may hold handles to the .idx and .pack files we want + // to delete during the 'git multi-pack-index expire' step. If one starts during the step, + // then it can still block those deletions, but we will clean them up in the next run. By + // running CloseActiveRepos() here, we ensure that we do not run twice with the same + // LibGit2Repo active across two calls. A "new" repo should not hold handles to .idx files + // that do not have corresponding .pack files, so we will clean them up in CleanStaleIdxFiles(). + this.Context.Repository.CloseActiveRepo(); + + GitProcess.Result expireResult = this.RunGitCommand((process) => process.MultiPackIndexExpire(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.MultiPackIndexExpire)); + + this.Context.Repository.OpenRepo(); + List staleIdxFiles = this.CleanStaleIdxFiles(out int numDeletionBlocked); - this.GetPackFilesInfo(out int expireCount, out long expireSize, out hasKeep); - + this.GetPackFilesInfo(out int expireCount, out long expireSize, out hasKeep); + GitProcess.Result verifyAfterExpire = this.RunGitCommand((process) => process.VerifyMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.VerifyMultiPackIndex)); - if (!this.Stopping && verifyAfterExpire.ExitCodeIsFailure) - { - this.LogErrorAndRewriteMultiPackIndex(activity); + if (!this.Stopping && verifyAfterExpire.ExitCodeIsFailure) + { + this.RepairMultiPackIndex(activity, verifyAfterExpire, recoveryEnabled); } GitProcess.Result repackResult = this.RunGitCommand((process) => process.MultiPackIndexRepack(this.Context.Enlistment.GitObjectsRoot, this.batchSize), nameof(GitProcess.MultiPackIndexRepack)); - this.GetPackFilesInfo(out int afterCount, out long afterSize, out hasKeep); - - GitProcess.Result verifyAfterRepack = this.RunGitCommand((process) => process.VerifyMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.VerifyMultiPackIndex)); + this.GetPackFilesInfo(out int afterCount, out long afterSize, out hasKeep); - if (!this.Stopping && verifyAfterRepack.ExitCodeIsFailure) - { - this.LogErrorAndRewriteMultiPackIndex(activity); + GitProcess.Result verifyAfterRepack = this.RunGitCommand((process) => process.VerifyMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.VerifyMultiPackIndex)); + + if (!this.Stopping && verifyAfterRepack.ExitCodeIsFailure) + { + this.RepairMultiPackIndex(activity, verifyAfterRepack, recoveryEnabled); } EventMetadata metadata = new EventMetadata(); @@ -171,5 +194,392 @@ protected override void PerformMaintenance() this.SaveLastRunTimeToFile(); } } + + /// + /// Reads the gvfs.enable-packfile-recovery kill switch. Virtual so unit tests can + /// override it; the LibGit2 invoker is null in tests, in which case recovery defaults to enabled. + /// + protected virtual bool IsPackfileRecoveryEnabled() + { + LibGit2RepoInvoker repoInvoker = this.Context.Repository.LibGit2RepoInvoker; + if (repoInvoker == null) + { + return GVFSConstants.GitConfig.EnablePackfileRecoveryDefault; + } + + return repoInvoker.GetConfigBoolOrDefault( + GVFSConstants.GitConfig.EnablePackfileRecovery, + GVFSConstants.GitConfig.EnablePackfileRecoveryDefault); + } + + private static bool ResultIndicatesCorruptPack(GitProcess.Result result) { + string errors = result?.Errors; + if (string.IsNullOrEmpty(errors)) + { + return false; + } + + // 'git multi-pack-index write/verify' reports an unreadable underlying packfile with + // messages like "could not load pack N" or "failed to load pack in position N". Both mean + // a packfile - not just the multi-pack-index - is corrupt. + return errors.IndexOf("could not load pack", StringComparison.OrdinalIgnoreCase) >= 0 + || errors.IndexOf("failed to load pack", StringComparison.OrdinalIgnoreCase) >= 0; + } + + /// + /// Returns the prefetch timestamp encoded in a prefetch pack file name + /// (prefetch-<timestamp>-<uniqueId>.pack), or null if the file is not a prefetch pack. + /// + private static long? GetPrefetchTimestamp(string packFileName) + { + if (!packFileName.StartsWith(GVFSConstants.PrefetchPackPrefix, StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + string[] parts = packFileName.Split('-'); + if (parts.Length > 1 && long.TryParse(parts[1], out long timestamp)) + { + return timestamp; + } + + return null; + } + + /// + /// Recovers from a failed multi-pack-index write or verify. When git reports it could not load a + /// pack, a packfile itself is corrupt (e.g. truncated by a past disk-full event) and regenerating + /// the multi-pack-index (MIDX) alone keeps failing because the rewrite re-scans the same bad pack. + /// Detect the corrupt pack(s) - and, when recovery is enabled, remove them - then delete and + /// regenerate the MIDX from the packs that remain (fast path, no full repack). + /// + private void RepairMultiPackIndex(ITracer activity, GitProcess.Result failure, bool recoveryEnabled) + { + bool prefetchRestoreNeeded = false; + + if (!this.Stopping && ResultIndicatesCorruptPack(failure)) + { + this.DetectAndRemoveCorruptPacks(activity, recoveryEnabled, out prefetchRestoreNeeded, failure.Errors); + } + + // Delete the (now stale) multi-pack-index and rebuild it from the packs that remain. This is + // non-destructive and runs regardless of the recovery kill switch. + this.LogErrorAndRewriteMultiPackIndex(activity); + + if (prefetchRestoreNeeded && !this.Stopping) + { + this.RequestPrefetchRestore(activity); + } + } + + /// + /// Verifies each packfile in the object cache with 'git verify-pack' and reports every unreadable + /// pack via telemetry (this detection runs even when recovery is disabled). When + /// is true, also removes each corrupt pack's files + /// (.pack/.idx/.keep/.rev). A corrupt prefetch pack additionally forces removal of every + /// later (higher-timestamp) prefetch pack and sets , + /// because prefetch packs are incremental - leaving a hole would let the newest surviving + /// timestamp advance past it so a later prefetch never backfills the gap. + /// + // public only for unit tests + public void DetectAndRemoveCorruptPacks(ITracer activity, bool recoveryEnabled, out bool prefetchRestoreNeeded, string failureErrors = null) + { + prefetchRestoreNeeded = false; + + if (!recoveryEnabled && this.reportedCorruptPacksWithRecoveryDisabled) + { + // Already verified every pack and reported the corrupt ones earlier in this maintenance + // run. Recovery is disabled, so nothing has changed on disk - skip the redundant rescan. + return; + } + + List packDirContents = this.Context + .FileSystem + .ItemsInDirectory(this.Context.Enlistment.GitPackRoot) + .ToList(); + + // Phase 1 - detection (read-only, always runs). git's own 'multi-pack-index verify' failure + // text usually already names the specific unreadable pack (e.g. "packfile pack-1234.pack does + // not match index"), so try verifying just those named packs first - much faster than + // verify-pack'ing every pack in the object cache. Fall back to the full scan whenever no + // candidate can be parsed, or when every candidate turns out to verify successfully (the + // write/verify failure that got us here must then be explained by some other pack). + HashSet candidateIdxPaths = this.ExtractCandidateCorruptIdxPaths(failureErrors); + List idxItemsToVerify = candidateIdxPaths.Count > 0 + ? packDirContents.Where(info => candidateIdxPaths.Contains(info.FullName)).ToList() + : packDirContents; + + long? minCorruptPrefetchTimestamp; + List corruptNonPrefetchIdxPaths; + HashSet corruptIdxPaths = this.VerifyPacksAndReportCorruption( + activity, + recoveryEnabled, + idxItemsToVerify, + out corruptNonPrefetchIdxPaths, + out minCorruptPrefetchTimestamp); + + if (this.Stopping) + { + return; + } + + if (corruptIdxPaths.Count == 0 && idxItemsToVerify != packDirContents) + { + // The candidate(s) parsed from the failure text turned out to be healthy - fall back to + // verifying every pack so a real corruption elsewhere is not missed. + corruptIdxPaths = this.VerifyPacksAndReportCorruption( + activity, + recoveryEnabled, + packDirContents, + out corruptNonPrefetchIdxPaths, + out minCorruptPrefetchTimestamp); + + if (this.Stopping) + { + return; + } + } + + if (corruptIdxPaths.Count == 0) + { + return; + } + + if (!recoveryEnabled) + { + EventMetadata skippedMetadata = this.CreateEventMetadata(); + skippedMetadata["Operation"] = "CorruptPackRecoverySkipped"; + skippedMetadata["CorruptPackCount"] = corruptIdxPaths.Count; + activity.RelatedWarning( + skippedMetadata, + $"Found {corruptIdxPaths.Count} corrupt packfile(s) but {GVFSConstants.GitConfig.EnablePackfileRecovery} is disabled; leaving packs in place.", + Keywords.Telemetry); + this.reportedCorruptPacksWithRecoveryDisabled = true; + return; + } + + // Phase 2 - deletion (gated). Build the set of prefetch packs to remove: the corrupt one and + // every later (>= timestamp) prefetch pack, whether or not those later packs are themselves + // corrupt, because prefetch packs are incremental. + List laterPrefetchIdxPaths = new List(); + if (minCorruptPrefetchTimestamp.HasValue) + { + foreach (DirectoryItemInfo info in packDirContents) + { + if (!string.Equals(Path.GetExtension(info.Name), ".pack", GVFSPlatform.Instance.Constants.PathComparison)) + { + continue; + } + + long? prefetchTimestamp = GetPrefetchTimestamp(info.Name); + if (prefetchTimestamp.HasValue && prefetchTimestamp.Value >= minCorruptPrefetchTimestamp.Value) + { + laterPrefetchIdxPaths.Add(Path.ChangeExtension(info.FullName, ".idx")); + } + } + } + + // Only request a prefetch restore once a corrupt prefetch pack is actually removed. If + // deletion is blocked (e.g. a handle is still open), the corrupt pack is still present, so + // running the restore now would just re-download around a cache that is still broken. + bool corruptPrefetchPackRemoved = false; + + // Close the LibGit2 repo so the .idx files can be deleted, then remove each pack set. + this.Context.Repository.CloseActiveRepo(); + try + { + foreach (string idxPath in corruptNonPrefetchIdxPaths) + { + if (this.Stopping) + { + return; + } + + this.RemovePackFileSet(activity, idxPath, "DeletedCorruptPack", $"Deleted corrupt packfile {Path.GetFileName(Path.ChangeExtension(idxPath, ".pack"))} during pack maintenance recovery."); + } + + foreach (string idxPath in laterPrefetchIdxPaths) + { + if (this.Stopping) + { + return; + } + + if (corruptIdxPaths.Contains(idxPath)) + { + bool removed = this.RemovePackFileSet(activity, idxPath, "DeletedCorruptPack", $"Deleted corrupt prefetch packfile {Path.GetFileName(Path.ChangeExtension(idxPath, ".pack"))} during pack maintenance recovery."); + corruptPrefetchPackRemoved = corruptPrefetchPackRemoved || removed; + } + else + { + this.RemovePackFileSet(activity, idxPath, "DeletedHealthyPrefetchPack", $"Deleted healthy prefetch packfile {Path.GetFileName(Path.ChangeExtension(idxPath, ".pack"))} because an earlier prefetch pack was corrupt; incremental prefetch packs after the corruption must be removed and re-fetched."); + } + } + } + finally + { + this.Context.Repository.OpenRepo(); + } + + prefetchRestoreNeeded = corruptPrefetchPackRemoved; + } + + /// + /// Matches pack/idx file names (e.g. "pack-<hash>.pack", "prefetch-123-abc.idx") as they + /// appear embedded in git's own error text - see packfile.c's "packfile %s does not match + /// index" / "packfile %s index unavailable" and "wrong index v2 file size in %s" messages. Pack + /// file names only ever contain word characters, hyphens, and dots, so this is precise and won't + /// pick up unrelated substrings. + /// + private static readonly Regex CorruptPackFileNamePattern = new Regex(@"[\w\-]+\.(?:pack|idx)", RegexOptions.Compiled); + + /// + /// Parses candidate corrupt pack file names directly out of a 'multi-pack-index write/verify' + /// failure's stderr, returning their .idx paths under . + /// Git's own error text usually already names the specific unreadable packfile, so this lets the + /// caller skip a full verify-pack scan of every pack in the object cache. Only paths that + /// actually exist on disk are returned, since the parsed text could (rarely) reference a pack + /// from a different object-dir or a message format this pattern doesn't recognize (e.g. the + /// ordinal-only "could not load pack N" from the write path, which names no file at all). + /// + private HashSet ExtractCandidateCorruptIdxPaths(string failureErrors) + { + HashSet idxPaths = new HashSet(GVFSPlatform.Instance.Constants.PathComparer); + if (string.IsNullOrEmpty(failureErrors)) + { + return idxPaths; + } + + string packRoot = this.Context.Enlistment.GitPackRoot; + foreach (Match match in CorruptPackFileNamePattern.Matches(failureErrors)) + { + string idxFileName = Path.GetFileNameWithoutExtension(match.Value) + ".idx"; + string idxPath = Path.Combine(packRoot, idxFileName); + if (this.Context.FileSystem.FileExists(idxPath)) + { + idxPaths.Add(idxPath); + } + } + + return idxPaths; + } + + /// + /// Runs 'git verify-pack' against each .idx in that has a + /// matching .pack on disk, and reports (via telemetry) every one that fails to verify. verify-pack + /// is an external git process, so it is safe to run with the LibGit2 repo open. + /// + private HashSet VerifyPacksAndReportCorruption( + ITracer activity, + bool recoveryEnabled, + List idxItemsToVerify, + out List corruptNonPrefetchIdxPaths, + out long? minCorruptPrefetchTimestamp) + { + minCorruptPrefetchTimestamp = null; + corruptNonPrefetchIdxPaths = new List(); + HashSet corruptIdxPaths = new HashSet(GVFSPlatform.Instance.Constants.PathComparer); + + foreach (DirectoryItemInfo info in idxItemsToVerify) + { + if (this.Stopping) + { + return corruptIdxPaths; + } + + if (!string.Equals(Path.GetExtension(info.Name), ".idx", GVFSPlatform.Instance.Constants.PathComparison)) + { + continue; + } + + string idxPath = info.FullName; + string packPath = Path.ChangeExtension(idxPath, ".pack"); + + // A dangling .idx with no matching .pack is handled by CleanStaleIdxFiles; here we only + // care about packs that exist on disk but cannot be read. + if (!this.Context.FileSystem.FileExists(packPath)) + { + continue; + } + + GitProcess.Result verifyPackResult = this.RunGitCommand((process) => process.VerifyPack(idxPath), nameof(GitProcess.VerifyPack)); + + if (this.Stopping) + { + return corruptIdxPaths; + } + + if (verifyPackResult.ExitCodeIsSuccess) + { + continue; + } + + long? prefetchTimestamp = GetPrefetchTimestamp(info.Name); + bool isPrefetchPack = prefetchTimestamp.HasValue; + corruptIdxPaths.Add(idxPath); + + EventMetadata foundMetadata = this.CreateEventMetadata(); + foundMetadata["Operation"] = "FoundCorruptPack"; + foundMetadata["Pack"] = info.Name; + foundMetadata["IsPrefetchPack"] = isPrefetchPack; + foundMetadata["RecoveryEnabled"] = recoveryEnabled; + activity.RelatedWarning(foundMetadata, $"Found corrupt packfile {info.Name} during pack maintenance.", Keywords.Telemetry); + + if (isPrefetchPack) + { + if (!minCorruptPrefetchTimestamp.HasValue || prefetchTimestamp.Value < minCorruptPrefetchTimestamp.Value) + { + minCorruptPrefetchTimestamp = prefetchTimestamp.Value; + } + } + else + { + corruptNonPrefetchIdxPaths.Add(idxPath); + } + } + + return corruptIdxPaths; + } + + /// + /// True if the packfile itself was deleted. The .pack file is what actually contains the corrupt + /// (or, for a later prefetch pack, stale) data, so its deletion result - not the sidecar + /// .idx/.keep/.rev files - is what determines whether recovery for this pack set succeeded. + /// + private bool RemovePackFileSet(ITracer activity, string idxPath, string operation, string message) + { + string packPath = Path.ChangeExtension(idxPath, ".pack"); + bool packDeleted = this.Context.FileSystem.TryDeleteFile(packPath); + + EventMetadata metadata = this.CreateEventMetadata(); + metadata["Operation"] = operation; + metadata["Pack"] = Path.GetFileName(packPath); + metadata["DeletePackResult"] = packDeleted; + metadata["DeleteIdxResult"] = this.Context.FileSystem.TryDeleteFile(idxPath); + metadata["DeleteKeepResult"] = this.Context.FileSystem.TryDeleteFile(Path.ChangeExtension(idxPath, ".keep")); + metadata["DeleteRevResult"] = this.Context.FileSystem.TryDeleteFile(Path.ChangeExtension(idxPath, ".rev")); + activity.RelatedWarning(metadata, message, Keywords.Telemetry); + + return packDeleted; + } + + private void RequestPrefetchRestore(ITracer activity) + { + if (this.requestPrefetch == null) + { + // No prefetch is available (e.g. not using a cache server). The removed prefetch packs' + // objects will be re-fetched on demand through normal virtualization. + EventMetadata metadata = this.CreateEventMetadata(); + metadata["Operation"] = "PrefetchRestoreUnavailable"; + activity.RelatedWarning( + metadata, + "Removed prefetch pack(s) but no prefetch restore is available. Missing objects will be re-fetched on demand.", + Keywords.Telemetry); + return; + } + + activity.RelatedInfo("Requesting a prefetch to restore removed prefetch packs and rebuild the commit-graph."); + this.requestPrefetch(); + } } } diff --git a/GVFS/GVFS.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs b/GVFS/GVFS.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs index 811b55e15..7f7aab907 100644 --- a/GVFS/GVFS.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs +++ b/GVFS/GVFS.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs @@ -11,6 +11,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; namespace GVFS.UnitTests.Maintenance { @@ -28,6 +29,8 @@ public class PackfileMaintenanceStepTests private string WriteCommand => $"-c core.multiPackIndex=true multi-pack-index write --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\" --no-progress"; private string RepackCommand => $"-c pack.threads=1 -c repack.packKeptObjects=true multi-pack-index repack --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\" --batch-size=2g --no-progress"; + private string VerifyPackCommand(string idxName) => $"verify-pack \"{Path.Combine(this.context.Enlistment.GitPackRoot, idxName)}\""; + [TestCase] public void PackfileMaintenanceIgnoreTimeRestriction() { @@ -142,6 +145,177 @@ public void PackfileMaintenanceRewriteOnBadVerify() commands[6].ShouldEqual(this.WriteCommand); } + [TestCase] + public void PackfileMaintenanceRemovesCorruptPackWhenVerifyReportsPackLoadFailure() + { + this.TestSetup(DateTime.UtcNow); + this.SetupVerifyFailsOnceWithPackLoadError(); + + // Per-pack verification: pack-2 is the corrupt one, the rest are healthy. + this.gitProcess.SetExpectedCommandResult( + "verify-pack ", + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode), + matchPrefix: true); + this.gitProcess.SetExpectedCommandResult( + this.VerifyPackCommand("pack-2.idx"), + () => new GitProcess.Result(string.Empty, "error: could not load pack\n", GitProcess.Result.GenericFailureCode)); + + PackfileMaintenanceStep step = new TestablePackfileMaintenanceStep(this.context, recoveryEnabled: true); + step.Execute(); + + this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); + + List commands = this.gitProcess.CommandsRun; + commands.Count(c => c.StartsWith("verify-pack ")).ShouldEqual(3); + + string packRoot = this.context.Enlistment.GitPackRoot; + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-2.pack")).ShouldBeFalse(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-2.idx")).ShouldBeFalse(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-1.pack")).ShouldBeTrue(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-3.pack")).ShouldBeTrue(); + + this.WarningsContain("FoundCorruptPack").ShouldBeTrue(); + this.WarningsContain("DeletedCorruptPack").ShouldBeTrue(); + } + + [TestCase] + public void PackfileMaintenanceFastPathVerifiesOnlyNamedPackWhenErrorNamesIt() + { + this.TestSetup(DateTime.UtcNow); + this.SetupVerifyFailsOnceWithNamedPackError("pack-2.pack"); + + // Only pack-2 should be verified via the fast path - no other pack's verify-pack result is + // even registered, so the test would fail with an unexpected-command error if the fallback + // full scan ran instead. + this.gitProcess.SetExpectedCommandResult( + this.VerifyPackCommand("pack-2.idx"), + () => new GitProcess.Result(string.Empty, "error: could not load pack\n", GitProcess.Result.GenericFailureCode)); + + PackfileMaintenanceStep step = new TestablePackfileMaintenanceStep(this.context, recoveryEnabled: true); + step.Execute(); + + this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); + + List commands = this.gitProcess.CommandsRun; + commands.Count(c => c.StartsWith("verify-pack ")).ShouldEqual(1); + commands.Where(c => c.StartsWith("verify-pack ")).Single().ShouldEqual(this.VerifyPackCommand("pack-2.idx")); + + string packRoot = this.context.Enlistment.GitPackRoot; + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-2.pack")).ShouldBeFalse(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-1.pack")).ShouldBeTrue(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-3.pack")).ShouldBeTrue(); + + this.WarningsContain("FoundCorruptPack").ShouldBeTrue(); + this.WarningsContain("DeletedCorruptPack").ShouldBeTrue(); + } + + [TestCase] + public void PackfileMaintenanceFallsBackToFullScanWhenNamedPackIsHealthy() + { + this.TestSetup(DateTime.UtcNow); + + // The verify failure text names pack-1, but pack-1 turns out to verify successfully; the + // real corrupt pack (pack-2) is only found once the code falls back to the full scan. + this.SetupVerifyFailsOnceWithNamedPackError("pack-1.pack"); + + this.gitProcess.SetExpectedCommandResult( + "verify-pack ", + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode), + matchPrefix: true); + this.gitProcess.SetExpectedCommandResult( + this.VerifyPackCommand("pack-2.idx"), + () => new GitProcess.Result(string.Empty, "error: could not load pack\n", GitProcess.Result.GenericFailureCode)); + + PackfileMaintenanceStep step = new TestablePackfileMaintenanceStep(this.context, recoveryEnabled: true); + step.Execute(); + + this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); + + List commands = this.gitProcess.CommandsRun; + + // 1 fast-path verify-pack (pack-1, healthy) + 3 full-scan verify-pack (pack-1/2/3). + commands.Count(c => c.StartsWith("verify-pack ")).ShouldEqual(4); + + string packRoot = this.context.Enlistment.GitPackRoot; + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-2.pack")).ShouldBeFalse(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-1.pack")).ShouldBeTrue(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-3.pack")).ShouldBeTrue(); + + this.WarningsContain("FoundCorruptPack").ShouldBeTrue(); + this.WarningsContain("DeletedCorruptPack").ShouldBeTrue(); + } + + [TestCase] + public void PackfileMaintenanceDetectsButDoesNotDeleteWhenRecoveryDisabled() + { + this.TestSetup(DateTime.UtcNow); + this.SetupVerifyFailsOnceWithPackLoadError(); + + this.gitProcess.SetExpectedCommandResult( + "verify-pack ", + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode), + matchPrefix: true); + this.gitProcess.SetExpectedCommandResult( + this.VerifyPackCommand("pack-2.idx"), + () => new GitProcess.Result(string.Empty, "error: could not load pack\n", GitProcess.Result.GenericFailureCode)); + + PackfileMaintenanceStep step = new TestablePackfileMaintenanceStep(this.context, recoveryEnabled: false); + step.Execute(); + + this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); + + List commands = this.gitProcess.CommandsRun; + + // Detection still runs (verify-pack on each pack), but nothing is deleted. + commands.Count(c => c.StartsWith("verify-pack ")).ShouldEqual(3); + + string packRoot = this.context.Enlistment.GitPackRoot; + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-2.pack")).ShouldBeTrue(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "pack-2.idx")).ShouldBeTrue(); + + this.WarningsContain("FoundCorruptPack").ShouldBeTrue(); + this.WarningsContain("CorruptPackRecoverySkipped").ShouldBeTrue(); + this.WarningsContain("DeletedCorruptPack").ShouldBeFalse(); + } + + [TestCase] + public void PackfileMaintenanceRemovesLaterPrefetchPacksAndRequestsPrefetch() + { + this.PrefetchTestSetup(DateTime.UtcNow); + this.SetupVerifyFailsOnceWithPackLoadError(); + + this.gitProcess.SetExpectedCommandResult( + "verify-pack ", + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode), + matchPrefix: true); + this.gitProcess.SetExpectedCommandResult( + this.VerifyPackCommand("prefetch-2-bbb.idx"), + () => new GitProcess.Result(string.Empty, "error: could not load pack\n", GitProcess.Result.GenericFailureCode)); + + bool prefetchRequested = false; + PackfileMaintenanceStep step = new TestablePackfileMaintenanceStep( + this.context, + recoveryEnabled: true, + requestPrefetch: () => prefetchRequested = true); + step.Execute(); + + this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); + + string packRoot = this.context.Enlistment.GitPackRoot; + + // The corrupt prefetch pack and every later prefetch pack are removed; the earlier healthy + // prefetch pack is kept. + this.context.FileSystem.FileExists(Path.Combine(packRoot, "prefetch-2-bbb.pack")).ShouldBeFalse(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "prefetch-3-ccc.pack")).ShouldBeFalse(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "prefetch-3-ccc.keep")).ShouldBeFalse(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "prefetch-1-aaa.pack")).ShouldBeTrue(); + this.context.FileSystem.FileExists(Path.Combine(packRoot, "prefetch-1-aaa.idx")).ShouldBeTrue(); + + this.WarningsContain("DeletedCorruptPack").ShouldBeTrue(); + this.WarningsContain("DeletedHealthyPrefetchPack").ShouldBeTrue(); + prefetchRequested.ShouldBeTrue(); + } + [TestCase] public void CountPackFiles() { @@ -240,5 +414,111 @@ private void TestSetup(DateTime lastRun, bool failOnVerify = false) this.RepackCommand, () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); } + + private void PrefetchTestSetup(DateTime lastRun) + { + string lastRunTime = EpochConverter.ToUnixEpochSeconds(lastRun).ToString(); + + this.gitProcess = new MockGitProcess(); + GVFSEnlistment enlistment = new MockGVFSEnlistment(this.gitProcess); + + MockFile timeFile = new MockFile(Path.Combine(enlistment.GitObjectsRoot, "info", PackfileMaintenanceStep.PackfileLastRunFileName), lastRunTime); + MockDirectory info = new MockDirectory( + Path.Combine(enlistment.GitObjectsRoot, "info"), + null, + new List() { timeFile }); + + // Three prefetch packs in ascending timestamp order, newest .keep'd (as GVFS does). + MockDirectory pack = new MockDirectory( + enlistment.GitPackRoot, + null, + new List() + { + new MockFile(Path.Combine(enlistment.GitPackRoot, "prefetch-1-aaa.pack"), "one"), + new MockFile(Path.Combine(enlistment.GitPackRoot, "prefetch-1-aaa.idx"), "1"), + new MockFile(Path.Combine(enlistment.GitPackRoot, "prefetch-2-bbb.pack"), "two"), + new MockFile(Path.Combine(enlistment.GitPackRoot, "prefetch-2-bbb.idx"), "2"), + new MockFile(Path.Combine(enlistment.GitPackRoot, "prefetch-3-ccc.pack"), "three"), + new MockFile(Path.Combine(enlistment.GitPackRoot, "prefetch-3-ccc.idx"), "3"), + new MockFile(Path.Combine(enlistment.GitPackRoot, "prefetch-3-ccc.keep"), string.Empty), + }); + + MockDirectory gitObjectsRoot = new MockDirectory(enlistment.GitObjectsRoot, new List() { info, pack }, null); + List directories = new List() { gitObjectsRoot }; + PhysicalFileSystem fileSystem = new MockFileSystem(new MockDirectory(enlistment.PrimaryEnlistmentRoot, directories, null)); + + this.tracer = new MockTracer(); + MockGitRepo repository = new MockGitRepo(this.tracer, enlistment, fileSystem); + this.context = new GVFSContext(this.tracer, fileSystem, repository, enlistment); + + this.gitProcess.SetExpectedCommandResult( + this.WriteCommand, + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); + this.gitProcess.SetExpectedCommandResult( + this.ExpireCommand, + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); + this.gitProcess.SetExpectedCommandResult( + this.RepackCommand, + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); + } + + /// + /// Makes the multi-pack-index verify fail the first time with a "could not load pack" error + /// (the corrupt-pack signature) and succeed afterwards. + /// + private void SetupVerifyFailsOnceWithPackLoadError() + { + int verifyCount = 0; + this.gitProcess.SetExpectedCommandResult( + this.VerifyCommand, + () => + { + verifyCount++; + return verifyCount == 1 + ? new GitProcess.Result(string.Empty, "failed to load pack in position 0\n", GitProcess.Result.GenericFailureCode) + : new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode); + }); + } + + /// + /// Makes the multi-pack-index verify fail the first time with an error that names + /// directly (as real git verify failures do - e.g. "failed to + /// load pack entry for oid[0] = ..." followed by "packfile pack-2.pack does not match index"), + /// and succeed afterwards. + /// + private void SetupVerifyFailsOnceWithNamedPackError(string packFileName) + { + int verifyCount = 0; + this.gitProcess.SetExpectedCommandResult( + this.VerifyCommand, + () => + { + verifyCount++; + return verifyCount == 1 + ? new GitProcess.Result(string.Empty, $"failed to load pack entry for oid[0] = abc\nerror: packfile {packFileName} does not match index\n", GitProcess.Result.GenericFailureCode) + : new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode); + }); + } + + private bool WarningsContain(string operation) + { + return this.tracer.StartActivityTracer.RelatedWarningEvents.Any(e => e.Contains(operation)); + } + + private class TestablePackfileMaintenanceStep : PackfileMaintenanceStep + { + private readonly bool recoveryEnabled; + + public TestablePackfileMaintenanceStep(GVFSContext context, bool recoveryEnabled, Action requestPrefetch = null) + : base(context, requireObjectCacheLock: false, forceRun: true, requestPrefetch: requestPrefetch) + { + this.recoveryEnabled = recoveryEnabled; + } + + protected override bool IsPackfileRecoveryEnabled() + { + return this.recoveryEnabled; + } + } } }