On Tue, Oct 03, 2023 at 12:27:51PM -0400, Taylor Blau wrote: > Note that this behavior is usually just a performance regression. But > it's possible it could be a correctness issue. > > Suppose an object was duplicated among the cruft and non-cruft pack. The > MIDX will pick the one from the pack with the lowest mtime, which will > always be the cruft one. But if the non-cruft pack happens to sort > earlier in lexical order, we'll treat that one as preferred, but not all > duplicates will be resolved in favor of that pack. > > So if we happened to have an object which appears in both packs > (e.g., due to a cruft object being freshened, causing it to appear > loose, and then repacking it via the `--geometric` repack) it's possible > the duplicate would be picked from the non-preferred pack. I'm not sure I understand how that is a correctness issue. The contents of the object are the same in either pack. Or do you mean that the pack-reuse code in pack-objects.c may get confused and try to use the wrong pack/offset when sending the object out? I would think it would always be coming from the preferred pack for that code (so the outcome is just that we fail to do the pack-reuse optimization very well, but we don't generate a wrong answer). Other than that, the explanation and patch make perfect sense to me, and the patch looks good. -Peff
