threads / patch / 64882

patch, 4 partsodb: support ODB source specific transaction handling

Subject: [PATCH 0/4] odb: support ODB source specific transaction handling

## tl;dr

32 messages between Jan 28, 2026 and Feb 5, 2026. Diffs are folded; open one to read it.

replies: 31people: 5as markdown or json

Justin Tobler· Jan 28, 2026, 23:45 UTC · lore
Greetings,

ODB transactions provide a mechanism for writing new objects in the repository that are in a pending state until they are committed. With pluggable ODBs, each source will likely need to handle transactions differently. This patch series prepares ODB transaction handling to be defined for each ODB source by creating a generic odb_transaction structure that stores a callback for committing the transaction.

For now, `struct odb_transaction_loose` is the only transaction implementation and what is always returned when creating a transaction. Additionally, this series lifts transaction logic that is common to all backends out of the implementation layer into `odb_transaction_{begin,commit}()` layer.

Thanks, -Justin

Justin Tobler (4):
  odb: store ODB source in `struct odb_transaction`
  object-file: rename transaction functions
  odb: prepare `struct odb_transaction` to support more sources
  odb: transparently handle common transaction behavior
 object-file.c | 77 +++++++++++++++++++++++++--------------------------
 object-file.h | 10 ++-----
 odb.c         | 22 +++++++++++++--
 odb.h         | 17 ++++++++++++
 4 files changed, 77 insertions(+), 49 deletions(-)
base-commit: ea717645d199f6f1b66058886475db3e8c9330e9
-- 
2.52.0.373.g68cb7f9e92
Justin Tobler· Jan 28, 2026, 23:45 UTC · re: Justin Tobler · lore

[PATCH 1/4] odb: store ODB source in `struct odb_transaction`

Each `struct odb_transaction` currently stores a reference to the `struct object_database`. Since transactions are handled per object source, instead store a reference to the source.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)
Show changes to object-file.c +11 −11
diff --git a/object-file.c b/object-file.c
index e7e4c3348f..196509b252 100644
--- a/object-file.c
+++ b/object-file.c
@@ -711,7 +711,7 @@ struct transaction_packfile {
 };
 
 struct odb_transaction {
-	struct object_database *odb;
+	struct odb_source *source;
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
@@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	if (!transaction || transaction->objdir)
 		return;
 
-	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
+	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
 	if (transaction->objdir)
 		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
 }
@@ -772,7 +772,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)
 	 * the final name is visible.
 	 */
 	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
-		    repo_get_object_directory(transaction->odb->repo));
+		    repo_get_object_directory(transaction->source->odb->repo));
 	temp = xmks_tempfile(temp_path.buf);
 	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
 	delete_tempfile(&temp);
