git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Justin Tobler <jltobler@gmail.com>
Date
Sep 9, 2025, 19:11 UTC
Message-ID
<20250909191134.555689-2-jltobler@gmail.com>
In-Reply-To
<20250909191134.555689-1-jltobler@gmail.com>

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.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 bulk-checkin.c | 18 ++++--------------
 bulk-checkin.h |  9 ++++-----
 cache-tree.c   | 12 +++++++++---
 object-file.c  | 12 ++++++++----
 read-cache.c   | 10 +++++++---
 5 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 124c4930676..0da5783090d 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)
+		BUG("ODB transaction already started");
 
-	odb->transaction->nesting += 1;
+	CALLOC_ARRAY(odb->transaction, 1);
+	odb->transaction->odb = odb;
 
 	return odb->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);
diff --git a/bulk-checkin.h b/bulk-checkin.h
index ac8887f476b..b4536d81fc2 100644
--- a/bulk-checkin.h
+++ b/bulk-checkin.h
@@ -38,9 +38,9 @@ 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. Only a single transaction
+ * can be pending at a time and must be ended before
+ * beginning another.
  */
 struct odb_transaction *begin_odb_transaction(struct object_database *odb);
 
@@ -53,8 +53,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/cache-tree.c b/cache-tree.c
index d225554eedd..5041639f99f 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -474,7 +474,7 @@ static int update_one(struct cache_tree *it,
 
 int cache_tree_update(struct index_state *istate, int flags)
 {
-	struct odb_transaction *transaction;
+	struct odb_transaction *transaction = NULL;
 	int skip, i;
 
 	i = verify_cache(istate, flags);
@@ -490,10 +490,16 @@ int cache_tree_update(struct index_state *istate, int flags)
 
 	trace_performance_enter();
 	trace2_region_enter("cache_tree", "update", the_repository);
-	transaction = begin_odb_transaction(the_repository->objects);
+
+	if (!the_repository->objects->transaction)
+		transaction = begin_odb_transaction(the_repository->objects);
+
 	i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
 		       "", 0, &skip, flags);
-	end_odb_transaction(transaction);
+
+	if (transaction)
+		end_odb_transaction(transaction);
+
 	trace2_region_leave("cache_tree", "update", the_repository);
 	trace_performance_leave("cache_tree_update");
 	if (i < 0)
diff --git a/object-file.c b/object-file.c
index bc15af42450..45f17a53a98 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1264,13 +1264,17 @@ int index_fd(struct index_state *istate, struct object_id *oid,
 		ret = index_core(istate, oid, fd, xsize_t(st->st_size),
 				 type, path, flags);
 	} else {
-		struct odb_transaction *transaction;
+		struct odb_transaction *transaction = NULL;
 
-		transaction = begin_odb_transaction(the_repository->objects);
-		ret = index_blob_bulk_checkin(transaction,
+		if (!the_repository->objects->transaction)
+			transaction = begin_odb_transaction(the_repository->objects);
+
+		ret = index_blob_bulk_checkin(the_repository->objects->transaction,
 					      oid, fd, xsize_t(st->st_size),
 					      path, flags);
-		end_odb_transaction(transaction);
+
+		if (transaction)
+			end_odb_transaction(transaction);
 	}
 
 	close(fd);
diff --git a/read-cache.c b/read-cache.c
index 229b8ef11c9..7e5501f0839 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3947,7 +3947,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, char *ps_matched,
 		       int include_sparse, int flags)
 {
-	struct odb_transaction *transaction;
+	struct odb_transaction *transaction = NULL;
 	struct update_callback_data data;
 	struct rev_info rev;
 
@@ -3973,9 +3973,13 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 	 * This function is invoked from commands other than 'add', which
 	 * may not have their own transaction active.
 	 */
-	transaction = begin_odb_transaction(repo->objects);
+	if (!repo->objects->transaction)
+		transaction = begin_odb_transaction(repo->objects);
+
 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
-	end_odb_transaction(transaction);
+
+	if (transaction)
+		end_odb_transaction(transaction);
 
 	release_revisions(&rev);
 	return !!data.add_errors;

-- 
2.51.0.193.g4975ec3473b
Previous: Justin ToblerNext: Patrick Steinhardt
Message 2 of 38 in “odb: add transaction interfaces to ODB subsystem”
  1. 0/6 odb: add transaction interfaces to ODB subsystemJustin Tobler, Sep 9, 2025
  2. 1/6 bulk-checkin: remove ODB transaction nestingJustin Tobler, Sep 9, 2025
  3. Patrick SteinhardtSep 11, 2025
  4. Justin ToblerSep 11, 2025
  5. Taylor BlauSep 15, 2025
  6. Justin ToblerSep 16, 2025
  7. Junio C HamanoSep 16, 2025
  8. Justin ToblerSep 16, 2025
  9. 2/6 builtin/update-index: end ODB transaction when --verbose is specifiedJustin Tobler, Sep 9, 2025
  10. Patrick SteinhardtSep 11, 2025
  11. Justin ToblerSep 11, 2025
  12. Patrick SteinhardtSep 15, 2025
  13. Justin ToblerSep 15, 2025
  14. Justin ToblerSep 15, 2025
  15. 3/6 bulk-checkin: drop flush_odb_transaction()Justin Tobler, Sep 9, 2025
  16. 4/6 object-file: relocate ODB transaction codeJustin Tobler, Sep 9, 2025
  17. 5/6 object-file: update naming from bulk-checkinJustin Tobler, Sep 9, 2025
  18. 6/6 odb: add transaction interfaceJustin Tobler, Sep 9, 2025
  19. Patrick SteinhardtSep 11, 2025
  20. Justin ToblerSep 11, 2025
  21. 0/6 odb: add transaction interfaces to ODB subsystemJustin Tobler, Sep 15, 2025
  22. 1/6 bulk-checkin: remove ODB transaction nestingJustin Tobler, Sep 15, 2025
  23. Karthik NayakSep 16, 2025
  24. Justin ToblerSep 16, 2025
  25. 2/6 builtin/update-index: end ODB transaction when --verbose is specifiedJustin Tobler, Sep 15, 2025
  26. Karthik NayakSep 16, 2025
  27. Justin ToblerSep 16, 2025
  28. 3/6 bulk-checkin: drop flush_odb_transaction()Justin Tobler, Sep 15, 2025
  29. 4/6 object-file: relocate ODB transaction codeJustin Tobler, Sep 15, 2025
  30. 5/6 object-file: update naming from bulk-checkinJustin Tobler, Sep 15, 2025
  31. 6/6 odb: add transaction interfaceJustin Tobler, Sep 15, 2025
  32. 0/6 odb: add transaction interfaces to ODB subsystemJustin Tobler, Sep 16, 2025
  33. 2/6 builtin/update-index: end ODB transaction when --verbose is specifiedJustin Tobler, Sep 16, 2025
  34. 1/6 bulk-checkin: remove ODB transaction nestingJustin Tobler, Sep 16, 2025
  35. 3/6 bulk-checkin: drop flush_odb_transaction()Justin Tobler, Sep 16, 2025
  36. 4/6 object-file: relocate ODB transaction codeJustin Tobler, Sep 16, 2025
  37. 5/6 object-file: update naming from bulk-checkinJustin Tobler, Sep 16, 2025
  38. 6/6 odb: add transaction interfaceJustin Tobler, Sep 16, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.