Re: [PATCH 1/6] bulk-checkin: remove ODB transaction nesting

From: Justin Tobler <jltobler@gmail.com>
Date: Sep 16, 2025, 17:47 UTC
Message-ID: <vy7ah7cvv2xtq74h4zn2peqnyprd672v5pkfxskduyznrcpp2x@uqs3ksrhw6z3>
In-Reply-To: <xmqqfrcmny8r.fsf@gitster.g>

On 25/09/16 09:44AM, Junio C Hamano wrote:

Show 7 quoted lines

> Justin Tobler <jltobler@gmail.com> writes:
> > Yes, this patch removes the logic that manages the internal nested
> > transaction counter in favor of requiring callers to check if a
> > transaction has already been started or not.
> 
> OK.  Is that related to those "make it a silent no-op to pass a
> NULL, instead of forcing the caller to check" changes?

The first version of this patch initially required {begin,end}_odb_transaction() callers to manually ensure there was not already a pending transaction. If this was not ensured, it could BUG() out.

Going forward, I think it probably makes sense to change to a silent no-op passing a NULL transaction value around. This should keep the existing callers mostly unchanged and pose less of a burden while still being able to drop the transaction nesting mechanism.

Show 5 quoted lines

> Can all callers tell reliably if there is a transaction already
> going?  One transaction may start somewhere and end in a far away
> place, and these two places may not be ancestor-decendant in the
> call graph but merely a distant relative that shares a grand grand
> parent.

To start a transaction, callers must have access to `struct object_directory`. To know if a transaction is already pending, the `transaction` field of this structure can be checked.

Ending a transaction does require the pending transaction be provided to it. All existing {begin,end}_odb_transaction() call sites don't currently have a problem with this though as they are relatively close together. If we wanted to end a transaction somewhere else far away from the start, the pending transaction would need to be either wired or accessed through the repository/odb.

> > The ultimate goal of this patch is make it so invoking
> > end_odb_transaction() on a transaction guarantees it is flushed.
> 
> Either flushed or a noop, I guess?

Yes. If the pending transaction is specified, it should always be committed. If a NULL transaction value is passed, it should function as a no-op.

-Justin