On Wed, May 03, 2023 at 05:22:04PM -0400, Taylor Blau wrote:

quoted

OK, I understand the use case you're trying to support, and your
approach mostly makes sense. But there are two things I was surprised by
in the implementation:

  1. Does this need to be tied to cruft packs? The same logic would
     apply to "repack -A" which turns objects loose (of course you
     probably don't want to do that in the long term for performance
     reasons, but it's conceptually the same thing; see below).

  2. Why is there a separate walk distinct from the one that rescues
     recent-but-unreachable objects?

quoted

Conceptually it seems to me that the simplest and most flexible way to
think of this new feature is: pretend these objects are recent enough to
be kept in the grace period, even though their mtimes do not qualify".

And then everything else would just fall out naturally. Am I missing
something?

I originally shied away from it, thinking that I wouldn't want to do an
expensive walk with all of the recent objects during a non-pruning
repack.

The two code paths are quite different in practice. In the pruning case,
we add only new objects from the kept packs and then start our traversal
there. In the non-pruning case, we just do
`add_objects_in_unpacked_packs()` which is really just a call to
`for_each_packed_object()`.

Right, that's what I'd expect. I think you are describing a cruft-pack
world here (because you say "kept packs"), but the traditional "repack
-k" versus "repack -A" is similar (if we are not doing something special
with recent objects, then there is no need to figure out what they
reach; we can just add them all).

So it gets tricky when you have a pack.extraCruftTips program and want
to invoke it in a non-pruning case. You could do something like:

  - call enumerate_and_traverse_cruft_objects() *always*, either because
    we were doing a pruning GC, or calling it after
    `enumerate_cruft_objects()` (in the non-pruning case)

  - ensure that enumerate_and_traverse_cruft_objects() is a noop when
    (a) cruft_expiration is set to zero, and (b) there are no
    pack.extraCruftTips programs specified

I'm not sure why you'd need to traverse, though. If we are in "-k" mode,
we are keeping everything anyway (so I don't even see the point of
asking the helper about extra tips). And all of those objects that are
not reachable from the regular traversal are by definition "cruft" and
go into the cruft pack.

Maybe I don't understand what you mean by "non-pruning" here.

-Peff