From: Justin Tobler Date: Tue, 16 Sep 2025 17:47:07 GMT Subject: Re: [PATCH 1/6] bulk-checkin: remove ODB transaction nesting Message-ID: In-Reply-To: On 25/09/16 09:44AM, Junio C Hamano wrote: > Justin Tobler 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. > 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