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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Phillip Wood <phillip.wood123@xxxxxxxxx> writes:

> Hi Karthik
>
> On 07/02/2025 07:34, Karthik Nayak wrote:
>> Git's reference updates are traditionally atomic
>
> I'm nitpicking but the updates aren't actually atomic, if a transaction
> updates two refs then it is possible for another process to see the one
> ref pointing to the new value and the other pointing to the old value.
>

Good point. This is true in the case of the files backend, since updates
involve locking individual files and during the commit phase, there is a
possibility that one ref is updated while the other is yet to be
(committing of the lock is not global but rather per ref file).

However this is not the case with the reftable backend, there, updates
are written to a new table and committed at the end after locking the
table. So in the reftable backend, this is indeed atomic.

>> - when updating
>> multiple references in a transaction, either all updates succeed or none
>> do. While this behavior is generally desirable,
>
> Isn't that the whole point of transactions?
>

Yup, this is the point of having a transaction indeed.

>> it can be limiting in> certain scenarios, particularly with the reftable backend where batching
>> multiple reference updates is more efficient than performing them
>> sequentially.
>>
>> This series introduces support for partial reference transactions,
>> allowing individual reference updates to fail while letting others
>> proceed.
>
> This sounds like it's abusing ref transactions to implement a
> performance optimization.

I understand where you're coming from. This is definitely a stray from
the regular atomic behavior, that transactions promise. But I would say
this is more of an exception handling for the regular transaction
mechanism and AFAIK this is also something that some of the databases
support (see EXCEPTION in PostgreSQL).

Overall, we're adding an exception handling support to the existing
transaction interface.

> I wonder if it would be better to provide that
> via a different interface than shares the same underling implementation
> as transactions. That would make it clear to someone reading the code
> that individual ref updates can fail without affecting the rest. Burying
> that detail in a flag makes it rather easy to miss.
>

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?

> Best Wishes
>
> Phillip
>

Thanks for engaging!

[snip]

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux