Re: [PATCH 0/7] midx: ignore cruft pack with `repack`, `expire`

[Derrick Stolee <derrickstolee@github.com>]

On 9/19/2022 9:55 PM, Taylor Blau wrote:
> This series fixes a pair of bugs that were originally pointed out by
> Gregory Szorc in [1].
> 
> Namely, that both `git multi-pack-index repack` and `git
> multi-pack-index expire` can cause us to "absorb" the cruft pack,
> distributing its objects into a new pack, and removing its metadata.
> 
> This is worth avoiding, since even though it doesn't result in object
> corruption, this bug removes semi-important metadata contained in the
> .mtimes file, which controls how fast objects leave the repository
> during a pruning GC.
> 
> This series teaches both sub-commands to avoid any cruft pack(s),
> preserving their metadata.

I gave these patches a careful review. They all look correct and
are closing possible gaps in this mixed mode of repacking with
different strategies.

I do think the 'expire' situation is very rare, but it's best to
be safe here.

Overall, the end result is that users could set up their background
maintenance as follows:

 1. maintenance.incremental-repack.schedule=daily to do an
    incremental repack every night.

 2. maintenance.gc.schedule=weekly to do a GC once a week.

 3. With gc.cruftPacks=true and a gc.pruneExpire value, the cruft
    packs will be written on that weekly job, only expiring old
    enough objects.

Before this series, that weekly GC creating cruft packs ran the
(very likely) risk of those cruft packs being rewritten with the
rest of the new object data, losing the .mtimes data.

(While the proposed schedule above is now possible, I don't
recommend it as a default. Not only will it delete objects
automatically, but the GC task is very expensive for very large
repositories and the incremental-repack task was explicitly
created to avoid that huge cost in those cases.)

Thanks for this update!
-Stolee