[PATCH 0/4] object-file: eagerly create ODB transaction temporary directory

[PATCH 0/4] object-file: eagerly create ODB transaction temporary directory

[Justin Tobler]

Greetings,

This series continues work for the "files" ODB transactions and makes it
so the underlying transaction temporary directory is created eagerly.
Previously, by default, it would wait for the first object write to
lazily create the temporary object directory. Furthermore, this series
also ensures that objects are written to the temporary directory for all
transactional object writes, not just when batched fsync is configured.

The first three patches consist of some minor preparatory refactors. The
final patch swaps to creating the temporary directory eagerly and drops
the lazy setup.

This series is based on 3c60ef1e38 (The 17th batch, 2026-11-08) with
jt/tmp-objdir-loose-idx-merge merged into it.

Thanks,
-Justin

Justin Tobler (4):
  object-file: rename default transaction prefix
  object-file: only flush batched fsync writes when enabled
  tmp-objdir: preserve errno when directory creation fails
  object-file: create ODB transaction temporary directory eagerly

 object-file.c         | 99 ++++++++++++++++---------------------------
 object-file.h         |  1 -
 odb/source-loose.c    |  6 ---
 t/t0004-unwritable.sh | 19 ++++-----
 tmp-objdir.c          |  8 ++++
 tmp-objdir.h          |  3 +-
 6 files changed, 54 insertions(+), 82 deletions(-)

-- 
2.56.0.173.gf57c9a7388

[PATCH 1/4] object-file: rename default transaction prefix

[Justin Tobler]

By default, the temporary directory created for a "files" ODB
transaction has its name prefixed with "bulk-fsync". This name dates
back to c0f4752ed2 (core.fsyncmethod: batched disk flushes for
loose-objects, 2022-04-04), where the temporary object directory only
served to batch fsyncs of loose objects.

ODB transactions have since grown into a more general interface for
writing objects though, and the temporary object directory is not
inherently tied to how objects are synced. In a subsequent commit, the
temporary object directory is created for every transaction regardless
of the configured fsync method, which would make the name misleading.

