From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
haoyurenzhuxia@gmail.com, git@vger.kernel.org,
derrickstolee@github.com, dyroneteng@gmail.com
Subject: Re: [RFC PATCH] midx.c: clean up .rev file
Date: Wed, 22 Jun 2022 12:58:42 -0700 [thread overview]
Message-ID: <xmqqtu8c31xp.fsf@gitster.g> (raw)
In-Reply-To: <YrNb2x2/7Z31XnFJ@nand.local> (Taylor Blau's message of "Wed, 22 Jun 2022 14:13:47 -0400")
Taylor Blau <me@ttaylorr.com> writes:
> - the MIDX file itself is written using a lock_file, so it is
> atomically moved into place, and the temporary file is either
> removed, or cleaned up automatically with a sigchain handler on
> process death
Good.
> - the bitmap (written in bitmap_writer_finish(), which is the path for
> both single- and multi-pack bitmaps) is written to a temporary file
> and moved into place after the bitmaps are written.
>
> ...but this temporary file isn't automatically cleaned up, so it
> could stick around after process death. Luckily the race window here
> is pretty small, since all of the bitmaps have been computed already
> and are held in memory.
>
> This is probably worth a cleanup on its own, too.
As long as the "temporary file" is clearly a temporary file that
"gc" can recognize and get rid of, it would be OK, I would think.
> - unless GIT_TEST_MIDX_WRITE_REV=1 is in your environment, we won't
> *write* a .rev file, hence this is pretty rare to deal with in
> practice.
OK, but if we were to write one, we should do the same "write into a
temporary, rename it in place" dance, right? Or is a separate .rev
file is pretty much a thing of last decade that we do not have to
worry too much about?
> So I think there are two things worth doing here:
>
> - make sure that the temporary file used to stage the .bitmap is a
> lock_file
Yes.
> - use a temporary file to stage the .rev file (when forced to write
> one), and ensure that that too is a lock_file
Yes.
Thanks.
next prev parent reply other threads:[~2022-06-22 19:58 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-22 11:50 [RFC PATCH] midx.c: clean up .rev file haoyurenzhuxia
2022-06-22 15:56 ` Ævar Arnfjörð Bjarmason
2022-06-22 17:53 ` Junio C Hamano
2022-06-22 18:13 ` Taylor Blau
2022-06-22 19:58 ` Junio C Hamano [this message]
2022-06-22 21:31 ` Taylor Blau
2022-06-27 5:05 ` Xiaowen Xia
2022-06-23 12:38 ` Teng Long
2022-06-27 3:53 ` Xiaowen Xia
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=xmqqtu8c31xp.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=derrickstolee@github.com \
--cc=dyroneteng@gmail.com \
--cc=git@vger.kernel.org \
--cc=haoyurenzhuxia@gmail.com \
--cc=me@ttaylorr.com \
/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.