All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Michael Haggerty" <mhagger@alum.mit.edu>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Stefan Beller" <sbeller@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"David Turner" <novalis@novalis.org>,
	"Brandon Williams" <bmwill@google.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] packed_ref_store: handle a packed-refs file that is a symlink
Date: Thu, 27 Jul 2017 12:40:31 -0700	[thread overview]
Message-ID: <xmqqlgn9mzww.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170727182854.wxjvjs2x746n3x2t@sigill.intra.peff.net> (Jeff King's message of "Thu, 27 Jul 2017 14:28:54 -0400")

Jeff King <peff@peff.net> writes:

> On Thu, Jul 27, 2017 at 10:19:48AM -0700, Junio C Hamano wrote:
>
>> Makes sense.  Makes me wonder why we need a separate .new file
>> (instead of writing into the .lock instead), but that is a different
>> issue.
>
> It comes from 42dfa7ece (commit_packed_refs(): use a staging file
> separate from the lockfile, 2017-06-23). That commit explains that we
> want to be able to put the new contents into service before we release
> the lock. But it doesn't say why that's useful.

By being able to hold the lock on packed-refs longer, I guess
something like this becomes possible:

 * hold the lock on packed-refs
 * hold the lock on loose ref A, B, C, ...
 * update packed-refs to include the freshest values of these refs
 * start serving packed-refs without releasing the lock
 * for X in A, B, C...: delete the loose ref X and unlock X
 * unlock the packed-refs

Other people racing with the sequence to recreate a loose ref that
is even fresher than the resulting packed-refs file, while we still
hold the lock on packed-refs, is perfectly OK.

But we must make sure our packed-refs is visible to others before
starting to delete and unlock the loose refs.

Hmph, but that is not a sufficient explanation.  I am not seeing
anything bad to happen if we unlock the packed-refs before deleting
loose refs that we have locks on, so there must be something else
that needs "new packed-refs is made visible way before we unlock it".

> I recall from past discussions that this will help close some races,
> and e5cc7d7d2 (repack_without_refs(): don't lock or unlock the packed
> refs, 2017-07-01) alludes to this. I think the races in question have to
> do with holding the packed-refs lock while pruning the just-packed
> files, but I'm having trouble digging up specifics in the archive.
>
> -Peff

  reply	other threads:[~2017-07-27 19:40 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-01 18:30 [PATCH v3 00/30] Create a reference backend for packed refs Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 01/30] t1408: add a test of stale packed refs covered by loose refs Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 02/30] add_packed_ref(): teach function to overwrite existing refs Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 03/30] packed_ref_store: new struct Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 04/30] packed_ref_store: move `packed_refs_path` here Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 05/30] packed_ref_store: move `packed_refs_lock` member here Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 06/30] clear_packed_ref_cache(): take a `packed_ref_store *` parameter Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 07/30] validate_packed_ref_cache(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 08/30] get_packed_ref_cache(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 09/30] get_packed_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 10/30] add_packed_ref(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 11/30] lock_packed_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 12/30] commit_packed_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 13/30] rollback_packed_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 14/30] get_packed_ref(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 15/30] repack_without_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 16/30] packed_peel_ref(): new function, extracted from `files_peel_ref()` Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 17/30] packed_ref_store: support iteration Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 18/30] packed_read_raw_ref(): new function, replacing `resolve_packed_ref()` Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 19/30] packed-backend: new module for handling packed references Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 20/30] packed_ref_store: make class into a subclass of `ref_store` Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 21/30] commit_packed_refs(): report errors rather than dying Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 22/30] commit_packed_refs(): use a staging file separate from the lockfile Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 23/30] packed_refs_lock(): function renamed from lock_packed_refs() Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 24/30] packed_refs_lock(): report errors via a `struct strbuf *err` Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 25/30] packed_refs_unlock(), packed_refs_is_locked(): new functions Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 26/30] clear_packed_ref_cache(): don't protest if the lock is held Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 27/30] commit_packed_refs(): remove call to `packed_refs_unlock()` Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 28/30] repack_without_refs(): don't lock or unlock the packed refs Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 29/30] t3210: add some tests of bogus packed-refs file contents Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 30/30] read_packed_refs(): die if `packed-refs` contains bogus data Michael Haggerty
2017-07-05  9:12 ` [PATCH v3 00/30] Create a reference backend for packed refs Jeff King
2017-07-20 23:05   ` Stefan Beller
2017-07-20 23:20     ` Jonathan Nieder
2017-07-26 23:39       ` [PATCH] packed_ref_store: handle a packed-refs file that is a symlink Michael Haggerty
2017-07-27  0:15         ` Stefan Beller
2017-07-27  0:18         ` Jonathan Nieder
2017-07-27 11:12           ` Michael Haggerty
2017-07-27 17:19         ` Junio C Hamano
2017-07-27 18:28           ` Jeff King
2017-07-27 19:40             ` Junio C Hamano [this message]
2017-07-28  6:07               ` Michael Haggerty
2021-05-31 14:18         ` Ævar Arnfjörð Bjarmason
2021-06-03 19:39           ` Jeff King
2021-06-03 19:58             ` [PATCH] t: use portable wrapper for readlink(1) Jeff King
2021-06-04 21:09               ` Ævar Arnfjörð Bjarmason
2021-06-03 20:23             ` [PATCH] packed_ref_store: handle a packed-refs file that is a symlink Felipe Contreras
2021-06-03 21:08               ` Jeff King
2021-06-03 22:25                 ` Felipe Contreras
2021-06-04 21:37                 ` Ævar Arnfjörð Bjarmason
2021-06-05  1:07                   ` Felipe Contreras
2021-06-04 21:12             ` Ævar Arnfjörð Bjarmason

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=xmqqlgn9mzww.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --cc=novalis@novalis.org \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.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.