Re: [PATCH v4 4/6] pack-objects: generate cruft packs at most one object over threshold

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

>> So would it be feasible to remember how 199MB cruft pack is lying in
>> the object store (i.e. earlier we packed as much as possible), and
>> add a logic that says "if there is nothing to expire out of this
>> one, do not attempt to repack---this is fine as-is"?
> .... So
> the majority of packs in this case should all be removed, and the small
> amount of cruft data remaining can be repacked into a small number of
> packs relatively quickly.

Given the above ...

> Suppose you have a 100MB cruft limit, and there are two cruft packs in
> the repository: one that is 99MB and another that is 1MB in size. Let's
> suppose further that if you combine these two packs, the resulting pack
> would be exactly 100MB in size.
>
> Today, repack will say, "I have two packs that sum together to be the
> value of --max-cruft-size", and mark them both to be removed (and
> replaced with the combined pack generated by pack-objects).

... yes, this logic to reach the above decision is exactly what I
said is broken.  Is there no way to fix that?

> But if the
> combined pack is exactly 100MB, then pack-objects will break the pack
> into two just before the 100MB limit, and we'll end up with the same two
> packs we started with.

If "the majority of packs should all be removed and remainder combined"
you stated earlier is true, then this case falls in a tiny minority
that we do not have to worry about, doesn't it?

> Ideally we would combine those packs into one that is at most one
> object's size larger than the threshold, and the steady state would be
> to avoid repacking it further.

I do not see why you can call that "Ideally".  Ideally, we would
combine those packs to create a pack (or two) without busting the
threshold, *and* avoid needress repacking.  Busting the given limit
should not be part of the definition of "ideal" solution.

> But in current Git we will keep repacking
> the two together, only to generate the same two packs we started with
> forever.

Yes.  That is because the logic that decides these packs need to be
broken and recombined is flawed.  Maybe it does not have sufficient
information to decide that it is no use to attempt combining them,
in which case leaving some more info to help the later invocation of
repack to tell that it would be useless to attempt combining these
packs when you do the initial repack would help, which was what I
suggested.  You've thought about the issue much longer than I did,
and would be able to come up with better ideas.