Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[master] Update Git, drop MIDX writes #1421

Merged
merged 2 commits into from
Aug 7, 2019
Merged

[master] Update Git, drop MIDX writes #1421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ccdedc
Update Git to fix slow commit-graph writes
derrickstolee Aug 6, 2019
d352d58
PostFetchJob: stop writing multi-pack-index
derrickstolee Aug 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion GVFS/GVFS.Build/GVFS.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

<PropertyGroup Label="Parameters">
<GVFSVersion>0.2.173.2</GVFSVersion>
<GitPackageVersion>2.20190805.1</GitPackageVersion>
<GitPackageVersion>2.20190806.1</GitPackageVersion>
</PropertyGroup>

<PropertyGroup Label="DefaultSettings">
Expand Down
16 changes: 11 additions & 5 deletions GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ 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;

Expand Down Expand Up @@ -105,15 +106,20 @@ protected override void PerformMaintenance()
activity.RelatedWarning($"Skipping {nameof(PackfileMaintenanceStep)} due to git pids {string.Join(",", processIds)}", Keywords.Telemetry);
return;
}
}

this.GetPackFilesInfo(out int beforeCount, out long beforeSize, out bool hasKeep);
}
this.GetPackFilesInfo(out int beforeCount, out long beforeSize, out bool hasKeep);

if (!hasKeep)
{
activity.RelatedWarning(this.CreateEventMetadata(), "Skipping pack maintenance due to no .keep file.");
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,
Expand Down
15 changes: 0 additions & 15 deletions GVFS/GVFS.Common/Maintenance/PostFetchStep.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ namespace GVFS.Common.Maintenance
public class PostFetchStep : GitMaintenanceStep
{
private const string CommitGraphChainLock = "commit-graph-chain.lock";
private const string MultiPackIndexLock = "multi-pack-index.lock";
private List<string> packIndexes;

public PostFetchStep(GVFSContext context, List<string> packIndexes, bool requireObjectCacheLock = true)
Expand All @@ -23,20 +22,6 @@ public PostFetchStep(GVFSContext context, List<string> packIndexes, bool require

protected override void PerformMaintenance()
{
using (ITracer activity = this.Context.Tracer.StartActivity("TryWriteMultiPackIndex", EventLevel.Informational))
{
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));

GitProcess.Result verifyResult = this.RunGitCommand((process) => process.VerifyMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.VerifyMultiPackIndex));
if (!this.Stopping && verifyResult.ExitCodeIsFailure)
{
this.LogErrorAndRewriteMultiPackIndex(activity);
}
}

if (this.packIndexes == null || this.packIndexes.Count == 0)
{
this.Context.Tracer.RelatedInfo(this.Area + ": Skipping commit-graph write due to no new packfiles");
Expand Down
25 changes: 0 additions & 25 deletions GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ namespace GVFS.FunctionalTests.Tests.EnlistmentPerFixture
public class PrefetchVerbTests : TestsWithEnlistmentPerFixture
{
private const string PrefetchCommitsAndTreesLock = "prefetch-commits-trees.lock";
private const string MultiPackIndexLock = "multi-pack-index.lock";
private const string LsTreeTypeInPathBranchName = "FunctionalTests/20181105_LsTreeTypeInPath";

private FileSystemRunner fileSystem;
Expand Down Expand Up @@ -137,27 +136,6 @@ public void PrefetchCleansUpStalePrefetchLock()
prefetchCommitsLockFile.ShouldNotExistOnDisk(this.fileSystem);
}

[TestCase, Order(11)]
[Category(Categories.MacTODO.TestNeedsToLockFile)] // PostFetchStepShouldComplete waits for a lock file
public void PrefetchCleansUpPackDir()
{
string multiPackIndexLockFile = Path.Combine(this.Enlistment.GetPackRoot(this.fileSystem), MultiPackIndexLock);
string oldGitTempFile = Path.Combine(this.Enlistment.GetPackRoot(this.fileSystem), "tmp_midx_XXXX");
string oldKeepFile = Path.Combine(this.Enlistment.GetPackRoot(this.fileSystem), "prefetch-00000000-HASH.keep");

this.fileSystem.WriteAllText(multiPackIndexLockFile, this.Enlistment.EnlistmentRoot);
this.fileSystem.WriteAllText(oldGitTempFile, this.Enlistment.EnlistmentRoot);
this.fileSystem.WriteAllText(oldKeepFile, this.Enlistment.EnlistmentRoot);

this.Enlistment.Prefetch("--commits");
this.Enlistment.PostFetchStep();
oldGitTempFile.ShouldNotExistOnDisk(this.fileSystem);
oldKeepFile.ShouldNotExistOnDisk(this.fileSystem);

this.PostFetchStepShouldComplete();
multiPackIndexLockFile.ShouldNotExistOnDisk(this.fileSystem);
}

[TestCase, Order(12)]
public void PrefetchFilesFromFileListFile()
{
Expand Down Expand Up @@ -236,9 +214,6 @@ private void PostFetchStepShouldComplete()
}
while (this.fileSystem.FileExists(objectCacheLock));

ProcessResult midxResult = GitProcess.InvokeProcess(this.Enlistment.RepoRoot, "multi-pack-index verify --object-dir=\"" + objectDir + "\"");
midxResult.ExitCode.ShouldEqual(0);

// A commit graph is not always generated, but if it is, then we want to ensure it is in a good state
if (this.fileSystem.FileExists(Path.Combine(objectDir, "info", "commit-graphs", "commit-graph-chain")))
{
Expand Down
15 changes: 0 additions & 15 deletions GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbWithoutSharedCacheTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,6 @@ public void PrefetchBuildsIdxWhenMissingFromPrefetchPack()
this.fileSystem.DeleteFile(idxPath);
idxPath.ShouldNotExistOnDisk(this.fileSystem);

// Remove midx that contains references to the pack
string midxPath = Path.Combine(this.Enlistment.GetObjectRoot(this.fileSystem), "pack", "multi-pack-index");
File.SetAttributes(midxPath, FileAttributes.Normal);
this.fileSystem.DeleteFile(midxPath);
midxPath.ShouldNotExistOnDisk(this.fileSystem);

// Prefetch should rebuild the missing idx
this.Enlistment.Prefetch("--commits");
this.PostFetchJobShouldComplete();
Expand Down Expand Up @@ -296,13 +290,10 @@ public void PrefetchCleansUpStaleTempPrefetchPacks()
[TestCase, Order(9)]
public void PrefetchCleansUpOphanedLockFiles()
{
// Multi-pack-index write happens even if the prefetch downloads nothing, while
// the commit-graph write happens only when the prefetch downloads at least one pack

string graphPath = Path.Combine(this.Enlistment.GetObjectRoot(this.fileSystem), "info", "commit-graphs", "commit-graph-chain");
string graphLockPath = graphPath + ".lock";
string midxPath = Path.Combine(this.PackRoot, "multi-pack-index");
string midxLockPath = midxPath + ".lock";

this.fileSystem.CreateEmptyFile(graphLockPath);

Expand All @@ -312,7 +303,6 @@ public void PrefetchCleansUpOphanedLockFiles()
// Force deleting the prefetch packs to make the prefetch non-trivial.
this.fileSystem.DeleteDirectory(this.PackRoot);
this.fileSystem.CreateDirectory(this.PackRoot);
this.fileSystem.CreateEmptyFile(midxLockPath);

// Re-mount so the post-fetch job runs
this.Enlistment.MountGVFS();
Expand All @@ -321,9 +311,7 @@ public void PrefetchCleansUpOphanedLockFiles()
this.PostFetchJobShouldComplete();

this.fileSystem.FileExists(graphLockPath).ShouldBeFalse(nameof(graphLockPath));
this.fileSystem.FileExists(midxLockPath).ShouldBeFalse(nameof(midxLockPath));
this.fileSystem.FileExists(graphPath).ShouldBeTrue(nameof(graphPath));
this.fileSystem.FileExists(midxPath).ShouldBeTrue(nameof(midxPath));
}

private void PackShouldHaveIdxFile(string pathPath)
Expand Down Expand Up @@ -406,9 +394,6 @@ private void PostFetchJobShouldComplete()
Thread.Sleep(500);
}

ProcessResult midxResult = GitProcess.InvokeProcess(this.Enlistment.RepoRoot, "multi-pack-index verify --object-dir=\"" + objectDir + "\"");
midxResult.ExitCode.ShouldEqual(0);

ProcessResult graphResult = GitProcess.InvokeProcess(this.Enlistment.RepoRoot, "commit-graph verify --shallow --object-dir=\"" + objectDir + "\"");
graphResult.ExitCode.ShouldEqual(0);
}
Expand Down
48 changes: 27 additions & 21 deletions GVFS/GVFS.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ public class PackfileMaintenanceStepTests
private const string KeepName = "pack-3.keep";
private MockTracer tracer;
private MockGitProcess gitProcess;
private GVFSContext context;

private GVFSContext context;
private string ExpireCommand => $"multi-pack-index expire --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\"";
private string VerifyCommand => $"-c core.multiPackIndex=true multi-pack-index verify --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\"";
private string WriteCommand => $"-c core.multiPackIndex=true multi-pack-index write --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\"";
Expand All @@ -39,11 +39,12 @@ public void PackfileMaintenanceIgnoreTimeRestriction()
this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0);
this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(0);
List<string> commands = this.gitProcess.CommandsRun;
commands.Count.ShouldEqual(4);
commands[0].ShouldEqual(this.ExpireCommand);
commands[1].ShouldEqual(this.VerifyCommand);
commands[2].ShouldEqual(this.RepackCommand);
commands[3].ShouldEqual(this.VerifyCommand);
commands.Count.ShouldEqual(5);
commands[0].ShouldEqual(this.WriteCommand);
commands[1].ShouldEqual(this.ExpireCommand);
commands[2].ShouldEqual(this.VerifyCommand);
commands[3].ShouldEqual(this.RepackCommand);
commands[4].ShouldEqual(this.VerifyCommand);
}

