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

> On Tue, Sep 09, 2025 at 02:11:29PM -0500, Justin Tobler wrote:
> > ODB transactions support being nested. Only the outermost
> > {begin,end}_odb_transaction() start and finish a transaction. This is
> > done so that certain object write codepaths that occur internally can be
> > optimized via ODB transactions without having to worry if a transaction
> > has already been started or not. This can make the interface a bit
> > awkward to use, as calling {begin,end}_odb_transaction() does not
> > guarantee that a transaction is actually started or ended.
> > 
> > Instead, be more explicit and require callers who use ODB transactions
> > internally to ensure there is not already a pending transaction before
> > beginning or ending a transaction.
> 
> I think one bit missing in the commit message is to explain what this
> buys us. Does it for example enable subsequent changes? Or is this
> really only done to have clean ownership semantics for the transaction?

> > @@ -389,14 +387,6 @@ void flush_odb_transaction(struct odb_transaction *transaction)
> >  
> >  void end_odb_transaction(struct odb_transaction *transaction)
> >  {
> > -	if (!transaction || transaction->nesting == 0)
> > -		BUG("Unbalanced ODB transaction nesting");
> > -
> > -	transaction->nesting -= 1;
> > -
> > -	if (transaction->nesting)
> > -		return;
> > -
> >  	flush_odb_transaction(transaction);
> >  	transaction->odb->transaction = NULL;
> >  	free(transaction);
> 
> But I wonder whether we also want to make this function here a bit more
> robust:
> 
>   - I think we should handle `NULL` pointers here and convert them into
>     a no-op. This allows us to skip the `if (transaction)` checks in
>     many places.