Skip to content

Commit

Permalink
Merge pull request #205: PackfileMaintenanceStepTests: fix flaky tests
Browse files Browse the repository at this point in the history
The PackfileMaintenanceStepTests have been failing periodically, and
mostly with failures in RepackAllToOnePack() leading to a failure in
ExpireAllButOneAndKeep().

The way we "force" Git to repack all is to provide a custom batch size.
This batch size used to mean "repack all pack-files with size at most
X until reaching a size of at least X". By specifying "total size - 1",
this required repacking all objects.

HOWEVER, the new logic actually computes an "expected" size of the
objects in a pack that are referenced by the multi-pack-index. If we
have duplicate objects, then those objects can cause two types of error:

1. The duplicates reduce the total expected size below the batch size.

2. The computation of "total expected size" causes round-off error.

Both types of error under-count the number of bytes in the final
pack-file.

By changing the batch size to "total size - min size + 1" we ensure the
following:

  I. The batch size is larger than the maximum pack size.

 II. Every pack-file is required to reach the batch size.

III. We have minimized the chance that duplicate objects will drop
     below this threshold.

With the use of gvfs-helper, we have less control over the pack-files
that are downloaded as part of the clone operation. At minimum, with
this change we should be in a better state than before.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
  • Loading branch information
derrickstolee authored Oct 30, 2019
2 parents a388f0c + 3e4c464 commit 872b0d6
Showing 1 changed file with 16 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Scalar.FunctionalTests.FileSystemRunners;
using Scalar.FunctionalTests.Tools;
using Scalar.Tests.Should;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand All @@ -27,7 +28,11 @@ public PackfileMaintenanceStepTests()
[TestCase, Order(1)]
public void RepackAllToOnePack()
{
this.GetPackSizes(out int beforeFetchCommitsAndTreesPackCount, out long maxSize, out long totalSize);
this.GetPackSizes(
out int beforeFetchCommitsAndTreesPackCount,
out long maxSize,
out long minSize,
out long totalSize);

// Cannot be sure of the count, but there should be two from the inital clone
beforeFetchCommitsAndTreesPackCount.ShouldBeAtLeast(2);
Expand All @@ -41,13 +46,13 @@ public void RepackAllToOnePack()
// Run the step to ensure we don't have any packs that will be expired during the repack step
this.Enlistment.PackfileMaintenanceStep();

this.GetPackSizes(out int afterPrefetchPackCount, out maxSize, out totalSize);
this.GetPackSizes(out int afterPrefetchPackCount, out maxSize, out minSize, out totalSize);

// Cannot be sure of the count, as the fetch-commits-and-trees uses parallel threads to get multiple packs
afterPrefetchPackCount.ShouldBeAtLeast(2);

this.Enlistment.PackfileMaintenanceStep(batchSize: totalSize - 1);
this.GetPackSizes(out int packCount, out maxSize, out totalSize);
this.Enlistment.PackfileMaintenanceStep(batchSize: totalSize - minSize + 1);
this.GetPackSizes(out int packCount, out maxSize, out minSize, out totalSize);

// We should not have expired any packs, but created a new one with repack
packCount.ShouldEqual(afterPrefetchPackCount + 1, $"incorrect number of packs after repack step: {packCount}");
Expand Down Expand Up @@ -76,10 +81,11 @@ private List<string> GetPackfiles()
return Directory.GetFiles(this.PackRoot, "*.pack").ToList();
}

private void GetPackSizes(out int packCount, out long maxSize, out long totalSize)
private void GetPackSizes(out int packCount, out long maxSize, out long minSize, out long totalSize)
{
totalSize = 0;
maxSize = 0;
minSize = long.MaxValue;
packCount = 0;

foreach (string file in this.GetPackfiles())
Expand All @@ -92,6 +98,11 @@ private void GetPackSizes(out int packCount, out long maxSize, out long totalSiz
{
maxSize = size;
}

if (size < minSize)
{
minSize = size;
}
}
}
}
Expand Down

0 comments on commit 872b0d6

Please sign in to comment.