git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, peff@peff.net, gitster@pobox.com
Subject: Re: [PATCH v2 0/3] prevent opening packs too early
Date: Thu, 09 Sep 2021 03:33:02 +0200	[thread overview]
Message-ID: <87y286fs3v.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <YTlfp/tRmNy0kt+b@nand.local>


On Wed, Sep 08 2021, Taylor Blau wrote:

> On Thu, Sep 09, 2021 at 02:50:59AM +0200, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Thu, Sep 09 2021, Ævar Arnfjörð Bjarmason wrote:
>>
>> > On Wed, Sep 08 2021, Taylor Blau wrote:
>> >
>> >> Here is a very small reroll of a couple of patches to make sure that packs are
>> >> not read before all of their auxiliary files exist and are moved into place, by
>> >> renaming the .idx file into place last.
>> >>
>> >> New since the original version is a patch to apply similar treatment to
>> >> index-pack, as noticed by Peff in [1]. This should be queued before Ævar's
>> >> series on top.
>> >
>> > I read Junio's earlier <xmqq8s063m7m.fsf@gitster.g>[1] as a suggestion
>> > that we combine the two into a singe series. I do think that yields a
>> > better end-result, in particular your 1/3 is much more readable if the
>> > refactoring in my 2/4.
>> >
>> > I'm mot of the way done with such a rewrite, which also incorporates
>> > your suggestion for how to manage memory in rename_tmp_packfile(), but
>> > I'll hold of on what you & Junio have to say about next steps before
>> > adding to the re-roll competition Junio mentioned...
>> >
>> > 1. https://lore.kernel.org/git/xmqq8s063m7m.fsf@gitster.g
>>
>> I've got that at
>> https://github.com/git/git/compare/master...avar:avar-tb/idx-rename-race-3
>
> Beautiful :-).
>
> I mentioned in my response to [1] that I missed that message when
> sending v2 of my series to fix a couple of these races. And I was even
> happy to unify our two series, but you did all of the work for me while
> I was eating dinner. Thank you!
>
> I fetched your branch, reviewed, and am happy with the result. So I
> would be content to apply my s-o-b to your patches and resubmit them as
> a unified series.

Sounds good to me, also in particular any typo/grammar etc. fixes to my
changes are most welcome, I tend to sneak those in, and any code changes
you see fit of course.

> But I did wonder if you wanted to include [2] in this series as well.
> It's kind of different, but also related enough that it probably makes
> sense to just pull it in. So I'm inclined to just do that, unless you
> have any objections.

In this latest push-out of "next" Junio's merged that one down already,
see 1972c5931bb (Merge branch 'ab/reverse-midx-optim' into next,
2021-09-08).

So I think at this point it could be built on top of that, but given
that the two don't conflict in any way (textually or semantically) it's
probably better to base this larger topic on "master" and proceed with
them independently.

