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 04:36:17 +0200 [thread overview]
Message-ID: <87pmtifp2e.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <87y286fs3v.fsf@evledraar.gmail.com>
On Thu, Sep 09 2021, Ævar Arnfjörð Bjarmason wrote:
> 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/
Also: That avar-tb/idx-rename-race-3 has a bug where I didn't update the
bulk-checkin.c code accordingly, missed it because I was running a glob
of tests that included "*pack*,*rev*", but missed "*large*". I've got a
avar-tb/idx-rename-race-4 which fixes it. Sorry about that.
https://github.com/git/git/compare/master...avar:avar-tb/idx-rename-race-4
next prev parent reply other threads:[~2021-09-09 2:41 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
2021-09-09 2:36 ` Ævar Arnfjörð Bjarmason [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pmtifp2e.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.
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.