Rename the default prefix to "transaction" accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/object-file.c b/object-file.c
index 132aaab6f8..201d9d6490 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1566,7 +1566,7 @@ int odb_transaction_files_begin(struct odb_source *source,
 	transaction->flags = flags;
 	strvec_init(&transaction->pack_names);
 
-	transaction->prefix = "bulk-fsync";
+	transaction->prefix = "transaction";
 	if (flags & ODB_TRANSACTION_RECEIVE) {
 		/*
 		 * ODB transactions for git-receive-pack(1) eagerly create a
-- 
2.56.0.173.gf57c9a7388

Re: [PATCH 1/4] object-file: rename default transaction prefix

[Patrick Steinhardt]

On Mon, Nov 09, 2026 at 06:22:48PM -0600, Justin Tobler wrote:
> By default, the temporary directory created for a "files" ODB
> transaction has its name prefixed with "bulk-fsync". This name dates
> back to c0f4752ed2 (core.fsyncmethod: batched disk flushes for
> loose-objects, 2022-04-04), where the temporary object directory only
> served to batch fsyncs of loose objects.
> 
> ODB transactions have since grown into a more general interface for
> writing objects though, and the temporary object directory is not
> inherently tied to how objects are synced.

Right. We don't really use it for a lot of cases yet, but we anticipate
that we'll start using it more often now that it becomes more generally
useful.

> In a subsequent commit, the
> temporary object directory is created for every transaction regardless
> of the configured fsync method, which would make the name misleading.
> 
> Rename the default prefix to "transaction" accordingly.

We could of course change the logic so that we make the prefix dependent
on the actual caller. But I really doubt that this would be sensible, so
I'm very much aligned with naming those directories more generically.

Patrick

Re: [PATCH 1/4] object-file: rename default transaction prefix

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

> By default, the temporary directory created for a "files" ODB
> transaction has its name prefixed with "bulk-fsync". This name dates
> back to c0f4752ed2 (core.fsyncmethod: batched disk flushes for
> loose-objects, 2022-04-04), where the temporary object directory only
> served to batch fsyncs of loose objects.
>
> ODB transactions have since grown into a more general interface for
> writing objects though, and the temporary object directory is not
> inherently tied to how objects are synced. In a subsequent commit, the
> temporary object directory is created for every transaction regardless
> of the configured fsync method, which would make the name misleading.
>
> Rename the default prefix to "transaction" accordingly.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  object-file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/object-file.c b/object-file.c
> index 132aaab6f8..201d9d6490 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1566,7 +1566,7 @@ int odb_transaction_files_begin(struct odb_source *source,
>  	transaction->flags = flags;
>  	strvec_init(&transaction->pack_names);
>
> -	transaction->prefix = "bulk-fsync";
> +	transaction->prefix = "transaction";

Okay so the prefix is fed to `tmp_objdir_create()`, which will then name
it 'tmp_objdir-transaction-XXXXXX'. So this makes sense as we use ODB
transactions in a general sense.

>  	if (flags & ODB_TRANSACTION_RECEIVE) {
>  		/*
>  		 * ODB transactions for git-receive-pack(1) eagerly create a
> --
> 2.56.0.173.gf57c9a7388

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH 2/4] object-file: only flush batched fsync writes when enabled

[Justin Tobler]

When committing an ODB transaction for the "files" backend, a full
hardware flush is issued against a temporary file before the staged
objects are migrated into the main object database. This flush acts as a
barrier for objects that have been written with batched fsync, where
`odb_transaction_files_fsync()` only issues a writeout request for each
object instead of fully syncing it.

The flush is issued whenever the transaction has a temporary object
directory though, regardless of whether batched fsync is actually
enabled. For transactions configured for git-receive-pack(1), the
temporary object directory is always created, so the flush is also
performed when objects are synced individually or not synced at all. In
those cases there are no pending writeout requests and the flush is
unnecessary. Only issue the flush when batched fsync is enabled
accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/object-file.c b/object-file.c
index 201d9d6490..da189111bd 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1330,6 +1330,29 @@ static int write_pack_lockfiles(struct odb_transaction_files *transaction)
 	return ret;
 }
 
+static void flush_batched_fsync(struct repository *repo)
+{
+	struct strbuf temp_path = STRBUF_INIT;
+	struct tempfile *temp;
+
+	/*
+	 * Issue a full hardware flush against a temporary file to ensure
+	 * that all objects are durable before any renames occur. The code in
+	 * odb_transaction_files_fsync has already issued a writeout
+	 * request, but it has not flushed any writeback cache in the storage
+	 * hardware or any filesystem logs. This fsync call acts as a barrier
+	 * to ensure that the data in each new object file is durable before
+	 * the final name is visible.
+	 */
+	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
+		    repo_get_object_directory(repo));
+	temp = xmks_tempfile(temp_path.buf);
+	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
+
+	strbuf_release(&temp_path);
+	delete_tempfile(&temp);
+}
+
 static int odb_transaction_files_commit(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction =
@@ -1339,24 +1362,8 @@ static int odb_transaction_files_commit(struct odb_transaction *base)
 	flush_packfile_transaction(transaction);
 
 	if (transaction->objdir) {
-		struct strbuf temp_path = STRBUF_INIT;
-		struct tempfile *temp;
-
-		/*
-		 * Issue a full hardware flush against a temporary file to ensure
-		 * that all objects are durable before any renames occur. The code in
-		 * odb_transaction_files_fsync has already issued a writeout
-		 * request, but it has not flushed any writeback cache in the storage
-		 * hardware or any filesystem logs. This fsync call acts as a barrier
-		 * to ensure that the data in each new object file is durable before
-		 * the final name is visible.
-		 */
-		strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
-			    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);
-		strbuf_release(&temp_path);
+		if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
+			flush_batched_fsync(transaction->base.source->odb->repo);
 
 		if (write_pack_lockfiles(transaction))
 			return error(_("unable to lock packfiles"));
-- 
2.56.0.173.gf57c9a7388

Re: [PATCH 2/4] object-file: only flush batched fsync writes when enabled

[Patrick Steinhardt]

On Mon, Nov 09, 2026 at 06:22:49PM -0600, Justin Tobler wrote:
> When committing an ODB transaction for the "files" backend, a full
> hardware flush is issued against a temporary file before the staged
> objects are migrated into the main object database. This flush acts as a
> barrier for objects that have been written with batched fsync, where
> `odb_transaction_files_fsync()` only issues a writeout request for each
> object instead of fully syncing it.

Hmm, okay. The difference here is bascially that we don't issue a
hardware flush. So we know that the data is not in the page cache
anymore, but it may still only be cached by the disk.

> The flush is issued whenever the transaction has a temporary object
> directory though, regardless of whether batched fsync is actually
> enabled. For transactions configured for git-receive-pack(1), the
> temporary object directory is always created, so the flush is also
> performed when objects are synced individually or not synced at all. In
> those cases there are no pending writeout requests and the flush is
> unnecessary. Only issue the flush when batched fsync is enabled
> accordingly.

So this is essentially a performance optimization?

> diff --git a/object-file.c b/object-file.c
> index 201d9d6490..da189111bd 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1330,6 +1330,29 @@ static int write_pack_lockfiles(struct odb_transaction_files *transaction)
>  	return ret;
>  }
>  
> +static void flush_batched_fsync(struct repository *repo)

Okay. Instead of adding more nesting you extract the code into a
function that gets called conditionally. Fair.

