Subject: Re: [PATCH 0/3] repack: handle --keep-pack, --max-pack-size for geometric repacks

From: Derrick Stolee <[email-masked]>
Date: Fri, 20 May 2022 16:05:29 -0400

On 5/20/2022 3:46 PM, Victoria Dye wrote:
> Taylor Blau wrote:
>> This series fixes two issues that Victoria and I noticed while working on an
>> unrelated issue yesterday.
>>
>>   - The first patch comes from Victoria's earlier submission[1], and addresses
>>     an issue where packs specified as kept via the `--keep-pack` option could
>>     potentially be removed (without rewriting their objects) during a
>>     `--geometric` repack.
>>
>>     The first patch is Victoria's alone, with some minor fixups applied from my
>>     review in [2]. It's included in this series since it's related, and avoids
>>     any conflicts when merging.
>>
> 
> I'm happy with the fixes you applied here and don't have anything else I'd
> like to add this patch.
> 
>>   - The latter two patches are mine, and address an issue where specifying a
>>     `--max-pack-size` value during a `--geometric` repack could result in object
>>     loss because of a false positive in our "did we write a pack with this
>>     name?" check (which can occur when the list of packs we wrote isn't sorted).
>>
>>     The first of these two patches demonstrates the issue (done in a separate
>>     patch, since the scenario is quite involved), and the second patch fixes the
>>     bug.
>>
> 
> I was worried about the robustness of the test, but some deeper diving
> revealed that it should produce consistent results. Otherwise, the fix
> itself is a straightforward (albeit hard to find in the first place). These
> two patches look good to me!
> 
>> Thanks in advance for your review.

I'm chiming in to say that I also read these patches and think they
are good. Couldn't find a way to improve them.

Thanks,
-Stolee