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

From: Derrick Stolee <derrickstolee@github.com>
Date: Sep 22, 2022, 14:14 UTC
Message-ID: <63a1c3d4-eff3-af10-4263-058c88e74594@github.com>
In-Reply-To: <cover.1663638929.git.me@ttaylorr.com>

On 9/19/2022 9:55 PM, Taylor Blau wrote:

Show 14 quoted lines

> 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