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.

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

PackfileMaintenanceStepTests: fix flaky tests #205

Merged
merged 1 commit into from
Oct 30, 2019
Merged

PackfileMaintenanceStepTests: fix flaky tests #205

merged 1 commit into from
Oct 30, 2019

Conversation

derrickstolee
Copy link
Contributor

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

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>
@derrickstolee derrickstolee added this to the MVP milestone Oct 30, 2019
@derrickstolee
Copy link
Contributor Author

See also microsoft/VFSForGit#1562.

@derrickstolee derrickstolee merged commit 872b0d6 into microsoft:master Oct 30, 2019
@derrickstolee derrickstolee deleted the fix-flaky-packfile-test branch November 18, 2019 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants