Skip to content

Commit

Permalink
PackfileMaintenanceStep: Use second-largest pack-size for batch size
Browse files Browse the repository at this point in the history
When the total size of all pack-files is larger than 2 gigabytes, then
we currently use a strange mechanism for trying to repack everything
else into a single pack. It was recommended that we should use the
second-largest pack-file size instead.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
  • Loading branch information
derrickstolee committed May 4, 2020
1 parent 489764b commit 72dff37
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 19 deletions.
6 changes: 4 additions & 2 deletions Scalar.Common/Maintenance/GitMaintenanceStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,12 @@ public void Stop()
}

// public only for unit tests
public void GetPackFilesInfo(out int count, out long size, out long maxSize, out bool hasKeep)
public void GetPackFilesInfo(out int count, out long size, out long secondLargestSize, out bool hasKeep)
{
count = 0;
size = 0;
maxSize = 0;
long maxSize = 0;
secondLargestSize = 0;
hasKeep = false;

foreach (DirectoryItemInfo info in this.Context.FileSystem.ItemsInDirectory(this.Context.Enlistment.GitPackRoot))
Expand All @@ -160,6 +161,7 @@ public void GetPackFilesInfo(out int count, out long size, out long maxSize, out

if (info.Length > maxSize)
{
secondLargestSize = maxSize;
maxSize = info.Length;
}
}
Expand Down
8 changes: 4 additions & 4 deletions Scalar.Common/Maintenance/LooseObjectsStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,13 @@ protected override void PerformMaintenance()
}

this.CountLooseObjects(out int beforeLooseObjectsCount, out long beforeLooseObjectsSize);
this.GetPackFilesInfo(out int beforePackCount, out long beforePackSize, out long beforeMaxSize, out bool _);
this.GetPackFilesInfo(out int beforePackCount, out long beforePackSize, out long beforeSize2, out bool _);

GitProcess.Result gitResult = this.RunGitCommand((process) => process.PrunePacked(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.PrunePacked));
CreatePackResult createPackResult = this.TryCreateLooseObjectsPackFile(out int objectsAddedToPack);

this.CountLooseObjects(out int afterLooseObjectsCount, out long afterLooseObjectsSize);
this.GetPackFilesInfo(out int afterPackCount, out long afterPackSize, out long afterMaxSize, out bool _);
this.GetPackFilesInfo(out int afterPackCount, out long afterPackSize, out long afterSize2, out bool _);

EventMetadata metadata = new EventMetadata();
metadata.Add("GitObjectsRoot", this.Context.Enlistment.GitObjectsRoot);
Expand All @@ -210,8 +210,8 @@ protected override void PerformMaintenance()
metadata.Add("EndingSize", afterLooseObjectsSize);
metadata.Add("StartingPackSize", beforePackSize);
metadata.Add("EndingPackSize", afterPackSize);
metadata.Add("StartingMaxSize", beforeMaxSize);
metadata.Add("EndingMaxSize", afterMaxSize);
metadata.Add("StartingSize2", beforeSize2);
metadata.Add("EndingSize2", afterSize2);

metadata.Add("RemovedCount", beforeLooseObjectsCount - afterLooseObjectsCount);
metadata.Add("LooseObjectsPutIntoPackFile", objectsAddedToPack);
Expand Down
19 changes: 11 additions & 8 deletions Scalar.Common/Maintenance/PackfileMaintenanceStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ protected override void PerformMaintenance()
GitProcess.Result expireResult = this.RunGitCommand((process) => process.MultiPackIndexExpire(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.MultiPackIndexExpire));

List<string> staleIdxFiles = this.CleanStaleIdxFiles(out int numDeletionBlocked);
this.GetPackFilesInfo(out int expireCount, out long expireSize, out long expireMaxSize, out hasKeep);
this.GetPackFilesInfo(out int expireCount, out long expireSize, out long expireSize2, out hasKeep);

