From: Justin Tobler <jltobler@gmail•com>
To: git@vger•kernel.org
Cc: Justin Tobler <jltobler@gmail•com>
Subject: [PATCH 1/6] bulk-checkin: remove ODB transaction nesting
Date: Tue, 9 Sep 2025 14:11:29 -0500 [thread overview]
Message-ID: <20250909191134.555689-2-jltobler@gmail.com> (raw)
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
next prev parent reply other threads:[~2025-09-09 19:11 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-09 19:11 [PATCH 0/6] odb: add transaction interfaces to ODB subsystem Justin Tobler
2025-09-09 19:11 ` Justin Tobler [this message]
2025-09-11 6:40 ` [PATCH 1/6] bulk-checkin: remove ODB transaction nesting Patrick Steinhardt
2025-09-11 15:17 ` Justin Tobler
2025-09-15 23:36 ` Taylor Blau
2025-09-16 2:55 ` Justin Tobler
2025-09-16 16:44 ` Junio C Hamano
2025-09-16 17:47 ` Justin Tobler
2025-09-09 19:11 ` [PATCH 2/6] builtin/update-index: end ODB transaction when --verbose is specified Justin Tobler
2025-09-11 6:40 ` Patrick Steinhardt
2025-09-11 15:34 ` Justin Tobler
2025-09-15 6:08 ` Patrick Steinhardt
2025-09-15 17:08 ` Justin Tobler
2025-09-15 22:03 ` Justin Tobler
2025-09-09 19:11 ` [PATCH 3/6] bulk-checkin: drop flush_odb_transaction() Justin Tobler
2025-09-09 19:11 ` [PATCH 4/6] object-file: relocate ODB transaction code Justin Tobler
2025-09-09 19:11 ` [PATCH 5/6] object-file: update naming from bulk-checkin Justin Tobler
2025-09-09 19:11 ` [PATCH 6/6] odb: add transaction interface Justin Tobler
2025-09-11 6:40 ` Patrick Steinhardt
2025-09-11 16:31 ` Justin Tobler
2025-09-15 20:29 ` [PATCH v2 0/6] odb: add transaction interfaces to ODB subsystem Justin Tobler
2025-09-15 20:29 ` [PATCH v2 1/6] bulk-checkin: remove ODB transaction nesting Justin Tobler
2025-09-16 7:57 ` Karthik Nayak
2025-09-16 15:00 ` Justin Tobler
2025-09-15 20:29 ` [PATCH v2 2/6] builtin/update-index: end ODB transaction when --verbose is specified Justin Tobler
2025-09-16 9:07 ` Karthik Nayak
2025-09-16 15:17 ` Justin Tobler
2025-09-15 20:29 ` [PATCH v2 3/6] bulk-checkin: drop flush_odb_transaction() Justin Tobler
2025-09-15 20:29 ` [PATCH v2 4/6] object-file: relocate ODB transaction code Justin Tobler
2025-09-15 20:29 ` [PATCH v2 5/6] object-file: update naming from bulk-checkin Justin Tobler
2025-09-15 20:29 ` [PATCH v2 6/6] odb: add transaction interface Justin Tobler
2025-09-16 18:29 ` [PATCH v3 0/6] odb: add transaction interfaces to ODB subsystem Justin Tobler
2025-09-16 18:29 ` [PATCH v3 1/6] bulk-checkin: remove ODB transaction nesting Justin Tobler
2025-09-16 18:29 ` [PATCH v3 2/6] builtin/update-index: end ODB transaction when --verbose is specified Justin Tobler
2025-09-16 18:29 ` [PATCH v3 3/6] bulk-checkin: drop flush_odb_transaction() Justin Tobler
2025-09-16 18:29 ` [PATCH v3 4/6] object-file: relocate ODB transaction code Justin Tobler
2025-09-16 18:29 ` [PATCH v3 5/6] object-file: update naming from bulk-checkin Justin Tobler
2025-09-16 18:29 ` [PATCH v3 6/6] odb: add transaction interface Justin Tobler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250909191134.555689-2-jltobler@gmail.com \
--to=jltobler@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox