All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Patrick Steinhardt <ps@pks.im>
Cc: Elijah Newren <newren@gmail.com>,
	Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 2/2] packfile: recover when a multi-pack-index names a removed pack
Date: Mon, 24 Aug 2026 03:06:01 -0400	[thread overview]
Message-ID: <20260824070601.GC149254@coredump.intra.peff.net> (raw)
In-Reply-To: <20260824065539.GA149254@coredump.intra.peff.net>

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

> 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

  reply	other threads:[~2026-08-24  7:06 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-18 22:34 [PATCH 0/2] Objects treated as missing despite being present, due to race with geometric repacking Elijah Newren via GitGitGadget
2026-08-18 22:34 ` [PATCH 1/2] replay: fail gracefully when a merge input is unreadable Elijah Newren via GitGitGadget
2026-08-19 18:09   ` Junio C Hamano
2026-08-21  1:44     ` Elijah Newren
2026-08-21  3:37       ` Junio C Hamano
2026-08-18 22:34 ` [PATCH 2/2] packfile: recover when a multi-pack-index names a removed pack Elijah Newren via GitGitGadget
2026-08-19 18:21   ` Junio C Hamano
2026-08-20  7:54   ` Patrick Steinhardt
2026-08-21  1:36     ` Elijah Newren
2026-08-24  4:48       ` Jeff King
2026-08-24  5:13         ` Patrick Steinhardt
2026-08-24  6:55           ` Jeff King
2026-08-24  7:06             ` Jeff King [this message]
2026-08-24  7:23               ` Jeff King
2026-08-24  4:55   ` Jeff King
2026-08-24  5:40     ` Patrick Steinhardt
2026-08-24  7:03       ` Jeff King
2026-08-24 14:45   ` Derrick Stolee
2026-08-24 14:46   ` Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260824070601.GC149254@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.