mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <>
To: Derrick Stolee <>
Cc: Taylor Blau <>,, Junio C Hamano <>,
	Victoria Dye <>
Subject: Re: [PATCH] midx.c: clear auxiliary MIDX files first
Date: Thu, 27 Oct 2022 16:28:17 -0400	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Wed, Oct 26, 2022 at 09:31:28AM -0400, Derrick Stolee wrote:

> This was always possible before, too: the midx could be read by a
> reader process before the repack process deletes that file. However,
> if the reader does not also gain a handle on the corresponding
> .bitmap before the repack process deletes that file, too, then the
> reader is also left thinking that no .bitmap exists.

Good point. Neither the writer _nor_ the reader is atomic. :)

> I think the old code is more correct, here. The window is slightly
> smaller, and the new code creates a window where the filesystem
> doesn't need to change for readers to get an imperfect view of
> things.

Yeah, I agree that the old code is nicer in that the window is a little

Do we want to do something about the warning, though? Falling back to a
slow path sucks, of course, but racily generating user-visible warnings
for something that is not actually a problem is also not great.

> Aside: in these cases where a .bitmap file is not found for a midx,
> do we fall back to trying to find a .bitmap file for a pack-file?
> That would assuage most of the concerns here about what happens in
> this window where the repack has a new .pack/.bitmap pair but the
> old midx still exists (without a .bitmap, depending on timing).

Yes, we should. Code paths generally go through open_bitmap(), which
tries the midx first, then looks for pack bitmaps.

And in that sense, the race after this patch is fairly harmless. We fail
to see the midx bitmap, but we'll see the pack one (which must have been
created before we deleted the midx one, assuming this is a "git repack
-adb" or equivalent).

Is that also true of the race before this patch? I'm not sure which
warning is being generated, but if it's in open_midx_bitmap_1(), then
the same logic would apply.


      parent reply	other threads:[~2022-10-27 20:29 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-25 18:25 [PATCH] midx.c: clear auxiliary MIDX files first Taylor Blau
2022-10-26  5:41 ` Jeff King
2022-10-26 13:31   ` Derrick Stolee
2022-10-26 19:59     ` Taylor Blau
2022-10-27 20:28     ` Jeff King [this message]

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:

  List information:

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

  git send-email \ \ \ \ \ \ \ \

* 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.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).