> +{
> +	struct strbuf temp_path = STRBUF_INIT;
> +	struct tempfile *temp;
> +
> +	/*
> +	 * Issue a full hardware flush against a temporary file to ensure
> +	 * that all objects are durable before any renames occur. The code in
> +	 * odb_transaction_files_fsync has already issued a writeout
> +	 * request, but it has not flushed any writeback cache in the storage
> +	 * hardware or any filesystem logs. This fsync call acts as a barrier
> +	 * to ensure that the data in each new object file is durable before
> +	 * the final name is visible.
> +	 */
> +	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
> +		    repo_get_object_directory(repo));
> +	temp = xmks_tempfile(temp_path.buf);
> +	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
> +
> +	strbuf_release(&temp_path);
> +	delete_tempfile(&temp);
> +}
> +
>  static int odb_transaction_files_commit(struct odb_transaction *base)
>  {
>  	struct odb_transaction_files *transaction =
> @@ -1339,24 +1362,8 @@ static int odb_transaction_files_commit(struct odb_transaction *base)
>  	flush_packfile_transaction(transaction);
>  
>  	if (transaction->objdir) {
> -		struct strbuf temp_path = STRBUF_INIT;
> -		struct tempfile *temp;
> -
> -		/*
> -		 * Issue a full hardware flush against a temporary file to ensure
> -		 * that all objects are durable before any renames occur. The code in
> -		 * odb_transaction_files_fsync has already issued a writeout
> -		 * request, but it has not flushed any writeback cache in the storage
> -		 * hardware or any filesystem logs. This fsync call acts as a barrier
> -		 * to ensure that the data in each new object file is durable before
> -		 * the final name is visible.
> -		 */
> -		strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
> -			    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);
> -		strbuf_release(&temp_path);
> +		if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
> +			flush_batched_fsync(transaction->base.source->odb->repo);

I assume that we only support batched fsyncs for loose objects in the
first place? In other words, if a transaction were to write a bunch of
packfiles, it would not be possible to batch-fsync those right now?

If so this new condition correct to me. But I think this should be
documented as part of the commit message, because it's an implicit
assumption.

Patrick

Re: [PATCH 2/4] object-file: only flush batched fsync writes when enabled

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

> When committing an ODB transaction for the "files" backend, a full
> hardware flush is issued against a temporary file before the staged
> objects are migrated into the main object database. This flush acts as a
> barrier for objects that have been written with batched fsync, where
> `odb_transaction_files_fsync()` only issues a writeout request for each
> object instead of fully syncing it.
>

Does `fsync(fd)` even guarantee syncing of previous writes?

> The flush is issued whenever the transaction has a temporary object
> directory though, regardless of whether batched fsync is actually
> enabled. For transactions configured for git-receive-pack(1), the
> temporary object directory is always created, so the flush is also
> performed when objects are synced individually or not synced at all. In
> those cases there are no pending writeout requests and the flush is
> unnecessary. Only issue the flush when batched fsync is enabled
> accordingly.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  object-file.c | 43 +++++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 201d9d6490..da189111bd 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1330,6 +1330,29 @@ static int write_pack_lockfiles(struct odb_transaction_files *transaction)
>  	return ret;
>  }
>
> +static void flush_batched_fsync(struct repository *repo)
> +{
> +	struct strbuf temp_path = STRBUF_INIT;
> +	struct tempfile *temp;
> +
> +	/*
> +	 * Issue a full hardware flush against a temporary file to ensure
> +	 * that all objects are durable before any renames occur. The code in
> +	 * odb_transaction_files_fsync has already issued a writeout
> +	 * request, but it has not flushed any writeback cache in the storage
> +	 * hardware or any filesystem logs. This fsync call acts as a barrier
> +	 * to ensure that the data in each new object file is durable before
> +	 * the final name is visible.
> +	 */

Nit: This could become a function comment, since it now describes the
function well.

> +	strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
> +		    repo_get_object_directory(repo));
> +	temp = xmks_tempfile(temp_path.buf);
> +	fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
> +
> +	strbuf_release(&temp_path);
> +	delete_tempfile(&temp);
> +}
> +

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH 3/4] tmp-objdir: preserve errno when directory creation fails

[Justin Tobler]

When `tmp_objdir_create()` fails to create the temporary object
directory, it cleans up after itself before returning NULL. Depending on
where the failure occurred, this cleanup may clobber `errno`, leaving
callers without a way to reliably tell why creating the directory
failed.

In a subsequent commit, ODB transactions create their temporary object
directory eagerly and report a more specific error regarding creation
failure. Preserve `errno` across the cleanup so that callers can inspect
it accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 tmp-objdir.c | 8 ++++++++
 tmp-objdir.h | 3 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/tmp-objdir.c b/tmp-objdir.c
index 5307717354..4b91d86787 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -148,10 +148,13 @@ static int setup_tmp_objdir(const char *root)
 {
 	char *path;
 	int ret = 0;
+	int saved_errno;
 
 	path = xstrfmt("%s/pack", root);
 	ret = mkdir(path, 0777);
+	saved_errno = errno;
 	free(path);
+	errno = saved_errno;
 
 	return ret;
 }
@@ -163,6 +166,7 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
 	struct odb_source_files *files = odb_source_files_downcast(r->objects->source);
 	static int installed_handlers;
 	struct tmp_objdir *t;
+	int saved_errno;
 
 	if (the_tmp_objdir)
 		BUG("only one tmp_objdir can be used at a time");
