Re: [PATCH 0/6] refs: introduce support for partial reference transactions

[Patrick Steinhardt <ps@pks.im>]

On Wed, Feb 19, 2025 at 02:34:15PM +0000, Phillip Wood wrote:
> On 12/02/2025 12:34, Karthik Nayak wrote:
> > Thinking this out, having a different interface sound good, but I feel
> > we'd end up with the same structure as currently presented in this
> > series. Only other way is to really split the implementation to support
> > partial transactions as a entity of its own. In that case, we'd end up
> > with code duplication.
> > 
> > Do you think you can expand a little more here?
> 
> I was thinking of a function that took a list of refs to update and made a
> best effort to update them, ignoring any updates that fail.
> 
> My concern with adding a flag to ignore errors in the transaction api is
> that a partial transaction is a contradiction in terms. I'm also concerned
> that it seems to be ignoring all errors. I'd be happier if there was someway
> for the caller to specify which errors to ignore or if the caller could
> provide a callback to handle any errors. That way a caller could ignore d/f
> conflicts but still cause the transaction to fail if there was an i/o or
> could create a reference if it did not exist but leave it unchanged if it
> did exist.

This is a fair point I think, and it's also something that I called out
in [1]. We shouldn't blanket-ignore all errors, but instead only ignore
a well-known subset of errors and then record the exact failure reason.
This allows for better and more unified error reporting, and would also
allow us to easily build mechanisms where callers can specify that we
are only expected to ignore a subset of those well-defined errors.

But I also think that this is another selling point for continuing to
build on top of the ref transaction. If we now want to record and ignore
specific errors, only, then this falls out quite naturally from the
current design of ref transactions. We already have all the ref updates
in there, so we can then simply set an `enum ref_transaction_error`
field for each of the failed updates.

Eventually, I think we could also allow for modes where we declare only
a subset of reference updates to be allowed to fail. I don't have a
specific usecase for this, and don't think it needs to be implemented
without one. But it's another thing that we could implement on top of
transactions rather trivially.

Patrick

[1]: <Z6YxA4BlhNwbeYk-@pks.im>