Re: [PATCH 1/1] Add preparing state to reference-transaction hook

>
> eric.peijian@gmail.com writes:
>
> > From: Eric Ju <eric.peijian@gmail.com>
> >
> > From: Eric Ju <eju@gitlab.com>
>
> This is curious.  The former matches the sign-off, but I somehow
> suspect that the @gitlab.com identity may be what you want to use
> for both of them, if this is a company sponsored work by an
> employee?  I dunno.
>

> Also the commit title deviates from the established "<area>: <what
> is done>" format.
>
>     Subject: [PATCH] refs: add 'preparing" phase to the transaction hook
>
> or something?
>

> > The "reference-transaction" hook is invoked multiple times during a ref
> > transaction. Each invocation corresponds to a different phase:
> >
> > - The "prepared" phase indicates that references have been locked.
> > - The "commit" phase indicates that all updates have been written to disk.
> > - The "abort" phase indicates that the transaction has been aborted and that
> >   all changes have been rolled back.
>
> "commit" -> "committed" and "abort" -> "aborted", if the existing
> documentation is to be trusted.
>

> > This hook can be used to learn about the updates that Git wants to perform.
> > For example, forges use it to coordinate reference updates across multiple
> > nodes.
> >
> > However, the phases are insufficient for some specific use cases. The earliest
> > observable phase in the "reference-transaction" hook is "prepared", at which
> > point Git has already taken exclusive locks on every affected reference. This
> > makes it suitable for last-chance validation, but not for serialization. So by
> > the time a hook sees the "prepared" phase, it has no way to defer locking, and
> > thus it cannot rearrange multiple concurrent ref transactions relative to one
> > another.
>
> I cannot quite picture how "rearrangement" would happen, though.
>
> Would the hook notice "ah there is a preparing hook invocation
> incoming", stall the caller by not immediately returning and instead
> wait for a different Git process to invoke the same ref-transaction
> hook "preparing" invocation, and somehow decide to let the latter go
> first before releasing the former?
>

> > Introduce a new "preparing" phase that runs before the "prepared" phase, that
> > is before Git acquires any reference lock on disk. This gives callers a
> > well-defined window to perform validation, enable higher-level ordering of
> > concurrent transactions, or reject the transaction entirely, all without
> > interfering with the locking state.
> >
> > This change is strictly speaking not backwards compatible. Existing hook
> > scripts that do not know to handle unknown phases handle the "preparing" state
>
> "know to handle unknown phrases handle"?
>

> > string will encounter an unknown phase, and that might cause them to return an
> > error now. But the hook is considered to expose internal implementation details
> > of how Git works, and as such we have been a bit more lenient with changing its
> > exact semantics, like for example in a8ae923f85 (refs: support symrefs in
> > 'reference-transaction' hook, 2024-05-07).
> >
> > An alternative would be to introduce a "reference-transaction-v2" hook that
> > knows about the new phase. This feels like a rather heavy-weight option though,
> > and was thus discarded.
>
> And documenting the design alternatives and decision like these two
> paragraphs is very much appreciated.
>
> The insertion of a new hook invocation itself is at a very much
> expected place in the code path.  Well written.
>
> Will queue.  Thanks.