[PATCH v3 1/6] bulk-checkin: remove ODB transaction nesting
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Sep 16, 2025, 18:29 UTC
- Message-ID
- <20250916182938.2193476-2-jltobler@gmail.com>
- In-Reply-To
- <20250916182938.2193476-1-jltobler@gmail.com>
ODB transactions support being nested. Only the outermost {begin,end}_odb_transaction() start and finish a transaction. This allows internal object write codepaths to be optimized with ODB transactions without worrying about whether a transaction is already active. When {begin,end}_odb_transaction() is invoked during an active transaction, these operations are essentially treated as no-ops. This can make the interface a bit awkward to use, as calling end_odb_transaction() does not guarantee that a transaction is actually ended. Thus, in situations where a transaction needs to be explicitly flushed, flush_odb_transaction() must be used.
To remove the need for an explicit transaction flush operation via flush_odb_transaction() and better clarify transaction semantics, drop the transaction nesting mechanism in favor of begin_odb_transaction() returning a NULL transaction value to signal it was a no-op, and end_odb_transaction() behaving as a no-op when a NULL transaction value is passed. This is safe for existing callers as the transaction value wired to end_odb_transaction() already comes from begin_odb_transaction() and thus continues the same no-op behavior when a transaction is already pending. With this model, passing a pending transaction to end_odb_transaction() ensures it is committed at that point in time.
Signed-off-by: Justin Tobler <jltobler@gmail.com> --- bulk-checkin.c | 22 ++++++++++------------ bulk-checkin.h | 8 +++----- object-file.c | 2 +- 3 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c index 124c493067..eb6ef704c3 100644 --- a/bulk-checkin.c +++ b/bulk-checkin.c @@ -33,7 +33,6 @@ struct bulk_checkin_packfile { struct odb_transaction { struct object_database *odb; - int nesting; struct tmp_objdir *objdir; struct bulk_checkin_packfile packfile; }; @@ -368,12 +367,11 @@ void fsync_loose_object_bulk_checkin(struct odb_transaction *transaction, struct odb_transaction *begin_odb_transaction(struct object_database *odb) { - if (!odb->transaction) { - CALLOC_ARRAY(odb->transaction, 1); - odb->transaction->odb = odb; - } + if (odb->transaction) + return NULL; - odb->transaction->nesting += 1; + CALLOC_ARRAY(odb->transaction, 1); + odb->transaction->odb = odb; return odb->transaction; } @@ -389,14 +387,14 @@ 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) + if (!transaction) return; + /* + * Ensure the transaction ending matches the pending transaction. + */ + ASSERT(transaction == transaction->odb->transaction); + flush_odb_transaction(transaction); transaction->odb->transaction = NULL; free(transaction); diff --git a/bulk-checkin.h b/bulk-checkin.h index ac8887f476..51d0ac6134 100644 --- a/bulk-checkin.h +++ b/bulk-checkin.h @@ -38,9 +38,8 @@ int index_blob_bulk_checkin(struct odb_transaction *transaction, /* * Tell the object database to optimize for adding * multiple objects. end_odb_transaction must be called - * to make new objects visible. Transactions can be nested, - * and objects are only visible after the outermost transaction - * is complete or the transaction is flushed. + * to make new objects visible. If a transaction is already + * pending, NULL is returned. */ struct odb_transaction *begin_odb_transaction(struct object_database *odb); @@ -53,8 +52,7 @@ void flush_odb_transaction(struct odb_transaction *transaction); /* * Tell the object database to make any objects from the - * current transaction visible if this is the final nested - * transaction. + * current transaction visible. */ void end_odb_transaction(struct odb_transaction *transaction); diff --git a/object-file.c b/object-file.c index bc15af4245..5e76573549 100644 --- a/object-file.c +++ b/object-file.c @@ -1267,7 +1267,7 @@ int index_fd(struct index_state *istate, struct object_id *oid, struct odb_transaction *transaction; transaction = begin_odb_transaction(the_repository->objects); - ret = index_blob_bulk_checkin(transaction, + ret = index_blob_bulk_checkin(the_repository->objects->transaction, oid, fd, xsize_t(st->st_size), path, flags); end_odb_transaction(transaction);
-- 2.51.0.193.g4975ec3473b