@@ -1344,7 +1344,7 @@ static int already_written(struct odb_transaction *transaction,
 			   struct object_id *oid)
 {
 	/* The object may already exist in the repository */
-	if (odb_has_object(transaction->odb, oid,
+	if (odb_has_object(transaction->source->odb, oid,
 			   HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
 		return 1;
 
@@ -1365,7 +1365,7 @@ static void prepare_packfile_transaction(struct odb_transaction *transaction,
 	if (!(flags & INDEX_WRITE_OBJECT) || state->f)
 		return;
 
-	state->f = create_tmp_packfile(transaction->odb->repo,
+	state->f = create_tmp_packfile(transaction->source->odb->repo,
 				       &state->pack_tmp_name);
 	reset_pack_idx_option(&state->pack_idx_opts);
 
@@ -1469,7 +1469,7 @@ static int stream_blob_to_pack(struct transaction_packfile *state,
 static void flush_packfile_transaction(struct odb_transaction *transaction)
 {
 	struct transaction_packfile *state = &transaction->packfile;
-	struct repository *repo = transaction->odb->repo;
+	struct repository *repo = transaction->source->odb->repo;
 	unsigned char hash[GIT_MAX_RAWSZ];
 	struct strbuf packname = STRBUF_INIT;
 	char *idx_tmp_name = NULL;
@@ -1494,7 +1494,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
 	}
 
 	strbuf_addf(&packname, "%s/pack/pack-%s.",
-		    repo_get_object_directory(transaction->odb->repo),
+		    repo_get_object_directory(transaction->source->odb->repo),
 		    hash_to_hex_algop(hash, repo->hash_algo));
 
 	stage_tmp_packfiles(repo, &packname, state->pack_tmp_name,
@@ -1553,7 +1553,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
 
 	header_len = format_object_header((char *)obuf, sizeof(obuf),
 					  OBJ_BLOB, size);
-	transaction->odb->repo->hash_algo->init_fn(&ctx);
+	transaction->source->odb->repo->hash_algo->init_fn(&ctx);
 	git_hash_update(&ctx, obuf, header_len);
 
 	/* Note: idx is non-NULL when we are writing */
@@ -1993,7 +1993,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
 		return NULL;
 
 	CALLOC_ARRAY(odb->transaction, 1);
-	odb->transaction->odb = odb;
+	odb->transaction->source = source;
 
 	return odb->transaction;
 }
@@ -2006,11 +2006,11 @@ void object_file_transaction_commit(struct odb_transaction *transaction)
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
-	ASSERT(transaction == transaction->odb->transaction);
+	ASSERT(transaction == transaction->source->odb->transaction);
 
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	transaction->odb->transaction = NULL;
+	transaction->source->odb->transaction = NULL;
 	free(transaction);
 }
 
-- 
2.52.0.373.g68cb7f9e92
Patrick Steinhardt· Jan 29, 2026, 11:24 UTC · re: Justin Tobler · lore

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:
> Each `struct odb_transaction` currently stores a reference to the
> `struct object_database`. Since transactions are handled per object
> source, instead store a reference to the source.
Makes sense.
Show 13 quoted lines
> diff --git a/object-file.c b/object-file.c
> index e7e4c3348f..196509b252 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
>  	if (!transaction || transaction->objdir)
>  		return;
>  
> -	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
> +	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
>  	if (transaction->objdir)
>  		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
>  }

This makes me wonder whether we should first refactor the `tmp_objdir` subsystem to receive a source instead of a repository as input. Otherwise we "pretend" that the transaction is on the source level, but we ultimately still end up creating the temporary directory in the repository's object directory unconditionally.

It wouldn't really change anything right now as we only ever write objects via the primary object source anyway, so the end result would be the same. But it just feels like a good first step to me to fix this conceptual inconsistency, and it shouldn't be too involved either as `tmp_objdir_create()` only has three callsites.

Patrick
Junio C Hamano· Jan 29, 2026, 19:25 UTC · re: Patrick Steinhardt · lore

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

Patrick Steinhardt <ps@pks.im> writes:
Show 32 quoted lines
> On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:
>> Each `struct odb_transaction` currently stores a reference to the
>> `struct object_database`. Since transactions are handled per object
>> source, instead store a reference to the source.
>
> Makes sense.
>
>> diff --git a/object-file.c b/object-file.c
>> index e7e4c3348f..196509b252 100644
>> --- a/object-file.c
>> +++ b/object-file.c
>> @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
>>  	if (!transaction || transaction->objdir)
>>  		return;
>>  
>> -	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
>> +	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
>>  	if (transaction->objdir)
>>  		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
>>  }
>
> This makes me wonder whether we should first refactor the `tmp_objdir`
> subsystem to receive a source instead of a repository as input.
> Otherwise we "pretend" that the transaction is on the source level, but
> we ultimately still end up creating the temporary directory in the
> repository's object directory unconditionally.
>
> It wouldn't really change anything right now as we only ever write
> objects via the primary object source anyway, so the end result would be
> the same. But it just feels like a good first step to me to fix this
> conceptual inconsistency, and it shouldn't be too involved either as
> `tmp_objdir_create()` only has three callsites.

I agree with your "not really change anything right now" comment, but a new odb source that will be invented in the future may not even be file based, and a generic-sounding tmp_objdir_create() that creates a temporary directory on the filesystem may not even be an appropriate abstraction.

If we have two or more odb sources both are filesystem based, on the other hand, I do not think it is particulary bad if these two odb sources belonging to the same repository took a temporary directory out of that repository. As long as one temporary object directory taken by one odb source is not used to commit the transaction into the other odb source, it would be fine, no?

Thanks.
Justin Tobler· Jan 29, 2026, 20:12 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

On 26/01/29 11:25AM, Junio C Hamano wrote:
Show 18 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> > This makes me wonder whether we should first refactor the `tmp_objdir`
> > subsystem to receive a source instead of a repository as input.
> > Otherwise we "pretend" that the transaction is on the source level, but
> > we ultimately still end up creating the temporary directory in the
> > repository's object directory unconditionally.
> >
> > It wouldn't really change anything right now as we only ever write
> > objects via the primary object source anyway, so the end result would be
> > the same. But it just feels like a good first step to me to fix this
> > conceptual inconsistency, and it shouldn't be too involved either as
> > `tmp_objdir_create()` only has three callsites.
> 
> I agree with your "not really change anything right now" comment,
> but a new odb source that will be invented in the future may not
> even be file based, and a generic-sounding tmp_objdir_create() that
> creates a temporary directory on the filesystem may not even be an
> appropriate abstraction.

Yup, the tmp-objdir system is really only useful in the context of file based ODB sources. I figure future ODB sources will have to have a different mechanism to temporarly store objects.

Interestingly, it looks like there are only three users of odb-tmpdir: remerge-diffs, git-recieve-pack, and ODB transactions. All of these use-cases seems like a reasonble fit to create an ODB transaction instead of managing the tmpdir directly. In the case of remerge-diffs the transaction would need to always be aborted. If this is done, then a tmpdir could become an internal detail of the ODB transaction for the files backend.

Show 6 quoted lines
> If we have two or more odb sources both are filesystem based, on the
> other hand, I do not think it is particulary bad if these two odb
> sources belonging to the same repository took a temporary directory
> out of that repository.  As long as one temporary object directory
> taken by one odb source is not used to commit the transaction into
> the other odb source, it would be fine, no?

I believe that we only support having a single tmpdir per Git process via `the_tmp_objdir` global anyway. So maybe on second thought configuring per source doesn't make much sense especially if only the "files" ODB source would make use of it. If we do go down the route of merging the tmpdir with the "files" ODB transaction implementation, we could probably just pass the concrete "files" ODB source to it instead.

-Justin
Junio C Hamano· Jan 29, 2026, 20:28 UTC · re: Justin Tobler · lore

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

Justin Tobler <jltobler@gmail.com> writes:
Show 7 quoted lines
> Interestingly, it looks like there are only three users of odb-tmpdir:
> remerge-diffs, git-recieve-pack, and ODB transactions. All of these
> use-cases seems like a reasonble fit to create an ODB transaction
> instead of managing the tmpdir directly. In the case of remerge-diffs
> the transaction would need to always be aborted. If this is done, then a
> tmpdir could become an internal detail of the ODB transaction for the
> files backend.
;-)  I agree 100%.

"Prepare to create objects that may be undone in the end", "Now make these objects we created so far as parmanent part of the object store", "Reject those objects we created so far as the transaction created them is being aborted" are requests at the right abstraction level. "Give me a temporary object directory" is not.

Justin Tobler· Jan 29, 2026, 21:54 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

On 26/01/29 12:28PM, Junio C Hamano wrote:
Show 17 quoted lines
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > Interestingly, it looks like there are only three users of odb-tmpdir:
> > remerge-diffs, git-recieve-pack, and ODB transactions. All of these
> > use-cases seems like a reasonble fit to create an ODB transaction
> > instead of managing the tmpdir directly. In the case of remerge-diffs
> > the transaction would need to always be aborted. If this is done, then a
> > tmpdir could become an internal detail of the ODB transaction for the
> > files backend.
> 
> ;-)  I agree 100%.
> 
> "Prepare to create objects that may be undone in the end", "Now make
> these objects we created so far as parmanent part of the object
> store", "Reject those objects we created so far as the transaction
> created them is being aborted" are requests at the right abstraction
> level.  "Give me a temporary object directory" is not.

Ok, I'll go ahead and leave the tmp-objdir stuff alone in this patch series and look into extending the ODB transaction usage to replace existing tmpdir callsites in a followup series.

Thanks, -Justin

Justin Tobler· Jan 29, 2026, 19:30 UTC · re: Patrick Steinhardt · lore

Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`

On 26/01/29 12:24PM, Patrick Steinhardt wrote:
Show 26 quoted lines
> On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:
> > diff --git a/object-file.c b/object-file.c
> > index e7e4c3348f..196509b252 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
> >  	if (!transaction || transaction->objdir)
> >  		return;
> >  
> > -	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
> > +	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
> >  	if (transaction->objdir)
> >  		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
> >  }
> 
> This makes me wonder whether we should first refactor the `tmp_objdir`
> subsystem to receive a source instead of a repository as input.
> Otherwise we "pretend" that the transaction is on the source level, but
> we ultimately still end up creating the temporary directory in the
> repository's object directory unconditionally.
> 
> It wouldn't really change anything right now as we only ever write
> objects via the primary object source anyway, so the end result would be
> the same. But it just feels like a good first step to me to fix this
> conceptual inconsistency, and it shouldn't be too involved either as
> `tmp_objdir_create()` only has three callsites.

Ya this seems reasonable and I agree that it makes sense to do as a first step. In the next version I'll make this change in a prepatory commit.

-Justin
Justin Tobler· Jan 28, 2026, 23:45 UTC · re: Justin Tobler · lore

[PATCH 2/4] object-file: rename transaction functions

In a subsequent commit, ODB transactions are made more generic to facilitate each ODB source providing its own transaction handling. Rename `object_file_transaction_{begin,commit}()` to `odb_transaction_loose_{begin,commit}()` to better match the future source specific transaction implementation.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 6 +++---
 object-file.h | 6 +++---
 odb.c         | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)
Show changes to 3 files +8 −8

object-file.c, object-file.h, odb.c

diff --git a/object-file.c b/object-file.c
index 196509b252..790be25f08 100644
--- a/object-file.c
+++ b/object-file.c
@@ -723,7 +723,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	 * We lazily create the temporary object directory
 	 * the first time an object might be added, since
 	 * callers may not know whether any objects will be
-	 * added at the time they call object_file_transaction_begin.
+	 * added at the time they call odb_transaction_loose_begin.
 	 */
 	if (!transaction || transaction->objdir)
 		return;
@@ -1985,7 +1985,7 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
 {
 	struct object_database *odb = source->odb;
 
@@ -1998,7 +1998,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
 	return odb->transaction;
 }
 
-void object_file_transaction_commit(struct odb_transaction *transaction)
+void odb_transaction_loose_commit(struct odb_transaction *transaction)
 {
 	if (!transaction)
 		return;
diff --git a/object-file.h b/object-file.h
index 1229d5f675..03f0474656 100644
--- a/object-file.h
+++ b/object-file.h
@@ -202,16 +202,16 @@ struct odb_transaction;
 
 /*
  * Tell the object database to optimize for adding
- * multiple objects. object_file_transaction_commit must be called
+ * multiple objects. odb_transaction_loose_commit must be called
  * to make new objects visible. If a transaction is already
  * pending, NULL is returned.
  */
-struct odb_transaction *object_file_transaction_begin(struct odb_source *source);
+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);
 
 /*
  * Tell the object database to make any objects from the
  * current transaction visible.
  */
-void object_file_transaction_commit(struct odb_transaction *transaction);
+void odb_transaction_loose_commit(struct odb_transaction *transaction);
 
 #endif /* OBJECT_FILE_H */
diff --git a/odb.c b/odb.c
index ac70b6a099..90dcbca821 100644
--- a/odb.c
+++ b/odb.c
@@ -1153,10 +1153,10 @@ void odb_reprepare(struct object_database *o)
 
 struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 {
-	return object_file_transaction_begin(odb->sources);
+	return odb_transaction_loose_begin(odb->sources);
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)
 {
-	object_file_transaction_commit(transaction);
+	odb_transaction_loose_commit(transaction);
 }
-- 
2.52.0.373.g68cb7f9e92
Justin Tobler· Jan 28, 2026, 23:45 UTC · re: Justin Tobler · lore

[PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources

Each ODB transaction should be specific to the ODB source it pertains to. Update `struct odb_transaction` to store a commit callback specific to its object source type. For now `struct odb_transaction_loose` is the only transaction type and what is always returned when starting a transaction.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 80 ++++++++++++++++++++++++++++-----------------------
 object-file.h |  6 ----
 odb.c         |  5 +++-
 odb.h         | 17 +++++++++++
 4 files changed, 65 insertions(+), 43 deletions(-)
Show changes to 4 files +65 −43

object-file.c, object-file.h, odb.c, odb.h

diff --git a/object-file.c b/object-file.c
index 790be25f08..e4739fc0cc 100644
--- a/object-file.c
+++ b/object-file.c
@@ -710,15 +710,17 @@ struct transaction_packfile {
 	uint32_t nr_written;
 };
 
-struct odb_transaction {
-	struct odb_source *source;
+struct odb_transaction_loose {
+	struct odb_transaction base;
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
 };
 
-static void prepare_loose_object_transaction(struct odb_transaction *transaction)
+static void prepare_loose_object_transaction(struct odb_transaction *base)
 {
+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
+
 	/*
 	 * We lazily create the temporary object directory
 	 * the first time an object might be added, since
@@ -728,14 +730,16 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	if (!transaction || transaction->objdir)
 		return;
 
-	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
+	transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
 	if (transaction->objdir)
 		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
 }
 
-static void fsync_loose_object_transaction(struct odb_transaction *transaction,
+static void fsync_loose_object_transaction(struct odb_transaction *base,
 					   int fd, const char *filename)
 {
+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
+
 	/*
 	 * If we have an active ODB transaction, we issue a call that
 	 * cleans the filesystem page cache but avoids a hardware flush
@@ -754,7 +758,7 @@ static void fsync_loose_object_transaction(struct odb_transaction *transaction,
 /*
  * Cleanup after batch-mode fsync_object_files.
  */
-static void flush_loose_object_transaction(struct odb_transaction *transaction)
+static void flush_loose_object_transaction(struct odb_transaction_loose *transaction)
 {
 	struct strbuf temp_path = STRBUF_INIT;
 	struct tempfile *temp;
@@ -772,7 +776,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)
 	 * the final name is visible.
 	 */
 	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
-		    repo_get_object_directory(transaction->source->odb->repo));
+		    repo_get_object_directory(transaction->base.source->odb->repo));
 	temp = xmks_tempfile(temp_path.buf);
 	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
 	delete_tempfile(&temp);
@@ -1340,11 +1344,11 @@ static int index_core(struct index_state *istate,
 	return ret;
 }
 
-static int already_written(struct odb_transaction *transaction,
+static int already_written(struct odb_transaction_loose *transaction,
 			   struct object_id *oid)
 {
 	/* The object may already exist in the repository */
-	if (odb_has_object(transaction->source->odb, oid,
+	if (odb_has_object(transaction->base.source->odb, oid,
 			   HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
 		return 1;
 
@@ -1358,14 +1362,14 @@ static int already_written(struct odb_transaction *transaction,
 }
 
 /* Lazily create backing packfile for the state */
-static void prepare_packfile_transaction(struct odb_transaction *transaction,
+static void prepare_packfile_transaction(struct odb_transaction_loose *transaction,
 					 unsigned flags)
 {
 	struct transaction_packfile *state = &transaction->packfile;
 	if (!(flags & INDEX_WRITE_OBJECT) || state->f)
 		return;
 
-	state->f = create_tmp_packfile(transaction->source->odb->repo,
+	state->f = create_tmp_packfile(transaction->base.source->odb->repo,
 				       &state->pack_tmp_name);
 	reset_pack_idx_option(&state->pack_idx_opts);
 
@@ -1466,10 +1470,10 @@ static int stream_blob_to_pack(struct transaction_packfile *state,
 	return 0;
 }
 
-static void flush_packfile_transaction(struct odb_transaction *transaction)
+static void flush_packfile_transaction(struct odb_transaction_loose *transaction)
 {
 	struct transaction_packfile *state = &transaction->packfile;
-	struct repository *repo = transaction->source->odb->repo;
+	struct repository *repo = transaction->base.source->odb->repo;
 	unsigned char hash[GIT_MAX_RAWSZ];
 	struct strbuf packname = STRBUF_INIT;
 	char *idx_tmp_name = NULL;
@@ -1494,7 +1498,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
 	}
 
 	strbuf_addf(&packname, "%s/pack/pack-%s.",
-		    repo_get_object_directory(transaction->source->odb->repo),
+		    repo_get_object_directory(transaction->base.source->odb->repo),
 		    hash_to_hex_algop(hash, repo->hash_algo));
 
 	stage_tmp_packfiles(repo, &packname, state->pack_tmp_name,
@@ -1534,7 +1538,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
  * binary blobs, they generally do not want to get any conversion, and
  * callers should avoid this code path when filters are requested.
  */
-static int index_blob_packfile_transaction(struct odb_transaction *transaction,
+static int index_blob_packfile_transaction(struct odb_transaction_loose *transaction,
 					   struct object_id *result_oid, int fd,
 					   size_t size, const char *path,
 					   unsigned flags)
@@ -1553,7 +1557,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
 
 	header_len = format_object_header((char *)obuf, sizeof(obuf),
 					  OBJ_BLOB, size);
-	transaction->source->odb->repo->hash_algo->init_fn(&ctx);
+	transaction->base.source->odb->repo->hash_algo->init_fn(&ctx);
 	git_hash_update(&ctx, obuf, header_len);
 
 	/* Note: idx is non-NULL when we are writing */
@@ -1629,10 +1633,11 @@ 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 object_database *odb = the_repository->objects;
 		struct odb_transaction *transaction;
 
-		transaction = odb_transaction_begin(the_repository->objects);
-		ret = index_blob_packfile_transaction(the_repository->objects->transaction,
+		transaction = odb_transaction_begin(odb);
+		ret = index_blob_packfile_transaction((struct odb_transaction_loose *)odb->transaction,
 						      oid, fd,
 						      xsize_t(st->st_size),
 						      path, flags);
@@ -1985,35 +1990,38 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
+static void odb_transaction_loose_commit(struct odb_transaction *base)
 {
-	struct object_database *odb = source->odb;
-
-	if (odb->transaction)
-		return NULL;
-
-	CALLOC_ARRAY(odb->transaction, 1);
-	odb->transaction->source = source;
-
-	return odb->transaction;
-}
-
-void odb_transaction_loose_commit(struct odb_transaction *transaction)
-{
-	if (!transaction)
-		return;
+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
 
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
-	ASSERT(transaction == transaction->source->odb->transaction);
+	ASSERT(base == base->source->odb->transaction);
 
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	transaction->source->odb->transaction = NULL;
+	base->source->odb->transaction = NULL;
 	free(transaction);
 }
 
+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
+{
+	struct odb_transaction_loose *transaction;
+	struct object_database *odb = source->odb;
+
+	if (odb->transaction)
+		return NULL;
+
+	transaction = xcalloc(1, sizeof(*transaction));
+	transaction->base.source = source;
+	transaction->base.commit = odb_transaction_loose_commit;
+
+	odb->transaction = &transaction->base;
+
+	return &transaction->base;
+}
+
 struct odb_source_loose *odb_source_loose_new(struct odb_source *source)
 {
 	struct odb_source_loose *loose;
diff --git a/object-file.h b/object-file.h
index 03f0474656..1bd355b7f4 100644
--- a/object-file.h
+++ b/object-file.h
@@ -208,10 +208,4 @@ struct odb_transaction;
  */
 struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);
 
-/*
- * Tell the object database to make any objects from the
- * current transaction visible.
- */
-void odb_transaction_loose_commit(struct odb_transaction *transaction);
-
 #endif /* OBJECT_FILE_H */
diff --git a/odb.c b/odb.c
index 90dcbca821..5b112f2464 100644
--- a/odb.c
+++ b/odb.c
@@ -1158,5 +1158,8 @@ struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 
 void odb_transaction_commit(struct odb_transaction *transaction)
 {
-	odb_transaction_loose_commit(transaction);
+	if (!transaction)
+		return;
+
+	transaction->commit(transaction);
 }
diff --git a/odb.h b/odb.h
index bab07755f4..83d3a37805 100644
--- a/odb.h
+++ b/odb.h
@@ -77,7 +77,24 @@ struct odb_source {
 struct packed_git;
 struct packfile_store;
 struct cached_object_entry;
+
+/*
+ * A transaction may be started for an object database prior to writing new
+ * objects via odb_transaction_begin(). These objects are not committed until
+ * odb_transaction_commit() is invoked. Only a single transaction may be pending
+ * at a time.
+ *
+ * Each ODB source is expected to implement its own transaction handling.
+ */
 struct odb_transaction;
+typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
+struct odb_transaction {
+	/* The ODB source the transaction is opened against. */
+	struct odb_source *source;
+
+	/* The ODB source specific callback invoked to commit a transaction. */
+	odb_transaction_commit_fn commit;
+};
 
 /*
  * The object database encapsulates access to objects in a repository. It
-- 
2.52.0.373.g68cb7f9e92
Patrick Steinhardt· Jan 29, 2026, 11:24 UTC · re: Justin Tobler · lore

Re: [PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources

On Wed, Jan 28, 2026 at 05:45:18PM -0600, Justin Tobler wrote:

The bit about supporting "more sources" in the subject reads a bit weird to me. We still only handle a single source in a transaction, not multiple ones. I guess what you rather want to say is that we handle "generic" sources? How about:

    odb: prepare `struct odb_source` to become generic
> Each ODB transaction should be specific to the ODB source it pertains
> to.

This is a claim that should probably be backed up a bit. I myself obviously agree with it, but I think it should be noted _why_ we want to have this in the first place.

The patch itself looks as expected to me, as we split up `struct odb_transaction` into two structures:

  - `struct odb_transaction` continues to exist, but is now the generic
    part that simply contains the source and a function pointer.
  - `struct odb_transaction_loose` is the backend-specific
    implementation.

One question though: is this tansaction really specific to loose objects? We also seem to be handling packfiles there in `prepare_packfile_transaction()`, so it rather feels like this is specific to the whole "files" backend. I might be misunderstanding though.

Show 5 quoted lines
> --- a/object-file.h
> +++ b/object-file.h
> @@ -208,10 +208,4 @@ struct odb_transaction;
>   */
>  struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);

Right, we still need this function to be public right now. But eventually it'll be converted into a function pointer part of the `struct odb_source`.

Show 5 quoted lines
> -/*
> - * Tell the object database to make any objects from the
> - * current transaction visible.
> - */
> -void odb_transaction_loose_commit(struct odb_transaction *transaction);

But this doesn't have to be public anymore as we have it available to us via `tranasction->commit()`. Makes sense.

Patrick
Justin Tobler· Jan 29, 2026, 19:41 UTC · re: Patrick Steinhardt · lore

Re: [PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources

On 26/01/29 12:24PM, Patrick Steinhardt wrote:
Show 8 quoted lines
> On Wed, Jan 28, 2026 at 05:45:18PM -0600, Justin Tobler wrote:
> 
> The bit about supporting "more sources" in the subject reads a bit weird
> to me. We still only handle a single source in a transaction, not
> multiple ones. I guess what you rather want to say is that we handle
> "generic" sources? How about:
> 
>     odb: prepare `struct odb_source` to become generic

The intent was that with generic transactions we it becomes possible to support additional ODB sources each with their own transaction implementation, but I agree the current wording is not great. I'll update in the next version.

Show 6 quoted lines
> > Each ODB transaction should be specific to the ODB source it pertains
> > to.
> 
> This is a claim that should probably be backed up a bit. I myself
> obviously agree with it, but I think it should be noted _why_ we want to
> have this in the first place.

Ya that's fair. I'll update the commit message in the next version to properly explain the intent here.

Show 14 quoted lines
> The patch itself looks as expected to me, as we split up `struct
> odb_transaction` into two structures:
> 
>   - `struct odb_transaction` continues to exist, but is now the generic
>     part that simply contains the source and a function pointer.
> 
>   - `struct odb_transaction_loose` is the backend-specific
>     implementation.
> 
> One question though: is this tansaction really specific to loose
> objects? We also seem to be handling packfiles there in
> `prepare_packfile_transaction()`, so it rather feels like this is
> specific to the whole "files" backend. I might be misunderstanding
> though.

The current transaction backend is primarily used to facilitate bulk writing of what would otherwise be loose objects into a packfile. I would like to eventually expand the use of ODB transactions though to cover areas like git-recieve-pack(1) which currently uses tmp-objdir directly. So it probably makes sense to call this odb_transaction_files. Will update in the next version.

-Justin
Justin Tobler· Jan 28, 2026, 23:45 UTC · re: Justin Tobler · lore

[PATCH 4/4] odb: transparently handle common transaction behavior

A new ODB transaction is created and returned via `odb_transaction_begin()` and stored in the ODB. Only a single transaction may be pending at a time. If the ODB already has a transaction, the function is expected to return NULL. Similarly, when committing a transaction via `odb_transaction_commit()` the transaction being committed must match the pending transaction and upon commit reset the ODB transaction to NULL.

These behaviors apply regardless of the ODB transaction implementation. Move the corresponding logic into `odb_transaction_{begin,commit}()` accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c |  9 ---------
 odb.c         | 17 ++++++++++++++++-
 2 files changed, 16 insertions(+), 10 deletions(-)
Show changes to 2 files +16 −10

object-file.c, odb.c

diff --git a/object-file.c b/object-file.c
index e4739fc0cc..0def5efa91 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1994,15 +1994,8 @@ static void odb_transaction_loose_commit(struct odb_transaction *base)
 {
 	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
 
-	/*
-	 * Ensure the transaction ending matches the pending transaction.
-	 */
-	ASSERT(base == base->source->odb->transaction);
-
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	base->source->odb->transaction = NULL;
-	free(transaction);
 }
 
 struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
@@ -2017,8 +2010,6 @@ struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
 	transaction->base.source = source;
 	transaction->base.commit = odb_transaction_loose_commit;
 
-	odb->transaction = &transaction->base;
-
 	return &transaction->base;
 }
 
diff --git a/odb.c b/odb.c
index 5b112f2464..332860735e 100644
--- a/odb.c
+++ b/odb.c
@@ -1153,7 +1153,15 @@ void odb_reprepare(struct object_database *o)
 
 struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 {
-	return odb_transaction_loose_begin(odb->sources);
+	struct odb_transaction *transaction;
+
+	if (odb->transaction)
+		return NULL;
+
+	transaction = odb_transaction_loose_begin(odb->sources);
+	odb->transaction = transaction;
+
+	return transaction;
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)
@@ -1161,5 +1169,12 @@ void odb_transaction_commit(struct odb_transaction *transaction)
 	if (!transaction)
 		return;
 
+	/*
+	 * Ensure the transaction ending matches the pending transaction.
+	 */
+	ASSERT(transaction == transaction->source->odb->transaction);
+
 	transaction->commit(transaction);
+	transaction->source->odb->transaction = NULL;
+	free(transaction);
 }
-- 
2.52.0.373.g68cb7f9e92
Patrick Steinhardt· Jan 29, 2026, 11:24 UTC · re: Justin Tobler · lore

Re: [PATCH 4/4] odb: transparently handle common transaction behavior

On Wed, Jan 28, 2026 at 05:45:19PM -0600, Justin Tobler wrote:
Show 11 quoted lines
> A new ODB transaction is created and returned via
> `odb_transaction_begin()` and stored in the ODB. Only a single
> transaction may be pending at a time. If the ODB already has a
> transaction, the function is expected to return NULL. Similarly, when
> committing a transaction via `odb_transaction_commit()` the transaction
> being committed must match the pending transaction and upon commit reset
> the ODB transaction to NULL.
> 
> These behaviors apply regardless of the ODB transaction implementation.
> Move the corresponding logic into `odb_transaction_{begin,commit}()`
> accordingly.

Besides deduplicating the logic, it also makes the ODB source only care about itself, whereas the ODB is responsible for managing the transaction's lifecycle. Which I think is a good thing.

Show 21 quoted lines
> diff --git a/odb.c b/odb.c
> index 5b112f2464..332860735e 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -1153,7 +1153,15 @@ void odb_reprepare(struct object_database *o)
>  
>  struct odb_transaction *odb_transaction_begin(struct object_database *odb)
>  {
> -	return odb_transaction_loose_begin(odb->sources);
> +	struct odb_transaction *transaction;
> +
> +	if (odb->transaction)
> +		return NULL;
> +
> +	transaction = odb_transaction_loose_begin(odb->sources);
> +	odb->transaction = transaction;
> +
> +	return transaction;
>  }
>  
>  void odb_transaction_commit(struct odb_transaction *transaction)
Nit: the `transaction` variable seems somewhat pointless to me.
Patrick
Justin Tobler· Feb 3, 2026, 00:09 UTC · re: Justin Tobler · lore

[PATCH v2 0/4] odb: support ODB source specific transaction handling

Greetings,

ODB transactions provide a mechanism for writing new objects in the repository that are in a pending state until they are committed. With pluggable ODBs, each source will likely need to handle transactions differently. This patch series prepares ODB transaction handling to be defined for each ODB source by creating a generic odb_transaction structure that stores a callback for committing the transaction.

For now, `struct odb_transaction_files` is the only transaction implementation and what is always returned when creating a transaction. Additionally, this series lifts transaction logic that is common to all backends out of the implementation layer into `odb_transaction_{begin,commit}()` layer.

Changes since V1:
- Renamed the files transaction type to be `odb_transaction_files`.
  Associated functions were also renamed accordingly.
- Updated commit messages.
- The `tmp_objdir` subsystem has been left unchange in the version. In a
  followup series, I plan to couple tmpdirs directly to ODB files
  transactions.

Thanks, -Justin

Justin Tobler (4):
  odb: store ODB source in `struct odb_transaction`
  object-file: rename transaction functions
  odb: prepare `struct odb_transaction` to become generic
  odb: transparently handle common transaction behavior
 object-file.c | 77 +++++++++++++++++++++++++--------------------------
 object-file.h | 10 ++-----
 odb.c         | 19 +++++++++++--
 odb.h         | 17 ++++++++++++
 4 files changed, 74 insertions(+), 49 deletions(-)
Range-diff against v1:
1:  525c96f5f2 = 1:  525c96f5f2 odb: store ODB source in `struct odb_transaction`
2:  5ee1085453 ! 2:  02528f49fb object-file: rename transaction functions
    @@ Commit message
         In a subsequent commit, ODB transactions are made more generic to
         facilitate each ODB source providing its own transaction handling.
         Rename `object_file_transaction_{begin,commit}()` to
    -    `odb_transaction_loose_{begin,commit}()` to better match the future
    +    `odb_transaction_files_{begin,commit}()` to better match the future
         source specific transaction implementation.
     
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
    @@ object-file.c: static void prepare_loose_object_transaction(struct odb_transacti
      	 * the first time an object might be added, since
      	 * callers may not know whether any objects will be
     -	 * added at the time they call object_file_transaction_begin.
    -+	 * added at the time they call odb_transaction_loose_begin.
    ++	 * added at the time they call odb_transaction_files_begin.
      	 */
      	if (!transaction || transaction->objdir)
      		return;
    @@ object-file.c: int read_loose_object(struct repository *repo,
      }
      
     -struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
    -+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    ++struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
      {
      	struct object_database *odb = source->odb;
      
    @@ object-file.c: struct odb_transaction *object_file_transaction_begin(struct odb_
      }
      
     -void object_file_transaction_commit(struct odb_transaction *transaction)
    -+void odb_transaction_loose_commit(struct odb_transaction *transaction)
    ++void odb_transaction_files_commit(struct odb_transaction *transaction)
      {
      	if (!transaction)
      		return;
    @@ object-file.h: struct odb_transaction;
      /*
       * Tell the object database to optimize for adding
     - * multiple objects. object_file_transaction_commit must be called
    -+ * multiple objects. odb_transaction_loose_commit must be called
    ++ * multiple objects. odb_transaction_files_commit must be called
       * to make new objects visible. If a transaction is already
       * pending, NULL is returned.
       */
     -struct odb_transaction *object_file_transaction_begin(struct odb_source *source);
    -+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);
    ++struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
      
      /*
       * Tell the object database to make any objects from the
       * current transaction visible.
       */
     -void object_file_transaction_commit(struct odb_transaction *transaction);
    -+void odb_transaction_loose_commit(struct odb_transaction *transaction);
    ++void odb_transaction_files_commit(struct odb_transaction *transaction);
      
      #endif /* OBJECT_FILE_H */
     
    @@ odb.c: void odb_reprepare(struct object_database *o)
      struct odb_transaction *odb_transaction_begin(struct object_database *odb)
      {
     -	return object_file_transaction_begin(odb->sources);
    -+	return odb_transaction_loose_begin(odb->sources);
    ++	return odb_transaction_files_begin(odb->sources);
      }
      
      void odb_transaction_commit(struct odb_transaction *transaction)
      {
     -	object_file_transaction_commit(transaction);
    -+	odb_transaction_loose_commit(transaction);
    ++	odb_transaction_files_commit(transaction);
      }
3:  a06bfd8c55 ! 3:  081c82428b odb: prepare `struct odb_transaction` to support more sources
    @@ Metadata
     Author: Justin Tobler <jltobler@gmail.com>
     
      ## Commit message ##
    -    odb: prepare `struct odb_transaction` to support more sources
    +    odb: prepare `struct odb_transaction` to become generic
     
    -    Each ODB transaction should be specific to the ODB source it pertains
    -    to. Update `struct odb_transaction` to store a commit callback specific
    -    to its object source type. For now `struct odb_transaction_loose` is the
    +    An ODB transaction handles how objects are stored temporarily and
    +    eventually committed. Due to object storage being implemented
    +    differently for a given ODB source, the ODB transactions must be
    +    implemented in a manner specific to the source the objects are being
    +    written to. To provide generic transactions, `struct odb_transaction` is
    +    updated to store a commit callback that can be configured to support a
    +    specific ODB source. For now `struct odb_transaction_files` is the
         only transaction type and what is always returned when starting a
         transaction.
     
    @@ object-file.c: struct transaction_packfile {
      
     -struct odb_transaction {
     -	struct odb_source *source;
    -+struct odb_transaction_loose {
    ++struct odb_transaction_files {
     +	struct odb_transaction base;
      
      	struct tmp_objdir *objdir;
    @@ object-file.c: struct transaction_packfile {
     -static void prepare_loose_object_transaction(struct odb_transaction *transaction)
     +static void prepare_loose_object_transaction(struct odb_transaction *base)
      {
    -+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
    ++	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
     +
      	/*
      	 * We lazily create the temporary object directory
    @@ object-file.c: static void prepare_loose_object_transaction(struct odb_transacti
     +static void fsync_loose_object_transaction(struct odb_transaction *base,
      					   int fd, const char *filename)
      {
    -+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
    ++	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
     +
      	/*
      	 * If we have an active ODB transaction, we issue a call that
    @@ object-file.c: static void fsync_loose_object_transaction(struct odb_transaction
       * Cleanup after batch-mode fsync_object_files.
       */
     -static void flush_loose_object_transaction(struct odb_transaction *transaction)
    -+static void flush_loose_object_transaction(struct odb_transaction_loose *transaction)
    ++static void flush_loose_object_transaction(struct odb_transaction_files *transaction)
      {
      	struct strbuf temp_path = STRBUF_INIT;
      	struct tempfile *temp;
    @@ object-file.c: static int index_core(struct index_state *istate,
      }
      
     -static int already_written(struct odb_transaction *transaction,
    -+static int already_written(struct odb_transaction_loose *transaction,
    ++static int already_written(struct odb_transaction_files *transaction,
      			   struct object_id *oid)
      {
      	/* The object may already exist in the repository */
    @@ object-file.c: static int already_written(struct odb_transaction *transaction,
      
      /* Lazily create backing packfile for the state */
     -static void prepare_packfile_transaction(struct odb_transaction *transaction,
    -+static void prepare_packfile_transaction(struct odb_transaction_loose *transaction,
    ++static void prepare_packfile_transaction(struct odb_transaction_files *transaction,
      					 unsigned flags)
      {
      	struct transaction_packfile *state = &transaction->packfile;
    @@ object-file.c: static int stream_blob_to_pack(struct transaction_packfile *state
      }
      
     -static void flush_packfile_transaction(struct odb_transaction *transaction)
    -+static void flush_packfile_transaction(struct odb_transaction_loose *transaction)
    ++static void flush_packfile_transaction(struct odb_transaction_files *transaction)
      {
      	struct transaction_packfile *state = &transaction->packfile;
     -	struct repository *repo = transaction->source->odb->repo;
    @@ object-file.c: static void flush_packfile_transaction(struct odb_transaction *tr
       * callers should avoid this code path when filters are requested.
       */
     -static int index_blob_packfile_transaction(struct odb_transaction *transaction,
    -+static int index_blob_packfile_transaction(struct odb_transaction_loose *transaction,
    ++static int index_blob_packfile_transaction(struct odb_transaction_files *transaction,
      					   struct object_id *result_oid, int fd,
      					   size_t size, const char *path,
      					   unsigned flags)
    @@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
     -		transaction = odb_transaction_begin(the_repository->objects);
     -		ret = index_blob_packfile_transaction(the_repository->objects->transaction,
     +		transaction = odb_transaction_begin(odb);
    -+		ret = index_blob_packfile_transaction((struct odb_transaction_loose *)odb->transaction,
    ++		ret = index_blob_packfile_transaction((struct odb_transaction_files *)odb->transaction,
      						      oid, fd,
      						      xsize_t(st->st_size),
      						      path, flags);
    @@ object-file.c: int read_loose_object(struct repository *repo,
      	return ret;
      }
      
    --struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    -+static void odb_transaction_loose_commit(struct odb_transaction *base)
    +-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
    ++static void odb_transaction_files_commit(struct odb_transaction *base)
      {
     -	struct object_database *odb = source->odb;
     -
    @@ object-file.c: int read_loose_object(struct repository *repo,
     -	return odb->transaction;
     -}
     -
    --void odb_transaction_loose_commit(struct odb_transaction *transaction)
    +-void odb_transaction_files_commit(struct odb_transaction *transaction)
     -{
     -	if (!transaction)
     -		return;
    -+	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
    ++	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
      
      	/*
      	 * Ensure the transaction ending matches the pending transaction.
    @@ object-file.c: int read_loose_object(struct repository *repo,
      	free(transaction);
      }
      
    -+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    ++struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
     +{
    -+	struct odb_transaction_loose *transaction;
    ++	struct odb_transaction_files *transaction;
     +	struct object_database *odb = source->odb;
     +
     +	if (odb->transaction)
    @@ object-file.c: int read_loose_object(struct repository *repo,
     +
     +	transaction = xcalloc(1, sizeof(*transaction));
     +	transaction->base.source = source;
    -+	transaction->base.commit = odb_transaction_loose_commit;
    ++	transaction->base.commit = odb_transaction_files_commit;
     +
     +	odb->transaction = &transaction->base;
     +
    @@ object-file.c: int read_loose_object(struct repository *repo,
      ## object-file.h ##
     @@ object-file.h: struct odb_transaction;
       */
    - struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);
    + struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
      
     -/*
     - * Tell the object database to make any objects from the
     - * current transaction visible.
     - */
    --void odb_transaction_loose_commit(struct odb_transaction *transaction);
    +-void odb_transaction_files_commit(struct odb_transaction *transaction);
     -
      #endif /* OBJECT_FILE_H */
     
    @@ odb.c: struct odb_transaction *odb_transaction_begin(struct object_database *odb
      
      void odb_transaction_commit(struct odb_transaction *transaction)
      {
    --	odb_transaction_loose_commit(transaction);
    +-	odb_transaction_files_commit(transaction);
     +	if (!transaction)
     +		return;
     +
4:  476ed1235c ! 4:  ed8164a3a3 odb: transparently handle common transaction behavior
    @@ Commit message
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
      ## object-file.c ##
    -@@ object-file.c: static void odb_transaction_loose_commit(struct odb_transaction *base)
    +@@ object-file.c: static void odb_transaction_files_commit(struct odb_transaction *base)
      {
    - 	struct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;
    + 	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
      
     -	/*
     -	 * Ensure the transaction ending matches the pending transaction.
    @@ object-file.c: static void odb_transaction_loose_commit(struct odb_transaction *
     -	free(transaction);
      }
      
    - struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    -@@ object-file.c: struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)
    + struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
    +@@ object-file.c: struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
      	transaction->base.source = source;
    - 	transaction->base.commit = odb_transaction_loose_commit;
    + 	transaction->base.commit = odb_transaction_files_commit;
      
     -	odb->transaction = &transaction->base;
     -
    @@ odb.c: void odb_reprepare(struct object_database *o)
      
      struct odb_transaction *odb_transaction_begin(struct object_database *odb)
      {
    --	return odb_transaction_loose_begin(odb->sources);
    -+	struct odb_transaction *transaction;
    -+
    +-	return odb_transaction_files_begin(odb->sources);
     +	if (odb->transaction)
     +		return NULL;
     +
    -+	transaction = odb_transaction_loose_begin(odb->sources);
    -+	odb->transaction = transaction;
    ++	odb->transaction = odb_transaction_files_begin(odb->sources);
     +
    -+	return transaction;
    ++	return odb->transaction;
      }
      
      void odb_transaction_commit(struct odb_transaction *transaction)
base-commit: ea717645d199f6f1b66058886475db3e8c9330e9
-- 
2.52.0.373.g68cb7f9e92
Justin Tobler· Feb 3, 2026, 00:09 UTC · re: Justin Tobler · lore

[PATCH v2 1/4] odb: store ODB source in `struct odb_transaction`

Each `struct odb_transaction` currently stores a reference to the `struct object_database`. Since transactions are handled per object source, instead store a reference to the source.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)
Show changes to object-file.c +11 −11
diff --git a/object-file.c b/object-file.c
index e7e4c3348f..196509b252 100644
--- a/object-file.c
+++ b/object-file.c
@@ -711,7 +711,7 @@ struct transaction_packfile {
 };
 
 struct odb_transaction {
-	struct object_database *odb;
+	struct odb_source *source;
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
@@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	if (!transaction || transaction->objdir)
 		return;
 
-	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
+	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
 	if (transaction->objdir)
 		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
 }
@@ -772,7 +772,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)
 	 * the final name is visible.
 	 */
 	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
-		    repo_get_object_directory(transaction->odb->repo));
+		    repo_get_object_directory(transaction->source->odb->repo));
 	temp = xmks_tempfile(temp_path.buf);
 	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
 	delete_tempfile(&temp);
@@ -1344,7 +1344,7 @@ static int already_written(struct odb_transaction *transaction,
 			   struct object_id *oid)
 {
 	/* The object may already exist in the repository */
-	if (odb_has_object(transaction->odb, oid,
+	if (odb_has_object(transaction->source->odb, oid,
 			   HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
 		return 1;
 
@@ -1365,7 +1365,7 @@ static void prepare_packfile_transaction(struct odb_transaction *transaction,
 	if (!(flags & INDEX_WRITE_OBJECT) || state->f)
 		return;
 
-	state->f = create_tmp_packfile(transaction->odb->repo,
+	state->f = create_tmp_packfile(transaction->source->odb->repo,
 				       &state->pack_tmp_name);
 	reset_pack_idx_option(&state->pack_idx_opts);
 
@@ -1469,7 +1469,7 @@ static int stream_blob_to_pack(struct transaction_packfile *state,
 static void flush_packfile_transaction(struct odb_transaction *transaction)
 {
 	struct transaction_packfile *state = &transaction->packfile;
-	struct repository *repo = transaction->odb->repo;
+	struct repository *repo = transaction->source->odb->repo;
 	unsigned char hash[GIT_MAX_RAWSZ];
 	struct strbuf packname = STRBUF_INIT;
 	char *idx_tmp_name = NULL;
@@ -1494,7 +1494,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
 	}
 
 	strbuf_addf(&packname, "%s/pack/pack-%s.",
-		    repo_get_object_directory(transaction->odb->repo),
+		    repo_get_object_directory(transaction->source->odb->repo),
 		    hash_to_hex_algop(hash, repo->hash_algo));
 
 	stage_tmp_packfiles(repo, &packname, state->pack_tmp_name,
@@ -1553,7 +1553,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
 
 	header_len = format_object_header((char *)obuf, sizeof(obuf),
 					  OBJ_BLOB, size);
-	transaction->odb->repo->hash_algo->init_fn(&ctx);
+	transaction->source->odb->repo->hash_algo->init_fn(&ctx);
 	git_hash_update(&ctx, obuf, header_len);
 
 	/* Note: idx is non-NULL when we are writing */
@@ -1993,7 +1993,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
 		return NULL;
 
 	CALLOC_ARRAY(odb->transaction, 1);
-	odb->transaction->odb = odb;
+	odb->transaction->source = source;
 
 	return odb->transaction;
 }
@@ -2006,11 +2006,11 @@ void object_file_transaction_commit(struct odb_transaction *transaction)
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
-	ASSERT(transaction == transaction->odb->transaction);
+	ASSERT(transaction == transaction->source->odb->transaction);
 
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	transaction->odb->transaction = NULL;
+	transaction->source->odb->transaction = NULL;
 	free(transaction);
 }
 
-- 
2.52.0.373.g68cb7f9e92
Justin Tobler· Feb 3, 2026, 00:10 UTC · re: Justin Tobler · lore

[PATCH v2 2/4] object-file: rename transaction functions

In a subsequent commit, ODB transactions are made more generic to facilitate each ODB source providing its own transaction handling. Rename `object_file_transaction_{begin,commit}()` to `odb_transaction_files_{begin,commit}()` to better match the future source specific transaction implementation.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 6 +++---
 object-file.h | 6 +++---
 odb.c         | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)
Show changes to 3 files +8 −8

object-file.c, object-file.h, odb.c

diff --git a/object-file.c b/object-file.c
index 196509b252..7b34a2b274 100644
--- a/object-file.c
+++ b/object-file.c
@@ -723,7 +723,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	 * We lazily create the temporary object directory
 	 * the first time an object might be added, since
 	 * callers may not know whether any objects will be
-	 * added at the time they call object_file_transaction_begin.
+	 * added at the time they call odb_transaction_files_begin.
 	 */
 	if (!transaction || transaction->objdir)
 		return;
@@ -1985,7 +1985,7 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
+struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
 {
 	struct object_database *odb = source->odb;
 
@@ -1998,7 +1998,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)
 	return odb->transaction;
 }
 
-void object_file_transaction_commit(struct odb_transaction *transaction)
+void odb_transaction_files_commit(struct odb_transaction *transaction)
 {
 	if (!transaction)
 		return;
diff --git a/object-file.h b/object-file.h
index 1229d5f675..b4a3341a89 100644
--- a/object-file.h
+++ b/object-file.h
@@ -202,16 +202,16 @@ struct odb_transaction;
 
 /*
  * Tell the object database to optimize for adding
- * multiple objects. object_file_transaction_commit must be called
+ * multiple objects. odb_transaction_files_commit must be called
  * to make new objects visible. If a transaction is already
  * pending, NULL is returned.
  */
-struct odb_transaction *object_file_transaction_begin(struct odb_source *source);
+struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
 
 /*
  * Tell the object database to make any objects from the
  * current transaction visible.
  */
-void object_file_transaction_commit(struct odb_transaction *transaction);
+void odb_transaction_files_commit(struct odb_transaction *transaction);
 
 #endif /* OBJECT_FILE_H */
diff --git a/odb.c b/odb.c
index ac70b6a099..a5e6fd01a9 100644
--- a/odb.c
+++ b/odb.c
@@ -1153,10 +1153,10 @@ void odb_reprepare(struct object_database *o)
 
 struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 {
-	return object_file_transaction_begin(odb->sources);
+	return odb_transaction_files_begin(odb->sources);
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)
 {
-	object_file_transaction_commit(transaction);
+	odb_transaction_files_commit(transaction);
 }
-- 
2.52.0.373.g68cb7f9e92
Justin Tobler· Feb 3, 2026, 00:10 UTC · re: Justin Tobler · lore

[PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

An ODB transaction handles how objects are stored temporarily and eventually committed. Due to object storage being implemented differently for a given ODB source, the ODB transactions must be implemented in a manner specific to the source the objects are being written to. To provide generic transactions, `struct odb_transaction` is updated to store a commit callback that can be configured to support a specific ODB source. For now `struct odb_transaction_files` is the only transaction type and what is always returned when starting a transaction.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 80 ++++++++++++++++++++++++++++-----------------------
 object-file.h |  6 ----
 odb.c         |  5 +++-
 odb.h         | 17 +++++++++++
 4 files changed, 65 insertions(+), 43 deletions(-)
Show changes to 4 files +65 −43

object-file.c, object-file.h, odb.c, odb.h

diff --git a/object-file.c b/object-file.c
index 7b34a2b274..d7e153c1b9 100644
--- a/object-file.c
+++ b/object-file.c
@@ -710,15 +710,17 @@ struct transaction_packfile {
 	uint32_t nr_written;
 };
 
-struct odb_transaction {
-	struct odb_source *source;
+struct odb_transaction_files {
+	struct odb_transaction base;
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
 };
 
-static void prepare_loose_object_transaction(struct odb_transaction *transaction)
+static void prepare_loose_object_transaction(struct odb_transaction *base)
 {
+	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
+
 	/*
 	 * We lazily create the temporary object directory
 	 * the first time an object might be added, since
@@ -728,14 +730,16 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
 	if (!transaction || transaction->objdir)
 		return;
 
-	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
+	transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
 	if (transaction->objdir)
 		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
 }
 
-static void fsync_loose_object_transaction(struct odb_transaction *transaction,
+static void fsync_loose_object_transaction(struct odb_transaction *base,
 					   int fd, const char *filename)
 {
+	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
+
 	/*
 	 * If we have an active ODB transaction, we issue a call that
 	 * cleans the filesystem page cache but avoids a hardware flush
@@ -754,7 +758,7 @@ static void fsync_loose_object_transaction(struct odb_transaction *transaction,
 /*
  * Cleanup after batch-mode fsync_object_files.
  */
-static void flush_loose_object_transaction(struct odb_transaction *transaction)
+static void flush_loose_object_transaction(struct odb_transaction_files *transaction)
 {
 	struct strbuf temp_path = STRBUF_INIT;
 	struct tempfile *temp;
@@ -772,7 +776,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)
 	 * the final name is visible.
 	 */
 	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
-		    repo_get_object_directory(transaction->source->odb->repo));
+		    repo_get_object_directory(transaction->base.source->odb->repo));
 	temp = xmks_tempfile(temp_path.buf);
 	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
 	delete_tempfile(&temp);
@@ -1340,11 +1344,11 @@ static int index_core(struct index_state *istate,
 	return ret;
 }
 
-static int already_written(struct odb_transaction *transaction,
+static int already_written(struct odb_transaction_files *transaction,
 			   struct object_id *oid)
 {
 	/* The object may already exist in the repository */
-	if (odb_has_object(transaction->source->odb, oid,
+	if (odb_has_object(transaction->base.source->odb, oid,
 			   HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
 		return 1;
 
@@ -1358,14 +1362,14 @@ static int already_written(struct odb_transaction *transaction,
 }
 
 /* Lazily create backing packfile for the state */
-static void prepare_packfile_transaction(struct odb_transaction *transaction,
+static void prepare_packfile_transaction(struct odb_transaction_files *transaction,
 					 unsigned flags)
 {
 	struct transaction_packfile *state = &transaction->packfile;
 	if (!(flags & INDEX_WRITE_OBJECT) || state->f)
 		return;
 
-	state->f = create_tmp_packfile(transaction->source->odb->repo,
+	state->f = create_tmp_packfile(transaction->base.source->odb->repo,
 				       &state->pack_tmp_name);
 	reset_pack_idx_option(&state->pack_idx_opts);
 
@@ -1466,10 +1470,10 @@ static int stream_blob_to_pack(struct transaction_packfile *state,
 	return 0;
 }
 
-static void flush_packfile_transaction(struct odb_transaction *transaction)
+static void flush_packfile_transaction(struct odb_transaction_files *transaction)
 {
 	struct transaction_packfile *state = &transaction->packfile;
-	struct repository *repo = transaction->source->odb->repo;
+	struct repository *repo = transaction->base.source->odb->repo;
 	unsigned char hash[GIT_MAX_RAWSZ];
 	struct strbuf packname = STRBUF_INIT;
 	char *idx_tmp_name = NULL;
@@ -1494,7 +1498,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
 	}
 
 	strbuf_addf(&packname, "%s/pack/pack-%s.",
-		    repo_get_object_directory(transaction->source->odb->repo),
+		    repo_get_object_directory(transaction->base.source->odb->repo),
 		    hash_to_hex_algop(hash, repo->hash_algo));
 
 	stage_tmp_packfiles(repo, &packname, state->pack_tmp_name,
@@ -1534,7 +1538,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)
  * binary blobs, they generally do not want to get any conversion, and
  * callers should avoid this code path when filters are requested.
  */
-static int index_blob_packfile_transaction(struct odb_transaction *transaction,
+static int index_blob_packfile_transaction(struct odb_transaction_files *transaction,
 					   struct object_id *result_oid, int fd,
 					   size_t size, const char *path,
 					   unsigned flags)
@@ -1553,7 +1557,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
 
 	header_len = format_object_header((char *)obuf, sizeof(obuf),
 					  OBJ_BLOB, size);
-	transaction->source->odb->repo->hash_algo->init_fn(&ctx);
+	transaction->base.source->odb->repo->hash_algo->init_fn(&ctx);
 	git_hash_update(&ctx, obuf, header_len);
 
 	/* Note: idx is non-NULL when we are writing */
@@ -1629,10 +1633,11 @@ 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 object_database *odb = the_repository->objects;
 		struct odb_transaction *transaction;
 
-		transaction = odb_transaction_begin(the_repository->objects);
-		ret = index_blob_packfile_transaction(the_repository->objects->transaction,
+		transaction = odb_transaction_begin(odb);
+		ret = index_blob_packfile_transaction((struct odb_transaction_files *)odb->transaction,
 						      oid, fd,
 						      xsize_t(st->st_size),
 						      path, flags);
@@ -1985,35 +1990,38 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
+static void odb_transaction_files_commit(struct odb_transaction *base)
 {
-	struct object_database *odb = source->odb;
-
-	if (odb->transaction)
-		return NULL;
-
-	CALLOC_ARRAY(odb->transaction, 1);
-	odb->transaction->source = source;
-
-	return odb->transaction;
-}
-
-void odb_transaction_files_commit(struct odb_transaction *transaction)
-{
-	if (!transaction)
-		return;
+	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
 
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
-	ASSERT(transaction == transaction->source->odb->transaction);
+	ASSERT(base == base->source->odb->transaction);
 
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	transaction->source->odb->transaction = NULL;
+	base->source->odb->transaction = NULL;
 	free(transaction);
 }
 
+struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
+{
+	struct odb_transaction_files *transaction;
+	struct object_database *odb = source->odb;
+
+	if (odb->transaction)
+		return NULL;
+
+	transaction = xcalloc(1, sizeof(*transaction));
+	transaction->base.source = source;
+	transaction->base.commit = odb_transaction_files_commit;
+
+	odb->transaction = &transaction->base;
+
+	return &transaction->base;
+}
+
 struct odb_source_loose *odb_source_loose_new(struct odb_source *source)
 {
 	struct odb_source_loose *loose;
diff --git a/object-file.h b/object-file.h
index b4a3341a89..a62d0de394 100644
--- a/object-file.h
+++ b/object-file.h
@@ -208,10 +208,4 @@ struct odb_transaction;
  */
 struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
 
-/*
- * Tell the object database to make any objects from the
- * current transaction visible.
- */
-void odb_transaction_files_commit(struct odb_transaction *transaction);
-
 #endif /* OBJECT_FILE_H */
diff --git a/odb.c b/odb.c
index a5e6fd01a9..349b4218a5 100644
--- a/odb.c
+++ b/odb.c
@@ -1158,5 +1158,8 @@ struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 
 void odb_transaction_commit(struct odb_transaction *transaction)
 {
-	odb_transaction_files_commit(transaction);
+	if (!transaction)
+		return;
+
+	transaction->commit(transaction);
 }
diff --git a/odb.h b/odb.h
index bab07755f4..83d3a37805 100644
--- a/odb.h
+++ b/odb.h
@@ -77,7 +77,24 @@ struct odb_source {
 struct packed_git;
 struct packfile_store;
 struct cached_object_entry;
+
+/*
+ * A transaction may be started for an object database prior to writing new
+ * objects via odb_transaction_begin(). These objects are not committed until
+ * odb_transaction_commit() is invoked. Only a single transaction may be pending
+ * at a time.
+ *
+ * Each ODB source is expected to implement its own transaction handling.
+ */
 struct odb_transaction;
+typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
+struct odb_transaction {
+	/* The ODB source the transaction is opened against. */
+	struct odb_source *source;
+
+	/* The ODB source specific callback invoked to commit a transaction. */
+	odb_transaction_commit_fn commit;
+};
 
 /*
  * The object database encapsulates access to objects in a repository. It
-- 
2.52.0.373.g68cb7f9e92
Toon Claes· Feb 3, 2026, 15:54 UTC · re: Justin Tobler · lore

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

Justin Tobler <jltobler@gmail.com> writes:
Show 39 quoted lines
> An ODB transaction handles how objects are stored temporarily and
> eventually committed. Due to object storage being implemented
> differently for a given ODB source, the ODB transactions must be
> implemented in a manner specific to the source the objects are being
> written to. To provide generic transactions, `struct odb_transaction` is
> updated to store a commit callback that can be configured to support a
> specific ODB source. For now `struct odb_transaction_files` is the
> only transaction type and what is always returned when starting a
> transaction.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  object-file.c | 80 ++++++++++++++++++++++++++++-----------------------
>  object-file.h |  6 ----
>  odb.c         |  5 +++-
>  odb.h         | 17 +++++++++++
>  4 files changed, 65 insertions(+), 43 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 7b34a2b274..d7e153c1b9 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -710,15 +710,17 @@ struct transaction_packfile {
>  	uint32_t nr_written;
>  };
>  
> -struct odb_transaction {
> -	struct odb_source *source;
> +struct odb_transaction_files {
> +	struct odb_transaction base;
>  
>  	struct tmp_objdir *objdir;
>  	struct transaction_packfile packfile;
>  };
>  
> -static void prepare_loose_object_transaction(struct odb_transaction *transaction)
> +static void prepare_loose_object_transaction(struct odb_transaction *base)
>  {
> +	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;

So you're assuming `struct odb_transaction` is the first field in `struct odb_transaction_files`?

I think it would be safer to do this instead:

+ struct odb_transaction_files *transaction = + container_of(base, struct odb_transaction_files, base);

(this also can be applied in a few other places in this patch)
-- 
Cheers,
Toon
Justin Tobler· Feb 3, 2026, 16:46 UTC · re: Toon Claes · lore

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

On 26/02/03 04:54PM, Toon Claes wrote:
Show 8 quoted lines
> Justin Tobler <jltobler@gmail.com> writes:
> > -static void prepare_loose_object_transaction(struct odb_transaction *transaction)
> > +static void prepare_loose_object_transaction(struct odb_transaction *base)
> >  {
> > +	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
> 
> So you're assuming `struct odb_transaction` is the first field in
> `struct odb_transaction_files`?

Yes, `struct odb_transaction_files` contains a reference to `struct odb_transaction` as its first member which is the "base" transaction type. This way we can easily cast to the containing "concrete" type.

Show 6 quoted lines
> I think it would be safer to do this instead:
> 
> +	struct odb_transaction_files *transaction =
> +		container_of(base, struct odb_transaction_files, base);
> 
> (this also can be applied in a few other places in this patch)

So long as `struct odb_transaction` remains the first member of `struct odb_transaction_files`, using `container_of()` is functionally the same thing as casting. This is also how we do things for `odb_read_stream` and `odb_loose_read_stream` so I don't think it is too big of a deal either way.

I've made the change locally, but will hold off from sending another version unless folks feel strongly about this change or there is other feedback.

Thanks, -Justin

Junio C Hamano· Feb 3, 2026, 22:54 UTC · re: Justin Tobler · lore

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

Justin Tobler <jltobler@gmail.com> writes:
Show 5 quoted lines
> So long as `struct odb_transaction` remains the first member of `struct
> odb_transaction_files`, using `container_of()` is functionally the same
> thing as casting. This is also how we do things for `odb_read_stream`
> and `odb_loose_read_stream` so I don't think it is too big of a deal
> either way.

It would be a good theme to clean them all up, together with existing ones, in a separate topic. Code hygiene matters.

Patrick Steinhardt· Feb 4, 2026, 06:26 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

On Tue, Feb 03, 2026 at 02:54:08PM -0800, Junio C Hamano wrote:
Show 10 quoted lines
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > So long as `struct odb_transaction` remains the first member of `struct
> > odb_transaction_files`, using `container_of()` is functionally the same
> > thing as casting. This is also how we do things for `odb_read_stream`
> > and `odb_loose_read_stream` so I don't think it is too big of a deal
> > either way.
> 
> It would be a good theme to clean them all up, together with
> existing ones, in a separate topic.  Code hygiene matters.

Fair indeed. Justin, will you handle this cleanup once this topic here is merged or shall I do it?

Patrick
Justin Tobler· Feb 4, 2026, 17:15 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

On 26/02/04 07:26AM, Patrick Steinhardt wrote:
Show 14 quoted lines
> On Tue, Feb 03, 2026 at 02:54:08PM -0800, Junio C Hamano wrote:
> > Justin Tobler <jltobler@gmail.com> writes:
> > 
> > > So long as `struct odb_transaction` remains the first member of `struct
> > > odb_transaction_files`, using `container_of()` is functionally the same
> > > thing as casting. This is also how we do things for `odb_read_stream`
> > > and `odb_loose_read_stream` so I don't think it is too big of a deal
> > > either way.
> > 
> > It would be a good theme to clean them all up, together with
> > existing ones, in a separate topic.  Code hygiene matters.
> 
> Fair indeed. Justin, will you handle this cleanup once this topic here
> is merged or shall I do it?

I've already have a patch that addresses `odb_transaction_files` and `odb_loose_read_stream`. I'll check to see if there are any others I notice and send a followup series after this one gets merged.

-Justin
Karthik Nayak· Feb 4, 2026, 10:31 UTC · re: Justin Tobler · lore

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

Justin Tobler <jltobler@gmail.com> writes:
[snip]
Show 10 quoted lines
> +
> +/*
> + * A transaction may be started for an object database prior to writing new
> + * objects via odb_transaction_begin(). These objects are not committed until
> + * odb_transaction_commit() is invoked. Only a single transaction may be pending
> + * at a time.
> + *
> + * Each ODB source is expected to implement its own transaction handling.
> + */
>  struct odb_transaction;
Nit: Wouldn't it be nicer to just the below `struct odb_transaction`
here and drop this line?
Show 13 quoted lines
> +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
> +struct odb_transaction {
> +	/* The ODB source the transaction is opened against. */
> +	struct odb_source *source;
> +
> +	/* The ODB source specific callback invoked to commit a transaction. */
> +	odb_transaction_commit_fn commit;
> +};
>
>  /*
>   * The object database encapsulates access to objects in a repository. It
> --
> 2.52.0.373.g68cb7f9e92

Just a question in general, is the idea to eventually also add support for {prepare, rollback} within odb transactions?

Justin Tobler· Feb 4, 2026, 17:38 UTC · re: Karthik Nayak · lore

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

On 26/02/04 02:31AM, Karthik Nayak wrote:
Show 17 quoted lines
> Justin Tobler <jltobler@gmail.com> writes:
> 
> [snip]
> 
> > +
> > +/*
> > + * A transaction may be started for an object database prior to writing new
> > + * objects via odb_transaction_begin(). These objects are not committed until
> > + * odb_transaction_commit() is invoked. Only a single transaction may be pending
> > + * at a time.
> > + *
> > + * Each ODB source is expected to implement its own transaction handling.
> > + */
> >  struct odb_transaction;
> 
> Nit: Wouldn't it be nicer to just the below `struct odb_transaction`
> here and drop this line?

I assume you mean drop the typedef in favor of defining it directly in the struct and thus removing the need for the forward declation. I kind of like having a typedef for the function callback, but I don't feel too strongly either way. I've ammended locally, but will hold off from sending another version unless there is other feedback.

Show 16 quoted lines
> > +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
> > +struct odb_transaction {
> > +	/* The ODB source the transaction is opened against. */
> > +	struct odb_source *source;
> > +
> > +	/* The ODB source specific callback invoked to commit a transaction. */
> > +	odb_transaction_commit_fn commit;
> > +};
> >
> >  /*
> >   * The object database encapsulates access to objects in a repository. It
> > --
> > 2.52.0.373.g68cb7f9e92
> 
> Just a question in general, is the idea to eventually also add support
> for {prepare, rollback} within odb transactions?

I'm not quite sure yet about "prepare", but certainly an "abort" or "rollback" will be introduced in a followup series. This will be useful as we expand ODB transaction usage to other operations that require the ability to remove temporary objects such as remerge-diffs.

Thanks, -Justin

Karthik Nayak· Feb 5, 2026, 11:20 UTC · re: Justin Tobler · lore

Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

Justin Tobler <jltobler@gmail.com> writes:
Show 25 quoted lines
> On 26/02/04 02:31AM, Karthik Nayak wrote:
>> Justin Tobler <jltobler@gmail.com> writes:
>>
>> [snip]
>>
>> > +
>> > +/*
>> > + * A transaction may be started for an object database prior to writing new
>> > + * objects via odb_transaction_begin(). These objects are not committed until
>> > + * odb_transaction_commit() is invoked. Only a single transaction may be pending
>> > + * at a time.
>> > + *
>> > + * Each ODB source is expected to implement its own transaction handling.
>> > + */
>> >  struct odb_transaction;
>>
>> Nit: Wouldn't it be nicer to just the below `struct odb_transaction`
>> here and drop this line?
>
> I assume you mean drop the typedef in favor of defining it directly in
> the struct and thus removing the need for the forward declation. I kind
> of like having a typedef for the function callback, but I don't feel too
> strongly either way. I've ammended locally, but will hold off from
> sending another version unless there is other feedback.
>
All good.
Show 22 quoted lines
>> > +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);
>> > +struct odb_transaction {
>> > +	/* The ODB source the transaction is opened against. */
>> > +	struct odb_source *source;
>> > +
>> > +	/* The ODB source specific callback invoked to commit a transaction. */
>> > +	odb_transaction_commit_fn commit;
>> > +};
>> >
>> >  /*
>> >   * The object database encapsulates access to objects in a repository. It
>> > --
>> > 2.52.0.373.g68cb7f9e92
>>
>> Just a question in general, is the idea to eventually also add support
>> for {prepare, rollback} within odb transactions?
>
> I'm not quite sure yet about "prepare", but certainly an "abort" or
> "rollback" will be introduced in a followup series. This will be useful
> as we expand ODB transaction usage to other operations that require the
> ability to remove temporary objects such as remerge-diffs.
>
Alright.
Justin Tobler· Feb 3, 2026, 00:10 UTC · re: Justin Tobler · lore

[PATCH v2 4/4] odb: transparently handle common transaction behavior

A new ODB transaction is created and returned via `odb_transaction_begin()` and stored in the ODB. Only a single transaction may be pending at a time. If the ODB already has a transaction, the function is expected to return NULL. Similarly, when committing a transaction via `odb_transaction_commit()` the transaction being committed must match the pending transaction and upon commit reset the ODB transaction to NULL.

These behaviors apply regardless of the ODB transaction implementation. Move the corresponding logic into `odb_transaction_{begin,commit}()` accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c |  9 ---------
 odb.c         | 14 +++++++++++++-
 2 files changed, 13 insertions(+), 10 deletions(-)
Show changes to 2 files +13 −10

object-file.c, odb.c

diff --git a/object-file.c b/object-file.c
index d7e153c1b9..1b62996ef0 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1994,15 +1994,8 @@ static void odb_transaction_files_commit(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
 
-	/*
-	 * Ensure the transaction ending matches the pending transaction.
-	 */
-	ASSERT(base == base->source->odb->transaction);
-
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
-	base->source->odb->transaction = NULL;
-	free(transaction);
 }
 
 struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
@@ -2017,8 +2010,6 @@ struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
 	transaction->base.source = source;
 	transaction->base.commit = odb_transaction_files_commit;
 
-	odb->transaction = &transaction->base;
-
 	return &transaction->base;
 }
 
diff --git a/odb.c b/odb.c
index 349b4218a5..1679cc0465 100644
--- a/odb.c
+++ b/odb.c
@@ -1153,7 +1153,12 @@ void odb_reprepare(struct object_database *o)
 
 struct odb_transaction *odb_transaction_begin(struct object_database *odb)
 {
-	return odb_transaction_files_begin(odb->sources);
+	if (odb->transaction)
+		return NULL;
+
+	odb->transaction = odb_transaction_files_begin(odb->sources);
+
+	return odb->transaction;
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)
@@ -1161,5 +1166,12 @@ void odb_transaction_commit(struct odb_transaction *transaction)
 	if (!transaction)
 		return;
 
+	/*
+	 * Ensure the transaction ending matches the pending transaction.
+	 */
+	ASSERT(transaction == transaction->source->odb->transaction);
+
 	transaction->commit(transaction);
+	transaction->source->odb->transaction = NULL;
+	free(transaction);
 }
-- 
2.52.0.373.g68cb7f9e92
Karthik Nayak· Feb 4, 2026, 10:34 UTC · re: Justin Tobler · lore

Re: [PATCH v2 4/4] odb: transparently handle common transaction behavior

Justin Tobler <jltobler@gmail.com> writes:
Show 8 quoted lines
> A new ODB transaction is created and returned via
> `odb_transaction_begin()` and stored in the ODB. Only a single
> transaction may be pending at a time. If the ODB already has a
> transaction, the function is expected to return NULL. Similarly, when
> committing a transaction via `odb_transaction_commit()` the transaction
> being committed must match the pending transaction and upon commit reset
> the ODB transaction to NULL.
>

But isn't this merely a limitation of the current implementation of the files transactions? Couldn't a potential ODB source support parallel transactions where this might no longer hold?

> These behaviors apply regardless of the ODB transaction implementation.
> Move the corresponding logic into `odb_transaction_{begin,commit}()`
> accordingly.
>
[snip]
Justin Tobler· Feb 4, 2026, 17:50 UTC · re: Karthik Nayak · lore

Re: [PATCH v2 4/4] odb: transparently handle common transaction behavior

On 26/02/04 10:34AM, Karthik Nayak wrote:
Show 13 quoted lines
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > A new ODB transaction is created and returned via
> > `odb_transaction_begin()` and stored in the ODB. Only a single
> > transaction may be pending at a time. If the ODB already has a
> > transaction, the function is expected to return NULL. Similarly, when
> > committing a transaction via `odb_transaction_commit()` the transaction
> > being committed must match the pending transaction and upon commit reset
> > the ODB transaction to NULL.
> 
> But isn't this merely a limitation of the current implementation of the
> files transactions? Couldn't a potential ODB source support parallel
> transactions where this might no longer hold?

Just to clarify, this limitation exists per Git process. For the time being, we only support writing objects to a single ODB source so a single transaction for object writes seems reasonable for now. Furthermore, the current "files" transaction backend relies on the tmp_odjdir subsystem which means only a single temp odjdir may exist for a Git process to write objects to.

-Justin
Karthik Nayak· Feb 5, 2026, 11:22 UTC · re: Justin Tobler · lore

Re: [PATCH v2 4/4] odb: transparently handle common transaction behavior

Justin Tobler <jltobler@gmail.com> writes:
Show 16 quoted lines
> On 26/02/04 10:34AM, Karthik Nayak wrote:
>> Justin Tobler <jltobler@gmail.com> writes:
>>
>> > A new ODB transaction is created and returned via
>> > `odb_transaction_begin()` and stored in the ODB. Only a single
>> > transaction may be pending at a time. If the ODB already has a
>> > transaction, the function is expected to return NULL. Similarly, when
>> > committing a transaction via `odb_transaction_commit()` the transaction
>> > being committed must match the pending transaction and upon commit reset
>> > the ODB transaction to NULL.
>>
>> But isn't this merely a limitation of the current implementation of the
>> files transactions? Couldn't a potential ODB source support parallel
>> transactions where this might no longer hold?
>
> Just to clarify, this limitation exists per Git process.
I missed this, makes sense now.
Show 8 quoted lines
> For the time
> being, we only support writing objects to a single ODB source so a
> single transaction for object writes seems reasonable for now.
> Furthermore, the current "files" transaction backend relies on the
> tmp_odjdir subsystem which means only a single temp odjdir may exist for
> a Git process to write objects to.
>
> -Justin
Thanks
Junio C Hamano· Feb 3, 2026, 01:16 UTC · re: Justin Tobler · lore

Re: [PATCH v2 0/4] odb: support ODB source specific transaction handling

Justin Tobler <jltobler@gmail.com> writes:
Show 8 quoted lines
> Changes since V1:
>
> - Renamed the files transaction type to be `odb_transaction_files`.
>   Associated functions were also renamed accordingly.
> - Updated commit messages.
> - The `tmp_objdir` subsystem has been left unchange in the version. In a
>   followup series, I plan to couple tmpdirs directly to ODB files
>   transactions.

Ah, I was wondering why the changes are so small (it looked essentially s/loose/files/ and nothing else to me), but if we are leaving the tmp_objdir stuff out of the scope (which is sensible), then there aren't much left to do relative to the previous iteration.

Looking good.  Will replace.
Thanks.
Patrick Steinhardt· Feb 4, 2026, 06:25 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 0/4] odb: support ODB source specific transaction handling

On Mon, Feb 02, 2026 at 05:16:11PM -0800, Junio C Hamano wrote:
Show 16 quoted lines
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > Changes since V1:
> >
> > - Renamed the files transaction type to be `odb_transaction_files`.
> >   Associated functions were also renamed accordingly.
> > - Updated commit messages.
> > - The `tmp_objdir` subsystem has been left unchange in the version. In a
> >   followup series, I plan to couple tmpdirs directly to ODB files
> >   transactions.
> 
> Ah, I was wondering why the changes are so small (it looked
> essentially s/loose/files/ and nothing else to me), but if we are
> leaving the tmp_objdir stuff out of the scope (which is sensible),
> then there aren't much left to do relative to the previous
> iteration.

Yeah, agreed. As long as there is a subsequent patch series that fixes the `tmp_objdir` thing I'm happy. It'd be a no-op refactoring for now anyway given that we always write via the temporary object directory, but it's a good cleanup to do on top.

Anyway, I'm happy with the status quo of this patch series, thanks!
Patrick

← back to recent threads