All of
 help / color / mirror / Atom feed
From: "René Scharfe" <>
To: "Ævar Arnfjörð Bjarmason" <>,
	"Bryan Turner" <>
Cc: Git Users <>, Patrick Steinhardt <>
Subject: Re: reference-transaction regression in 2.36.0-rc1
Date: Wed, 13 Apr 2022 18:21:47 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Am 13.04.22 um 16:34 schrieb Ævar Arnfjörð Bjarmason:
> On Tue, Apr 12 2022, Bryan Turner wrote:
>> On Tue, Apr 12, 2022 at 2:20 AM Bryan Turner <> wrote:
>>> It looks like Git 2.36.0-rc1 includes the changes to eliminate/pare
>>> back reference transactions being raised independently for loose and
>>> packed refs. It's a great improvement, and one we're very grateful
>>> for.
>>> It looks like there's a regression, though. When deleting a ref that
>>> _only_ exists in packed-refs, by the time the "prepared" callback is
>>> raised the ref has already been deleted completely. Normally when
>>> "prepared" is raised the ref is still present. The ref still being
>>> present is important to us, since the reference-transaction hook is
>>> frequently not passed any previous hash; we resolve the ref during
>>> "prepared", if the previous hash is the null SHA1, so that we can
>>> correctly note what the tip commit was when the ref was deleted.
>> I've re-tested this with 2.36.0-rc2 and it has the same regression (as
>> expected). However, in playing with it more, the regression is more
>> serious than I had initially considered. It goes beyond just losing
>> access to the SHA of the tip commit for deleted refs. If a ref only
>> exists packed, this regression means vetoing the "prepared" callback
>> _cannot prevent its deletion_, which violates the contract for the
>> reference-transaction as I understand it.
>> Here's a slightly modified reproduction:
>> git init ref-tx
>> cd ref-tx
>> git commit -m "Initial commit" --allow-empty
>> git branch to-delete
>> git pack-refs --all
>> echo 'exit 1;' > .git/hooks/reference-transaction
>> chmod +x .git/hooks/reference-transaction
>> git branch -d to-delete
>> Running this reproduction ends with:
>> $ git branch -d to-delete
>> fatal: ref updates aborted by hook
>> $ git rev-parse to-delete --
>> fatal: bad revision 'to-delete'
>> Even though the reference-transaction vetoed "prepared", the ref was
>> still deleted.
>> In Bitbucket, we use the reference-transaction to perform replication.
>> When we get the "prepared" callback on one machine, we dispatch the
>> same change(s) to other replicas. Those replicas process the changes
>> and arrive at their own "prepared" callbacks (or don't), at which
>> point they vote to commit or rollback. The votes are tallied and the
>> majority decision wins.
>> With this regression, that sort of setup no longer works reliably for
>> ref deletions. If the ref only exists packed, it's deleted (and
>> _visibly_ deleted) before we ever get the "prepared" callback. So if
>> coordination fails (i.e. the majority votes to rollback), even if we
>> try to abort the change it's already too late.
> This does look lik a series regression.
> I haven't had time to bisect this, but I suspect that it'll come down to
> something in the 991b4d47f0a (Merge branch
> 'ps/avoid-unnecessary-hook-invocation-with-packed-refs', 2022-02-18)
> series.

Indeed, it bisects to 2ed1b64ebd (refs: skip hooks when deleting
uncovered packed refs, 2022-01-17).

> I happen to know that Patrick is OoO until after the final v2.36.0 is
> scheduled (but I don't know for sure that we won't spot this thread &
> act on it before then).
> Is this something you think you'll be able to dig into further and
> possibly come up with a patch? It looks like you're way ahead of at
> least myself in knowing how this *should* work :)

  reply	other threads:[~2022-04-13 16:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-12  9:20 reference-transaction regression in 2.36.0-rc1 Bryan Turner
2022-04-12 20:53 ` Bryan Turner
2022-04-13 14:34   ` Ævar Arnfjörð Bjarmason
2022-04-13 16:21     ` René Scharfe [this message]
2022-04-13 19:52     ` Junio C Hamano
2022-04-13 22:44       ` Junio C Hamano
2022-04-13 22:55         ` Bryan Turner
2022-04-13 22:56         ` Junio C Hamano
2022-04-13 23:31           ` Junio C Hamano
2022-04-15  2:37       ` Bryan Turner
2022-04-15 13:27         ` Ævar Arnfjörð Bjarmason
2022-04-15 16:53           ` Junio C Hamano
     [not found]             ` <>
2022-04-27 11:05               ` Patrick Steinhardt
     [not found]                 ` <>
2022-05-02 11:12                   ` Patrick Steinhardt

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:

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