[TestCase]
Expand Down Expand Up @@ -82,11 +83,12 @@ public void PackfileMaintenancePassTimeRestriction()
this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0);
this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(0);
List<string> commands = this.gitProcess.CommandsRun;
commands.Count.ShouldEqual(4);
commands[0].ShouldEqual(this.ExpireCommand);
commands[1].ShouldEqual(this.VerifyCommand);
commands[2].ShouldEqual(this.RepackCommand);
commands[3].ShouldEqual(this.VerifyCommand);
commands.Count.ShouldEqual(5);
commands[0].ShouldEqual(this.WriteCommand);
commands[1].ShouldEqual(this.ExpireCommand);
commands[2].ShouldEqual(this.VerifyCommand);
commands[3].ShouldEqual(this.RepackCommand);
commands[4].ShouldEqual(this.VerifyCommand);
}

[TestCase]
Expand Down Expand Up @@ -130,13 +132,14 @@ public void PackfileMaintenanceRewriteOnBadVerify()
this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(2);

List<string> commands = this.gitProcess.CommandsRun;
commands.Count.ShouldEqual(6);
commands[0].ShouldEqual(this.ExpireCommand);
commands[1].ShouldEqual(this.VerifyCommand);
commands[2].ShouldEqual(this.WriteCommand);
commands[3].ShouldEqual(this.RepackCommand);
commands[4].ShouldEqual(this.VerifyCommand);
commands[5].ShouldEqual(this.WriteCommand);
commands.Count.ShouldEqual(7);
commands[0].ShouldEqual(this.WriteCommand);
commands[1].ShouldEqual(this.ExpireCommand);
commands[2].ShouldEqual(this.VerifyCommand);
commands[3].ShouldEqual(this.WriteCommand);
commands[4].ShouldEqual(this.RepackCommand);
commands[5].ShouldEqual(this.VerifyCommand);
commands[6].ShouldEqual(this.WriteCommand);
}

[TestCase]
Expand Down Expand Up @@ -222,8 +225,11 @@ private void TestSetup(DateTime lastRun, bool failOnVerify = false)

// Create and return Context
this.tracer = new MockTracer();
this.context = new GVFSContext(this.tracer, fileSystem, repository, enlistment);

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));
Expand Down
Loading