> [2]: https://lore.kernel.org/git/patch-1.1-366ba928bd-20210908T010743Z-avarab@gmail.com/


  reply	other threads:[~2021-09-09  1:35 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01  2:05 [PATCH 0/2] pack-write,repack: prevent opening packs too early Taylor Blau
2021-09-01  2:06 ` [PATCH 1/2] pack-write.c: rename `.idx` file into place last Taylor Blau
2021-09-01  2:06 ` [PATCH 2/2] builtin/repack.c: move `.idx` files " Taylor Blau
2021-09-01  3:53 ` [PATCH 0/2] pack-write,repack: prevent opening packs too early Jeff King
2021-09-01  4:29   ` Taylor Blau
2021-09-01  4:59     ` Jeff King
2021-09-01  5:12       ` Taylor Blau
2021-09-01  6:08         ` Jeff King
2021-09-01 21:40           ` Taylor Blau
2021-09-07 16:07             ` Jeff King
2021-09-07 19:42 ` [PATCH 0/3] rename *.idx file into place last (also after *.bitmap) Ævar Arnfjörð Bjarmason
2021-09-07 19:42   ` [PATCH 1/3] pack-write: use more idiomatic strbuf usage for packname construction Ævar Arnfjörð Bjarmason
2021-09-07 22:21     ` Taylor Blau
2021-09-07 23:22       ` Ævar Arnfjörð Bjarmason
2021-09-07 19:42   ` [PATCH 2/3] pack-write: split up finish_tmp_packfile() function Ævar Arnfjörð Bjarmason
2021-09-07 22:28     ` Taylor Blau
2021-09-07 19:42   ` [PATCH 3/3] pack-write: rename *.idx file into place last (really!) Ævar Arnfjörð Bjarmason
2021-09-07 22:31     ` Taylor Blau
2021-09-07 22:36   ` [PATCH 0/3] rename *.idx file into place last (also after *.bitmap) Taylor Blau
2021-09-07 19:48 ` [PATCH 0/2] pack-write,repack: prevent opening packs too early Ævar Arnfjörð Bjarmason
2021-09-08  0:38 ` [PATCH v2 0/4] rename *.idx file into place last (also after *.bitmap) Ævar Arnfjörð Bjarmason
2021-09-08  0:38   ` [PATCH v2 1/4] pack.h: line-wrap the definition of finish_tmp_packfile() Ævar Arnfjörð Bjarmason
2021-09-08  0:38   ` [PATCH v2 2/4] pack-write: refactor renaming in finish_tmp_packfile() Ævar Arnfjörð Bjarmason
2021-09-08  4:22     ` Taylor Blau
2021-09-08  0:38   ` [PATCH v2 3/4] pack-write: split up finish_tmp_packfile() function Ævar Arnfjörð Bjarmason
2021-09-08  0:38   ` [PATCH v2 4/4] pack-write: rename *.idx file into place last (really!) Ævar Arnfjörð Bjarmason
2021-09-08  1:14     ` Ævar Arnfjörð Bjarmason
2021-09-08  9:18       ` Ævar Arnfjörð Bjarmason
2021-09-08  4:24     ` Taylor Blau
2021-09-08 22:17 ` [PATCH v2 0/3] prevent opening packs too early Taylor Blau
2021-09-08 22:17   ` [PATCH v2 1/3] pack-write.c: rename `.idx` files into place last Taylor Blau
2021-09-08 22:17   ` [PATCH v2 2/3] builtin/repack.c: move " Taylor Blau
2021-09-08 22:17   ` [PATCH v2 3/3] builtin/index-pack.c: " Taylor Blau
2021-09-08 23:52   ` [PATCH v2 0/3] prevent opening packs too early Ævar Arnfjörð Bjarmason
2021-09-09  0:50     ` Ævar Arnfjörð Bjarmason
2021-09-09  1:13       ` Taylor Blau
2021-09-09  1:33         ` Ævar Arnfjörð Bjarmason [this message]
2021-09-09  2:36           ` Ævar Arnfjörð Bjarmason
2021-09-09  2:49             ` Taylor Blau
2021-09-09  3:24 ` [PATCH 0/9] packfile: avoid .idx rename races Taylor Blau
2021-09-09  3:24   ` [PATCH 1/9] pack.h: line-wrap the definition of finish_tmp_packfile() Taylor Blau
2021-09-09  3:24   ` [PATCH 2/9] bulk-checkin.c: store checksum directly Taylor Blau
2021-09-09  7:38     ` Ævar Arnfjörð Bjarmason
2021-09-09  3:24   ` [PATCH 3/9] pack-write: refactor renaming in finish_tmp_packfile() Taylor Blau
2021-09-09 19:29     ` Junio C Hamano
2021-09-09 21:07       ` Taylor Blau
2021-09-09 23:30         ` Junio C Hamano
2021-09-09 23:31           ` Taylor Blau
2021-09-10  1:29         ` Junio C Hamano
2021-09-09  3:24   ` [PATCH 4/9] pack-write.c: rename `.idx` files after `*.rev` Taylor Blau
2021-09-09  7:46     ` Ævar Arnfjörð Bjarmason
2021-09-09 14:37       ` Taylor Blau
2021-09-09 19:32     ` Junio C Hamano
2021-09-09  3:25   ` [PATCH 5/9] builtin/repack.c: move `.idx` files into place last Taylor Blau
2021-09-09 19:38     ` Junio C Hamano
2021-09-09 21:08       ` Taylor Blau
2021-09-09  3:25   ` [PATCH 6/9] index-pack: refactor renaming in final() Taylor Blau
2021-09-09 19:45     ` Junio C Hamano
2021-09-09 21:11       ` Taylor Blau
2021-09-09  3:25   ` [PATCH 7/9] builtin/index-pack.c: move `.idx` files into place last Taylor Blau
2021-09-09  7:52     ` Ævar Arnfjörð Bjarmason
2021-09-09 19:45     ` Junio C Hamano
2021-09-09  3:25   ` [PATCH 8/9] pack-write: split up finish_tmp_packfile() function Taylor Blau
2021-09-09  3:25   ` [PATCH 9/9] pack-objects: rename .idx files into place after .bitmap files Taylor Blau
2021-09-09  7:54     ` Ævar Arnfjörð Bjarmason
2021-09-09 19:52       ` Junio C Hamano
2021-09-09 21:13         ` Taylor Blau
2021-09-09  8:06   ` [PATCH 0/9] packfile: avoid .idx rename races Ævar Arnfjörð Bjarmason
2021-09-09 14:40     ` Taylor Blau
2021-09-09 19:52       ` Junio C Hamano
2021-09-09 23:24   ` [PATCH v2 " Taylor Blau
2021-09-09 23:24     ` [PATCH v2 1/9] pack.h: line-wrap the definition of finish_tmp_packfile() Taylor Blau
2021-09-09 23:24     ` [PATCH v2 2/9] bulk-checkin.c: store checksum directly Taylor Blau
2021-09-09 23:24     ` [PATCH v2 3/9] pack-write: refactor renaming in finish_tmp_packfile() Taylor Blau
2021-09-09 23:24     ` [PATCH v2 4/9] pack-write.c: rename `.idx` files after `*.rev` Taylor Blau
2021-09-09 23:24     ` [PATCH v2 5/9] builtin/repack.c: move `.idx` files into place last Taylor Blau
2021-09-09 23:24     ` [PATCH v2 6/9] index-pack: refactor renaming in final() Taylor Blau
2021-09-09 23:24     ` [PATCH v2 7/9] builtin/index-pack.c: move `.idx` files into place last Taylor Blau
2021-09-09 23:24     ` [PATCH v2 8/9] pack-write: split up finish_tmp_packfile() function Taylor Blau
2021-09-09 23:25     ` [PATCH v2 9/9] pack-objects: rename .idx files into place after .bitmap files Taylor Blau
2021-09-10  1:35     ` [PATCH v2 0/9] packfile: avoid .idx rename races Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=87y286fs3v.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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).