Re: [PATCH 2/2] packfile: recover when a multi-pack-index names a removed pack

From: Jeff King <peff@peff.net>
Date: Aug 24, 2026, 07:06 UTC
Message-ID: <20260824070601.GC149254@coredump.intra.peff.net>
In-Reply-To: <20260824065539.GA149254@coredump.intra.peff.net>

On Mon, Aug 24, 2026 at 02:55:39AM -0400, Jeff King wrote:

Show 17 quoted lines

> Right. It would be OK to skip Elijah's fallback workaround when
> SECOND_READ is not set; the QUICK callers are prepared to accept the
> false negative. But since it is cheap-ish to do the fallback check, it
> is perhaps OK to just do it on the first pass?
> 
> I wonder how true that is. Imagine you had a midx covering a million
> packs, and you notice an object is missing, but you're in QUICK mode. Do
> you really want to individually check each of those million pack idx
> files (that were otherwise not even opened or mmap'd because they're
> covered by the midx!).
> 
> I think it's mostly academic. You'd have to do the million-pack search
> if we are not in QUICK mode. And the point of QUICK mode is mostly
> avoiding tons of fruitless searches for objects we don't actually have.
> The bsearch() conditional means that we _know_ this is a racy negative
> and not just some object we never even had. So it would trigger
> generally only when the search is useful.

Actually, thinking on this more: we _don't_ usually scan the million packs for an object we actually have. If the object is available in a new pack, the SECOND_READ scan should find that pack and put it at the front of the packfile list (because they sort by reverse mtime), and we'd find the object immediately, without having to open the new packs.

It's only the case that this patch is helping (when the object is not moved at all, but an existing duplicate is hidden in the midx) where we have to re-scan all of those packs. But we don't know which case is which until we get to the SECOND_READ stage. So I think this probably should only kick in for SECOND_READ.

-Peff