GitProcess.Result verifyAfterExpire = this.RunGitCommand((process) => process.VerifyMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.VerifyMultiPackIndex));

Expand All @@ -136,15 +136,18 @@ protected override void PerformMaintenance()
this.LogErrorAndRewriteMultiPackIndex(activity);
}

if (this.batchSize.Equals(DefaultBatchSizeBytes.ToString()) && expireSize - expireMaxSize < DefaultBatchSizeBytes && expireCount > 2)
if (this.batchSize.Equals(DefaultBatchSizeBytes.ToString()) &&
expireSize < DefaultBatchSizeBytes &&
expireCount > 2)
{
// Ignoring the largest pack, try repacking the rest of the packs into a single pack.
// Allow a 3% error rate due to the esitmations made in the repack command.
this.batchSize = ((long)((expireSize - expireMaxSize) * 0.97)).ToString();
// Ignoring the largest pack, repack the rest up to the size of the
// second-smallest pack. This results in a geometrically-decreasing
// list of pack sizes after the largest pack.
this.batchSize = expireSize2.ToString();
}

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 long afterMaxSize, out hasKeep);
this.GetPackFilesInfo(out int afterCount, out long afterSize, out long afterSize2, out hasKeep);

GitProcess.Result verifyAfterRepack = this.RunGitCommand((process) => process.VerifyMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.VerifyMultiPackIndex));

Expand All @@ -160,10 +163,10 @@ protected override void PerformMaintenance()
metadata.Add(nameof(beforeSize), beforeSize);
metadata.Add(nameof(expireCount), expireCount);
metadata.Add(nameof(expireSize), expireSize);
metadata.Add(nameof(expireMaxSize), expireMaxSize);
metadata.Add(nameof(expireSize2), expireSize2);
metadata.Add(nameof(afterCount), afterCount);
metadata.Add(nameof(afterSize), afterSize);
metadata.Add(nameof(afterMaxSize), afterMaxSize);
metadata.Add(nameof(afterSize2), afterSize2);
metadata.Add("VerifyAfterExpireExitCode", verifyAfterExpire.ExitCode);
metadata.Add("VerifyAfterRepackExitCode", verifyAfterRepack.ExitCode);
metadata.Add("NumStaleIdxFiles", staleIdxFiles.Count);
Expand Down
10 changes: 5 additions & 5 deletions Scalar.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class PackfileMaintenanceStepTests
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}\"";
private string RepackCommand => $"-c pack.threads=1 multi-pack-index repack --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\" --batch-size=5";
private string RepackCommand => $"-c pack.threads=1 multi-pack-index repack --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\" --batch-size=3";

[TestCase]
public void PackfileMaintenanceIgnoreTimeRestriction()
Expand Down Expand Up @@ -149,18 +149,18 @@ public void CountPackFiles()

PackfileMaintenanceStep step = new PackfileMaintenanceStep(this.context, requireObjectCacheLock: false, forceRun: true);

step.GetPackFilesInfo(out int count, out long size, out long maxSize, out bool hasKeep);
step.GetPackFilesInfo(out int count, out long size, out long secondSmallestSize, out bool hasKeep);
count.ShouldEqual(3);
size.ShouldEqual(11);
maxSize.ShouldEqual(5);
secondSmallestSize.ShouldEqual(3);
hasKeep.ShouldEqual(true);

this.context.FileSystem.DeleteFile(Path.Combine(this.context.Enlistment.GitPackRoot, KeepName));

step.GetPackFilesInfo(out count, out size, out maxSize, out hasKeep);
step.GetPackFilesInfo(out count, out size, out secondSmallestSize, out hasKeep);
count.ShouldEqual(3);
size.ShouldEqual(11);
maxSize.ShouldEqual(5);
secondSmallestSize.ShouldEqual(3);
hasKeep.ShouldEqual(false);
}

Expand Down

0 comments on commit 72dff37

Please sign in to comment.