@@ -185,8 +189,10 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
 		chdir_notify_register(tmp_objdir_reparent, t);
 
 	if (!mkdtemp(t->path.buf)) {
+		saved_errno = errno;
 		/* free, not destroy, as we never touched the filesystem */
 		tmp_objdir_free(t);
+		errno = saved_errno;
 		return NULL;
 	}
 
@@ -197,7 +203,9 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
 	}
 
 	if (setup_tmp_objdir(t->path.buf)) {
+		saved_errno = errno;
 		tmp_objdir_destroy(t);
+		errno = saved_errno;
 		return NULL;
 	}
 
diff --git a/tmp-objdir.h b/tmp-objdir.h
index a301b01d84..4ef6750a01 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -26,7 +26,8 @@ struct tmp_objdir;
 
 /*
  * Create a new temporary object directory with the specified prefix and
- * install the directory as the primary write target; returns NULL on failure.
+ * install the directory as the primary write target; returns NULL and sets
+ * errno on failure.
  */
 struct tmp_objdir *tmp_objdir_create(struct repository *r, const char *prefix,
 				     int will_destroy);
-- 
2.56.0.173.gf57c9a7388

Re: [PATCH 3/4] tmp-objdir: preserve errno when directory creation fails

[Patrick Steinhardt]

On Mon, Nov 09, 2026 at 06:22:50PM -0600, Justin Tobler wrote:
> diff --git a/tmp-objdir.c b/tmp-objdir.c
> index 5307717354..4b91d86787 100644
> --- a/tmp-objdir.c
> +++ b/tmp-objdir.c
> @@ -148,10 +148,13 @@ static int setup_tmp_objdir(const char *root)
>  {
>  	char *path;
>  	int ret = 0;
> +	int saved_errno;
>  
>  	path = xstrfmt("%s/pack", root);
>  	ret = mkdir(path, 0777);
> +	saved_errno = errno;
>  	free(path);
> +	errno = saved_errno;
>  
>  	return ret;
>  }

This whole function here is somewhat pointless, it's only really
required because memory handling would otherwise be awkward. That
could've been avoided if its only caller had a single return path, but
it doesn't.

No need to fix that as part of your patch series though, if you ask me.

> @@ -163,6 +166,7 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
>  	struct odb_source_files *files = odb_source_files_downcast(r->objects->source);
>  	static int installed_handlers;
>  	struct tmp_objdir *t;
> +	int saved_errno;
>  
>  	if (the_tmp_objdir)
>  		BUG("only one tmp_objdir can be used at a time");

This could've been a local variable in each of the two blocks, but okay.

> @@ -185,8 +189,10 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
>  		chdir_notify_register(tmp_objdir_reparent, t);
>  
>  	if (!mkdtemp(t->path.buf)) {
> +		saved_errno = errno;
>  		/* free, not destroy, as we never touched the filesystem */
>  		tmp_objdir_free(t);
> +		errno = saved_errno;
>  		return NULL;
>  	}
>  
> @@ -197,7 +203,9 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
>  	}
>  
>  	if (setup_tmp_objdir(t->path.buf)) {
> +		saved_errno = errno;
>  		tmp_objdir_destroy(t);
> +		errno = saved_errno;
>  		return NULL;
>  	}

Okay, both early returns are adapted in the same way. Makes sense.

Patrick

Re: [PATCH 3/4] tmp-objdir: preserve errno when directory creation fails

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 3600 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

> When `tmp_objdir_create()` fails to create the temporary object
> directory, it cleans up after itself before returning NULL. Depending on
> where the failure occurred, this cleanup may clobber `errno`, leaving
> callers without a way to reliably tell why creating the directory
> failed.
>
> In a subsequent commit, ODB transactions create their temporary object
> directory eagerly and report a more specific error regarding creation
> failure. Preserve `errno` across the cleanup so that callers can inspect
> it accordingly.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  tmp-objdir.c | 8 ++++++++
>  tmp-objdir.h | 3 ++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/tmp-objdir.c b/tmp-objdir.c
> index 5307717354..4b91d86787 100644
> --- a/tmp-objdir.c
> +++ b/tmp-objdir.c
> @@ -148,10 +148,13 @@ static int setup_tmp_objdir(const char *root)
>  {
>  	char *path;
>  	int ret = 0;
> +	int saved_errno;
>
>  	path = xstrfmt("%s/pack", root);
>  	ret = mkdir(path, 0777);
> +	saved_errno = errno;
>  	free(path);
> +	errno = saved_errno;
>

This and the below pattern of save+restore seems a bit finicky to me.
Could we instead do:

1. Move this function inline

modified   tmp-objdir.c
@@ -144,18 +144,6 @@ static void env_replace(struct strvec *env, const
char *key, const char *val)
 	strvec_pushf(env, "%s=%s", key, val);
 }

-static int setup_tmp_objdir(const char *root)
-{
-	char *path;
-	int ret = 0;
-
-	path = xstrfmt("%s/pack", root);
-	ret = mkdir(path, 0777);
-	free(path);
-
-	return ret;
-}
-
 struct tmp_objdir *tmp_objdir_create(struct repository *r,
 				     const char *prefix,
 				     int will_destroy)
