Re: [PATCH v2 3/4] packfile: recover object lookups racing a concurrent repack

On Wed, Aug 26, 2026 at 10:57 PM Jeff King [off-list ref] wrote:

On Tue, Aug 25, 2026 at 07:00:28PM +0000, Elijah Newren via GitGitGadget wrote:

quoted

  1. open_pack_index() fails, so we print

        error: packfile <path> index unavailable

     and report the pack as unusable, even though the object still lives
     in the replacement pack.

  2. A normal lookup recovers: odb_read_object_info_extended() issues a
     second read that reloads the on-disk pack state and finds the object
     in its new home, making the message above mere noise.  But an
     OBJECT_INFO_QUICK lookup deliberately skips that second read to stay
     fast on a genuine miss, so it does *not* recover: it reports the
     object as absent even though it still lives in the replacement pack.
     A resident reader that resolves objects with a QUICK lookup -- such
     as the `git mktree --batch` process the tests below drive -- then
     produces wrong results.  Even where a spurious miss is not fatal it
     is not harmless: `git upload-pack` checks a client's "have" lines
     with a QUICK lookup, and a dropped "have" removes a common object
     from the negotiation, so the client is sent more than it needs.

Maybe I am still being dense, but this description does not make any
sense to me at all.

The _point_ of QUICK is to accept those false negatives. It is the right
thing for upload-pack to do, to avoid re-scans for objects which we
simply don't have (and don't necessarily expect to have).

It's far more likely that I am the one being dense.  My rough line of thinking:

* We see "packfile ... index unavailable" in our logging
* There's only one thing that remove packfiles
* Investigate the mechanism
* Look for other affected callers (e.g. mktree --batch)
* Consider corrective measures

Steps 1-4 above are probably fine, and step 5 may have been where I
went off the rails.  My thinking there, wrong or right, was:

* It makes sense that we don't want to reprepare most of the time
* ...but _if_ we know of the existence of some specific packfile in
this process and that packfile has since disappeared by the time we go
to open or read it, is that a special case?  Should it be?

It sounds like mktree is wrong to be using QUICK at all. It comes from
817b0f6027 (mktree: do not check type of remote objects, 2022-06-21)
which rewrote a call to vanilla oid_object_info(). From the description
there it probably should be using SKIP_FETCH_OBJECT but not QUICK. Or
possibly it should use neither unless --missing is given.

So I don't see QUICK itself here violating any contract (even if it
_could_ find the object in some cases with just a little more work, as
in the case that we were discussing for v1).

I'll drop this patch and instead send a small mktree change that stops
passing OBJECT_INFO_QUICK (keeping SKIP_FETCH_OBJECT), so mktree
recovers via the normal reprepare like every other non-QUICK reader.
That removes the packfile.c changes entirely, so both the
reload-under-QUICK hack and the .idx/.pack raciness you noted in
pack_index_is_missing() go away with them.