Re: [PATCH 06/11] multi-pack-index: implement 'expire' subcommand

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 11, 2019, 18:51 UTC
Message-ID: <xmqqo934klah.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <8213541052a7d040aad4352ee981c0de01cc6516.1560209720.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> The 'git multi-pack-index expire' subcommand looks at the existing
> mult-pack-index, counts the number of objects referenced in each

s/mult/&i/

> pack-file, deletes the pack-fils with no referenced objects, and

s/fils/files/

> rewrites the multi-pack-index to no longer reference those packs.

As we do *not* want to expire a packfile that was created after the last run of "git multi-pack-index write", "... in each pack-file" in the above paragraph is a bit tricky. It cannot literally be "in each packfile", but a subset of all existing packfiles that are known to the midx we are looking at.

An obvious alternative is to actually update midx file (so it no longer "looks at the existing multi-pack-index", but "makes sure we have up-to-date view of the multi-pack-index that covers all packs).

And I actually suspect that you implemented that alternative here. IOW, "looks at the existing multi-pack-index" in the first paragraph is misleading.

I wondered if the error behaviour of add_pack_to_midx() that ignores a pack that cannot be added to midx for whatever reason can cause issues, but such a pack would not become part of m->packs[] so it may not have anything count[]ed from it, but it will not be eligible for expiration, either. So we'd be safe there, I think.

> Test that a new pack-file that covers the contents of two other
> pack-files leads to those pack-files being deleted during the
> expire subcommand. Be sure to read the multi-pack-index to ensure
> it no longer references those packs.

I am curious to learn how we guarantee that the midx logic chooses to record the copy of an object in the new single packfile over the other copy of the same object in one of the two older and presumably smaller packfiles for all objects in the latter two packfiles, because they cannot be expired if even one object were used from there.