@@ -163,6 +151,8 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
 	struct odb_source_files *files =
odb_source_files_downcast(r->objects->source);
 	static int installed_handlers;
 	struct tmp_objdir *t;
+	char *path;
+	int ret;

 	if (the_tmp_objdir)
 		BUG("only one tmp_objdir can be used at a time");
@@ -196,7 +186,11 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
 		installed_handlers++;
 	}

-	if (setup_tmp_objdir(t->path.buf)) {
+	path = xstrfmt("%s/pack", t->path.buf);
+	ret = mkdir(path, 0777);
+	free(path);
+
+	if (ret) {
 		tmp_objdir_destroy(t);
 		return NULL;
 	}

--8<---

2. Then we can only set the errno we actually care about and jump to
exit

modified   tmp-objdir.c
@@ -151,6 +151,7 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
 	struct odb_source_files *files =
odb_source_files_downcast(r->objects->source);
 	static int installed_handlers;
 	struct tmp_objdir *t;
+	int saved_errno;
 	char *path;
 	int ret;

@@ -175,9 +176,10 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
 		chdir_notify_register(tmp_objdir_reparent, t);

 	if (!mkdtemp(t->path.buf)) {
+		saved_errno = errno;
 		/* free, not destroy, as we never touched the filesystem */
 		tmp_objdir_free(t);
-		return NULL;
+		goto fail;
 	}

 	the_tmp_objdir = t;
@@ -191,8 +193,9 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
 	free(path);

 	if (ret) {
+		saved_errno = errno;
 		tmp_objdir_destroy(t);
-		return NULL;
+		goto fail;
 	}

 	env_append(&t->env, ALTERNATE_DB_ENVIRONMENT,
@@ -215,6 +218,10 @@ struct tmp_objdir *tmp_objdir_create(struct repository *r,
 	t->repo->disable_ref_updates = true;

 	return t;
+
+fail:
+	errno = saved_errno;
+	return NULL;
 }

--8<---

This avoids having to care about other syscalls which may be added in
the future and breaking our setup as done in the patch here.

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH 4/4] object-file: create ODB transaction temporary directory eagerly

[Justin Tobler]

By default, ODB transactions for the "files" backend create their
temporary object directory lazily once the first loose object is about
to be written, and only if batched fsync is enabled. Transactions
configured for git-receive-pack(1) are the exception as the directory is
needed to quarantine incoming objects up front. Lazily creating the
temporary directory in this manner is awkward for a couple of reasons:

  - Writing loose objects has to know about the transaction internals to
    trigger the lazy creation of the temporary object directory.

  - Whether objects are staged in a temporary object directory, and thus
    whether they become visible at once on commit, depends on the
    configured fsync method.

Create the temporary object directory eagerly for every transaction when
it is started instead. This also ensures that no object is written to
the main object database before the transaction is committed.

Note that committing a transaction migrates every written object into
the primary object directory even when batched fsync is disabled. This
adds some minor overhead to ODB transaction callers that previously only
used a temporary directory if batched fsync was enabled. The following
benchmark uses the default fsync configuration and adds 10000 files of 4
KiB each to a freshly initialized repository via git-add(1):

  Benchmark 1: git add . (rev = HEAD~)
    Time (mean ± σ):     895.0 ms ±  23.8 ms    [User: 545.3 ms, System: 340.7 ms]
    Range (min … max):   866.7 ms … 928.7 ms    20 runs

  Benchmark 2: git add . (rev = HEAD)
    Time (mean ± σ):     956.3 ms ±  26.1 ms    [User: 541.4 ms, System: 405.2 ms]
    Range (min … max):   923.9 ms … 993.6 ms    20 runs

  Summary
    git add . (rev = HEAD~) ran
      1.07 ± 0.04 times faster than git add . (rev = HEAD)

Typically large numbers of loose objects are not expected to be written
into an ODB transaction so the overhead here should be marginal.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 object-file.c         | 56 +++++++++----------------------------------
 object-file.h         |  1 -
 odb/source-loose.c    |  6 -----
 t/t0004-unwritable.sh | 19 +++++++--------
 4 files changed, 19 insertions(+), 63 deletions(-)

diff --git a/object-file.c b/object-file.c
index da189111bd..a9fb42705c 100644
--- a/object-file.c
+++ b/object-file.c
@@ -489,7 +489,6 @@ struct odb_transaction_files {
 
 	struct tmp_objdir *objdir;
 	struct transaction_packfile packfile;
-	const char *prefix;
 
 	struct strvec pack_names;
 	struct tempfile **pack_lockfiles;
@@ -497,27 +496,6 @@ struct odb_transaction_files {
 	size_t pack_lockfiles_alloc;
 };
 
-int odb_transaction_files_prepare(struct odb_transaction *base)
-{
-	struct odb_transaction_files *transaction =
-		container_of_or_null(base, struct odb_transaction_files, base);
-
-	/*
-	 * 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 odb_transaction_files_begin.
-	 */
-	if (!transaction || transaction->objdir)
-		return 0;
-
-	transaction->objdir = tmp_objdir_create(base->source->odb->repo, transaction->prefix, 0);
-	if (!transaction->objdir)
-		return error(_("unable to create temporary object directory"));
-
-	return 0;
-}
-
 void odb_transaction_files_fsync(struct odb_transaction *base,
 				 int fd, const char *filename)
 {
@@ -1548,13 +1526,10 @@ static int odb_transaction_files_env(struct odb_transaction *base,
 {
 	struct odb_transaction_files *transaction =
 		container_of(base, struct odb_transaction_files, base);
-	int ret;
 
-	ret = odb_transaction_files_prepare(&transaction->base);
-	if (!ret)
-		strvec_pushv(env, tmp_objdir_env(transaction->objdir));
+	strvec_pushv(env, tmp_objdir_env(transaction->objdir));
 
-	return ret;
+	return 0;
 }
 
 int odb_transaction_files_begin(struct odb_source *source,
@@ -1562,6 +1537,8 @@ int odb_transaction_files_begin(struct odb_source *source,
 				enum odb_transaction_flags flags)
 {
 	struct odb_transaction_files *transaction;
+	const char *prefix;
+	int ret;
 
 	transaction = xcalloc(1, sizeof(*transaction));
 	transaction->base.source = source;
@@ -1573,24 +1550,13 @@ int odb_transaction_files_begin(struct odb_source *source,
 	transaction->flags = flags;
 	strvec_init(&transaction->pack_names);
 
-	transaction->prefix = "transaction";
-	if (flags & ODB_TRANSACTION_RECEIVE) {
-		/*
-		 * ODB transactions for git-receive-pack(1) eagerly create a
-		 * temporary directory and use a different temporary directory
-		 * prefix.
-		 *
-		 * NEEDSWORK: This transaction flag is only used by the "files"
-		 * backend to special case temporary directory set up and
-		 * handling. Ideally transaction users should not have to care
-		 * though. To avoid this, we could eagerly create the temporary
-		 * directory and use the same prefix name for all transactions.
-		 */
-		transaction->prefix = "incoming";
-		if (odb_transaction_files_prepare(&transaction->base)) {
-			free(transaction);
-			return -1;
-		}
+	prefix = flags & ODB_TRANSACTION_RECEIVE ? "incoming" : "transaction";
+	transaction->objdir = tmp_objdir_create(source->odb->repo, prefix, 0);
+	if (!transaction->objdir) {
+		ret = error_errno(_("unable to create temporary object directory in %s"),
+				  source->path);
+		free(transaction);
+		return ret;
 	}
 
 	*out = &transaction->base;
diff --git a/object-file.h b/object-file.h
index f44758c4f8..9ad79bbde0 100644
--- a/object-file.h
+++ b/object-file.h
@@ -177,7 +177,6 @@ int odb_transaction_files_begin(struct odb_source *source,
 				struct odb_transaction **out,
 				enum odb_transaction_flags flags);
 
-int odb_transaction_files_prepare(struct odb_transaction *base);
 void odb_transaction_files_fsync(struct odb_transaction *base,
 				 int fd, const char *filename);
 
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 50cb388c35..9330ae94b5 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -769,9 +769,6 @@ static int write_loose_object(struct odb_source_loose *loose,
 	static struct strbuf tmp_file = STRBUF_INIT;
 	static struct strbuf filename = STRBUF_INIT;
 
-	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
-		odb_transaction_files_prepare(loose->base.odb->transaction);
-
 	odb_loose_path(loose, &filename, oid);
 
 	fd = start_loose_object_common(loose, &tmp_file, filename.buf, flags,
@@ -860,9 +857,6 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,
 	char hdr[MAX_HEADER_LEN];
 	int hdrlen;
 
-	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
-		odb_transaction_files_prepare(loose->base.odb->transaction);
-
 	/* Since oid is not determined, save tmp file to odb path. */
 	strbuf_addf(&filename, "%s/", loose->base.path);
 	hdrlen = format_object_header(hdr, sizeof(hdr), in_stream->type, in_stream->size);
diff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh
index 3bdafbae0f..7292672a66 100755
--- a/t/t0004-unwritable.sh
+++ b/t/t0004-unwritable.sh
@@ -24,8 +24,8 @@ test_expect_success POSIXPERM,SANITY 'write-tree should notice unwritable reposi
 test_lazy_prereq WRITE_TREE_OUT 'test -e "$TRASH_DIRECTORY"/out.write-tree'
 test_expect_success WRITE_TREE_OUT 'write-tree output on unwritable repository' '
 	cat >expect <<-\EOF &&
-	error: insufficient permission for adding an object to repository database .git/objects
-	fatal: git-write-tree: error building trees
+	error: unable to create temporary object directory in .git/objects: Permission denied
+	fatal: failed to start ODB transaction
 	EOF
 	test_cmp expect out.write-tree
 '
@@ -39,8 +39,8 @@ test_expect_success POSIXPERM,SANITY 'commit should notice unwritable repository
 test_lazy_prereq COMMIT_OUT 'test -e "$TRASH_DIRECTORY"/out.commit'
 test_expect_success COMMIT_OUT 'commit output on unwritable repository' '
 	cat >expect <<-\EOF &&
-	error: insufficient permission for adding an object to repository database .git/objects
-	error: Error building trees
+	error: unable to create temporary object directory in .git/objects: Permission denied
+	fatal: failed to start ODB transaction
 	EOF
 	test_cmp expect out.commit
 '
@@ -55,9 +55,8 @@ test_expect_success POSIXPERM,SANITY 'update-index should notice unwritable repo
 test_lazy_prereq UPDATE_INDEX_OUT 'test -e "$TRASH_DIRECTORY"/out.update-index'
 test_expect_success UPDATE_INDEX_OUT 'update-index output on unwritable repository' '
 	cat >expect <<-\EOF &&
-	error: insufficient permission for adding an object to repository database .git/objects
-	error: file: failed to insert into database
-	fatal: Unable to process path file
+	error: unable to create temporary object directory in .git/objects: Permission denied
+	fatal: failed to start ODB transaction
 	EOF
 	test_cmp expect out.update-index
 '
@@ -72,10 +71,8 @@ test_expect_success POSIXPERM,SANITY 'add should notice unwritable repository' '
 test_lazy_prereq ADD_OUT 'test -e "$TRASH_DIRECTORY"/out.add'
 test_expect_success ADD_OUT 'add output on unwritable repository' '
 	cat >expect <<-\EOF &&
-	error: insufficient permission for adding an object to repository database .git/objects
-	error: file: failed to insert into database
-	error: unable to index file '\''file'\''
-	fatal: updating files failed
+	error: unable to create temporary object directory in .git/objects: Permission denied
+	fatal: failed to start ODB transaction
 	EOF
 	test_cmp expect out.add
 '
-- 
2.56.0.173.gf57c9a7388

Re: [PATCH 4/4] object-file: create ODB transaction temporary directory eagerly

[Patrick Steinhardt]

On Mon, Nov 09, 2026 at 06:22:51PM -0600, Justin Tobler wrote:
> By default, ODB transactions for the "files" backend create their
> temporary object directory lazily once the first loose object is about
> to be written, and only if batched fsync is enabled. Transactions
> configured for git-receive-pack(1) are the exception as the directory is
> needed to quarantine incoming objects up front. Lazily creating the
> temporary directory in this manner is awkward for a couple of reasons:
> 
>   - Writing loose objects has to know about the transaction internals to
>     trigger the lazy creation of the temporary object directory.
> 
>   - Whether objects are staged in a temporary object directory, and thus
>     whether they become visible at once on commit, depends on the
>     configured fsync method.

And three: the whole premise of creating ad-hoc transactions behind the
caller's back for the purpose of bulk-flushing writes to disk is
somewhat flawed in the first place. In a perfect world I'd think that
callers would manually manage their object transactions in all cases
where they know to write multiple objects at once.

But we don't live in that world (yet), and this step here brings us a
bit closer to it.

> diff --git a/object-file.c b/object-file.c
> index da189111bd..a9fb42705c 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1573,24 +1550,13 @@ int odb_transaction_files_begin(struct odb_source *source,
>  	transaction->flags = flags;
>  	strvec_init(&transaction->pack_names);
>  
> -	transaction->prefix = "transaction";
> -	if (flags & ODB_TRANSACTION_RECEIVE) {
> -		/*
> -		 * ODB transactions for git-receive-pack(1) eagerly create a
> -		 * temporary directory and use a different temporary directory
> -		 * prefix.
> -		 *
> -		 * NEEDSWORK: This transaction flag is only used by the "files"
> -		 * backend to special case temporary directory set up and
> -		 * handling. Ideally transaction users should not have to care
> -		 * though. To avoid this, we could eagerly create the temporary
> -		 * directory and use the same prefix name for all transactions.
> -		 */
> -		transaction->prefix = "incoming";
> -		if (odb_transaction_files_prepare(&transaction->base)) {
> -			free(transaction);
> -			return -1;
> -		}
> +	prefix = flags & ODB_TRANSACTION_RECEIVE ? "incoming" : "transaction";

Makes me wonder whether it's important to keep the "incoming" prefix
here or whether there is an argument to be made that it should simply be
called "transaction", as well.

That may be out of scope for this series though.

> +	transaction->objdir = tmp_objdir_create(source->odb->repo, prefix, 0);
> +	if (!transaction->objdir) {
> +		ret = error_errno(_("unable to create temporary object directory in %s"),
> +				  source->path);
> +		free(transaction);
> +		return ret;
>  	}
>  
>  	*out = &transaction->base;

Nice, this logic becomes quite a bit simpler.

> diff --git a/odb/source-loose.c b/odb/source-loose.c
> index 50cb388c35..9330ae94b5 100644
> --- a/odb/source-loose.c
> +++ b/odb/source-loose.c
> @@ -769,9 +769,6 @@ static int write_loose_object(struct odb_source_loose *loose,
>  	static struct strbuf tmp_file = STRBUF_INIT;
>  	static struct strbuf filename = STRBUF_INIT;
>  
> -	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
> -		odb_transaction_files_prepare(loose->base.odb->transaction);
> -
>  	odb_loose_path(loose, &filename, oid);
>  
>  	fd = start_loose_object_common(loose, &tmp_file, filename.buf, flags,
> @@ -860,9 +857,6 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,
>  	char hdr[MAX_HEADER_LEN];
>  	int hdrlen;
>  
> -	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
> -		odb_transaction_files_prepare(loose->base.odb->transaction);
> -
>  	/* Since oid is not determined, save tmp file to odb path. */
>  	strbuf_addf(&filename, "%s/", loose->base.path);
>  	hdrlen = format_object_header(hdr, sizeof(hdr), in_stream->type, in_stream->size);

Nice to see that we can get rid of all of this leaking abstraction.

> diff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh
> index 3bdafbae0f..7292672a66 100755
> --- a/t/t0004-unwritable.sh
> +++ b/t/t0004-unwritable.sh
> @@ -24,8 +24,8 @@ test_expect_success POSIXPERM,SANITY 'write-tree should notice unwritable reposi
>  test_lazy_prereq WRITE_TREE_OUT 'test -e "$TRASH_DIRECTORY"/out.write-tree'
>  test_expect_success WRITE_TREE_OUT 'write-tree output on unwritable repository' '
>  	cat >expect <<-\EOF &&
> -	error: insufficient permission for adding an object to repository database .git/objects
> -	fatal: git-write-tree: error building trees
> +	error: unable to create temporary object directory in .git/objects: Permission denied
> +	fatal: failed to start ODB transaction

Not a new issue, but this second error message is awful. An average user
won't have any idea what an "ODB" even is. s/ODB/object/ would make a
lot more sense.

Patrick

Re: [PATCH 4/4] object-file: create ODB transaction temporary directory eagerly

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

> By default, ODB transactions for the "files" backend create their
> temporary object directory lazily once the first loose object is about
> to be written, and only if batched fsync is enabled. Transactions
> configured for git-receive-pack(1) are the exception as the directory is
> needed to quarantine incoming objects up front. Lazily creating the
> temporary directory in this manner is awkward for a couple of reasons:
>
>   - Writing loose objects has to know about the transaction internals to
>     trigger the lazy creation of the temporary object directory.
>
>   - Whether objects are staged in a temporary object directory, and thus
>     whether they become visible at once on commit, depends on the
>     configured fsync method.
>
> Create the temporary object directory eagerly for every transaction when
> it is started instead. This also ensures that no object is written to
> the main object database before the transaction is committed.
>
> Note that committing a transaction migrates every written object into
> the primary object directory even when batched fsync is disabled. This
> adds some minor overhead to ODB transaction callers that previously only
> used a temporary directory if batched fsync was enabled. The following
> benchmark uses the default fsync configuration and adds 10000 files of 4
> KiB each to a freshly initialized repository via git-add(1):
>
>   Benchmark 1: git add . (rev = HEAD~)
>     Time (mean ± σ):     895.0 ms ±  23.8 ms    [User: 545.3 ms, System: 340.7 ms]
>     Range (min … max):   866.7 ms … 928.7 ms    20 runs
>
>   Benchmark 2: git add . (rev = HEAD)
>     Time (mean ± σ):     956.3 ms ±  26.1 ms    [User: 541.4 ms, System: 405.2 ms]
>     Range (min … max):   923.9 ms … 993.6 ms    20 runs
>
>   Summary
>     git add . (rev = HEAD~) ran
>       1.07 ± 0.04 times faster than git add . (rev = HEAD)
>

The explanation hre is well written and nicely explains what the cover
letter didn't _cover_. Nice.

> Typically large numbers of loose objects are not expected to be written
> into an ODB transaction so the overhead here should be marginal.
>

Question: why is that?

> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  object-file.c         | 56 +++++++++----------------------------------
>  object-file.h         |  1 -
>  odb/source-loose.c    |  6 -----
>  t/t0004-unwritable.sh | 19 +++++++--------
>  4 files changed, 19 insertions(+), 63 deletions(-)

The changes itself looks good.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 0/4] object-file: eagerly create ODB transaction temporary directory

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

> Greetings,
>
> This series continues work for the "files" ODB transactions and makes it
> so the underlying transaction temporary directory is created eagerly.

Without knowing enough, my first question would be 'why'. Over the top,
'lazily' seems to be a good way to go about this. Hopefully the
individual commits explain this.

> Previously, by default, it would wait for the first object write to
> lazily create the temporary object directory. Furthermore, this series
> also ensures that objects are written to the temporary directory for all
> transactional object writes, not just when batched fsync is configured.
>
> The first three patches consist of some minor preparatory refactors. The
> final patch swaps to creating the temporary directory eagerly and drops
> the lazy setup.
>
> This series is based on 3c60ef1e38 (The 17th batch, 2026-11-08) with
> jt/tmp-objdir-loose-idx-merge merged into it.
>
> Thanks,
> -Justin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]