[PATCH 0/4] odb/files: reload alternates on retry again

[PATCH 0/4] odb/files: reload alternates on retry again

[Patrick Steinhardt]

Hi,

we recently noticed that we don't reload alternates anymore when an
object cannot be found. This used to be the case, but broke with
5946a564cd (odb/source: make `read_object_info()` function pluggable,
2026-03-05). And while it's a rather uncommon scenario that alternates
are getting rewritten while a Git process runs we managed to hit some
such scenarios in production systems.

In any case, this patch series fixes that. In practice, only the second
commit is really needed to restore functionality. I was a bit unhappy
about the performance impact of this change though, as fixing the logic
regressed us from 180ms when looking up 20k missing objects to 309ms. So
the last two patches do some optimizations that bring us down to 197ms.

The series is built on top of 47fdf2208c (The 7th batch, 2026-10-19)
with ps/odb-files-alternates-cleanups at f211a3f0b5 (odb/source: drop
`local` flag, 2026-10-15) merged into it.

Thanks!

Patrick

---
Patrick Steinhardt (4):
      odb/source-files: rename alternates-related functions
      odb/source-files: reload alternates on retry
      odb/source-files: avoid rechecking already-known alternates
      odb/source-files: only reload alternates when the file has changed

 odb/source-files.c  | 84 +++++++++++++++++++++++++++++++----------------------
 odb/source-files.h  |  4 +++
 t/t1006-cat-file.sh | 32 ++++++++++++++++++++
 3 files changed, 86 insertions(+), 34 deletions(-)


---
base-commit: c8e565497cff47f6b23a6b96bef6fa82afa11667
change-id: 20261014-pks-odb-files-reload-alternates-dfc6dc010122

[PATCH 1/4] odb/source-files: rename alternates-related functions

[Patrick Steinhardt]

The functions that prepare and add alternates in the "files" backend
were originally part of the object database layer, which explains why
they still have an "odb_" prefix. That prefix does not make much sense
anymore now that those are part of the "files" backend.

Drop the prefix.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 5b4277fbfd..d31da6e5d9 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -348,9 +348,9 @@ static int read_alternates(const char *object_dir, struct strvec *out)
 	return 0;
 }
 
-static void odb_add_alternate_recursively(struct odb_source_files *files,
-					  const char *path,
-					  int depth)
+static void add_alternate_recursively(struct odb_source_files *files,
+				      const char *path,
+				      int depth)
 {
 	struct odb_files_dir *alternate;
 	struct strvec alternates = STRVEC_INIT;
@@ -377,15 +377,15 @@ static void odb_add_alternate_recursively(struct odb_source_files *files,
 		      path);
 	} else {
 		for (size_t i = 0; i < alternates.nr; i++)
-			odb_add_alternate_recursively(files, alternates.v[i], depth + 1);
+			add_alternate_recursively(files, alternates.v[i], depth + 1);
 	}
 
  out:
 	strvec_clear(&alternates);
 }
 
-static void odb_prepare_alternates(struct odb_source_files *files,
-				   const char *alternate_db)
+static void prepare_alternates(struct odb_source_files *files,
+			       const char *alternate_db)
 {
 	struct strvec alternates = STRVEC_INIT;
 
@@ -393,7 +393,7 @@ static void odb_prepare_alternates(struct odb_source_files *files,
 	read_alternates(files->dirs->abspath, &alternates);
 
 	for (size_t i = 0; i < alternates.nr; i++)
-		odb_add_alternate_recursively(files, alternates.v[i], 0);
+		add_alternate_recursively(files, alternates.v[i], 0);
 
 	strvec_clear(&alternates);
 }
@@ -410,7 +410,7 @@ static void odb_source_files_prepare(struct odb_source *source,
 	 * for the lifetime of the process.
 	 */
 	if (flags & ODB_PREPARE_FLUSH_CACHES)
-		odb_prepare_alternates(files, NULL);
+		prepare_alternates(files, NULL);
 
 	for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
 		odb_source_prepare(&dir->loose->base, flags);
@@ -1299,7 +1299,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 	if (!is_absolute_path(object_dir))
 		chdir_notify_register(odb_source_files_reparent, files);
 
-	odb_prepare_alternates(files, alternates);
+	prepare_alternates(files, alternates);
 
 	free(object_dir);
 	free(alternates);

-- 
2.56.0.269.g68944adf51.dirty

[PATCH 2/4] odb/source-files: reload alternates on retry

[Patrick Steinhardt]

When an object ID cannot be found we retry reading it from our packfiles
after we have flushed the respective caches of the packed backends. But
this misses one scenario: it may happen that the object databases'
alternates get rewritten concurrently and all of its objects are moved
to such an alternate. If so, we won't be able to find any of the objects
anymore. This used to be working just fine, but with 5946a564cd
(odb/source: make `read_object_info()` function pluggable, 2026-03-05)
we have accidentally dropped this logic.

This scenario sounds somewhat unlikely, but we've actually been hitting
it in the wild: when running long-lived git-cat-file(1) processes that
we interactively query over many minutes, and that repository gets
forked concurrently, then we move all objects into an object pool, write
a new alternates file, and the git-cat-file(1) process will remain
broken for the rest of its lifetime.

Fix this bug by re-preparing alternates when the object in question
could not be found and then retrying the read. Note that this also
requires us to re-read loose objects, which is an operation that we
previously skipped on the retry. But we only need to do this for the
loose object directories that are actually new.

Note that the result is still incomplete though, as we only reload the
alternates of the local object directory. But alternates are added
recursively, so in theory we'd also have to check whether alternates of
any of the alternates have changed. But we never did that before the
regression, either, so for now we don't care about this edge case.

Furthermore, this change causes a performance regression when asking for
lots of missing objects. But that regression is basically in line with
the performance we had before this mechanism got broken. The following
benchmark demonstrates this when asking for 20k missing commits in an
empty repository with a single empty alternate:

  Benchmark 1: git cat-file --batch-check (rev = 5946a564cd~)
    Time (mean ± σ):     282.8 ms ±   2.9 ms    [User: 99.8 ms, System: 178.6 ms]
    Range (min … max):   279.7 ms … 288.3 ms    10 runs

  Benchmark 2: git cat-file --batch-check (rev = 5946a564cd)
    Time (mean ± σ):     180.0 ms ±   1.7 ms    [User: 55.9 ms, System: 120.3 ms]
    Range (min … max):   178.2 ms … 183.9 ms    16 runs

  Benchmark 3: git cat-file --batch-check (rev = HEAD~)
    Time (mean ± σ):     187.7 ms ±   3.7 ms    [User: 59.7 ms, System: 124.3 ms]
    Range (min … max):   182.2 ms … 193.7 ms    15 runs

  Benchmark 4: git cat-file --batch-check (rev = HEAD)
    Time (mean ± σ):     309.8 ms ±   5.3 ms    [User: 105.9 ms, System: 199.1 ms]
    Range (min … max):   302.5 ms … 316.0 ms    10 runs

  Summary
    git cat-file --batch-check (rev = 5946a564cd) ran
      1.04 ± 0.02 times faster than git cat-file --batch-check (rev = HEAD~)
      1.57 ± 0.02 times faster than git cat-file --batch-check (rev = 5946a564cd~)
      1.72 ± 0.03 times faster than git cat-file --batch-check (rev = HEAD)

There's no helping this though, with the regression fixed we will be
slower compared to before as we have to do more work. But the next
commits will improve the status quo a bit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c  | 12 ++++++++----
 t/t1006-cat-file.sh | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index d31da6e5d9..86f3fd1136 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -425,6 +425,7 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 							      struct strbuf *errmsg)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
+	struct odb_files_dir **tail = NULL;
 	enum odb_read_status status = ODB_READ_NOT_FOUND;
 	enum odb_read_status ret;
 	bool retry = false;
@@ -448,10 +449,11 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 			status = ret;
 	}
 
-	for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
-		if (retry)
-			break;
-
+	/*
+	 * When we have reprepared alternates then we only re-scan directories
+	 * that were newly added.
+	 */
+	for (struct odb_files_dir *dir = tail ? *tail : files->dirs; dir; dir = dir->next) {
 		ret = odb_source_read_object_info(&dir->loose->base, oid, oi, flags,
 						  status == ODB_READ_NOT_FOUND ? errmsg : NULL);
 		if (!ret)
@@ -461,6 +463,8 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 	}
 
 	if (!retry && !(flags & OBJECT_INFO_QUICK)) {
+		tail = files->dirs_tail;
+		prepare_alternates(files, NULL);
 		retry = true;
 		goto retry;
 	}
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 72fc8acc61..f2b24dd278 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -1481,4 +1481,36 @@ test_objects_filter "object:type=commit"
 test_objects_filter "object:type=tag"
 test_objects_filter "object:type=tree"
 
+test_expect_success PIPE 'concurrently written alternates are loaded' '
+	test_when_finished "rm -rf repo alternate stdin stdout" &&
+	git init repo &&
+	git init alternate &&
+	oid=$(git -C alternate hash-object -w --stdin </dev/null) &&
+	mkfifo stdin &&
+	mkfifo stdout &&
+	{ git -C repo cat-file --batch-check <stdin >stdout & } &&
+	(
+		exec 8>stdin &&
+		exec 9<stdout &&
+
+		# Read the object the first time, without having alternates
+		# wired up yet. The object should be reported as missing.
+		echo "$oid" >&8 &&
+		read line <&9 &&
+		echo "$line" >actual &&
+		echo "$oid missing" >expect &&
+		test_cmp expect actual &&
+
+		# Read the object the second time, this time with the alternate
+		# set up. The backend should reload alternates and thus find
+		# the object.
+		echo "$PWD/alternate/.git/objects" >repo/.git/objects/info/alternates &&
+		echo "$oid" >&8 &&
+		read line <&9 &&
+		echo "$line" >actual &&
+		echo "$oid blob 0" >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_done

-- 
2.56.0.269.g68944adf51.dirty

Re: [PATCH 2/4] odb/source-files: reload alternates on retry

[Christian Couder]

On Tue, Oct 20, 2026 at 11:01 AM Patrick Steinhardt <ps@pks.im> wrote:

> diff --git a/odb/source-files.c b/odb/source-files.c
> index d31da6e5d9..86f3fd1136 100644
> --- a/odb/source-files.c
> +++ b/odb/source-files.c
> @@ -425,6 +425,7 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
>                                                               struct strbuf *errmsg)
>  {
>         struct odb_source_files *files = odb_source_files_downcast(source);
> +       struct odb_files_dir **tail = NULL;
>         enum odb_read_status status = ODB_READ_NOT_FOUND;
>         enum odb_read_status ret;
>         bool retry = false;
> @@ -448,10 +449,11 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
>                         status = ret;
>         }
>
> -       for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
> -               if (retry)
> -                       break;
> -
> +       /*
> +        * When we have reprepared alternates then we only re-scan directories
> +        * that were newly added.
> +        */
> +       for (struct odb_files_dir *dir = tail ? *tail : files->dirs; dir; dir = dir->next) {
>                 ret = odb_source_read_object_info(&dir->loose->base, oid, oi, flags,
>                                                   status == ODB_READ_NOT_FOUND ? errmsg : NULL);
>                 if (!ret)
> @@ -461,6 +463,8 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
>         }
>
>         if (!retry && !(flags & OBJECT_INFO_QUICK)) {
> +               tail = files->dirs_tail;
> +               prepare_alternates(files, NULL);

This calls `read_alternates(files->dirs->abspath, ...)`, but when we
use a temporary primary directory, `files->dirs` is that temporary
primary directory. The prepare_alternates() call will then quietly
find nothing new because temporary primary directories have no
alternates.

Before 5946a564cd (odb/source: make `read_object_info()` function
pluggable, 2026-03-05), when there was a temporary primary directory,
reloading the alternates also found nothing new for the same reason
(no alternates).

So we could think that there is no behavior change, but in the
meantime there was bdee7b3013 (builtin/receive-pack: stage incoming
objects via ODB transactions, 2026-07-10) which made the quarantine
the temporary primary directory in receive-pack. Before that commit,
receive-pack only passed its quarantine to child processes through
environment variables, so its own lookups reloaded the repository's
real alternates.

I wonder if it would be better to always reload the repository's real
alternates, instead of `files->dirs`'s alternates. This way, objects
that moved to an alternate would still be found, not only during a
receive-pack like before 5946a564cd, but whenever a temporary primary
directory is used.

It looks like with your 4/4 patch this improvement would add no extra cost.

Then `files->dirs` should also likely be replaced by the repository's
own object directory in the following places inside
odb_files_dir_is_usable():

- in the hashmap_entry_init() call that seeds the duplicate map, and
- in strbuf_realpath() when we are supposed to compare against the
resolved path of the primary.

These are not reachable today when a temporary primary directory is
installed, as no alternates are read then. But they would become
reachable if we reload the repository's real alternates, instead of
`files->dirs`'s alternates.

>                 retry = true;
>                 goto retry;
>         }

[PATCH 3/4] odb/source-files: avoid rechecking already-known alternates

[Patrick Steinhardt]

When adding alternates we first check whether the alternate is usable
via `odb_files_dir_is_usable()`. This function performs three checks:

  - It verifies that the alternate is not the same as the primary object
    directory.

  - It verifies that the alternate exists on disk.

  - It verifies that the alternate isn't already known to us by looking
    it up in `dirs_by_path`.

The first two checks hit the filesystem, whereas the last one is a mere
hashmap lookup. But the way the code is currently arranged we perform
the hashmap lookup last, which may cause us to do a couple of syscalls
only to then notice that we already know the alternate anyway, which is
of course quite inefficient.

Furthermore, the first check has gotten more expensive over time. While
we traditionally stored the relative path to the primary object
directory, we nowadays store an absolute path. This change is overall a
sensible one as it brings it in line with alternate object directories,
for which we always used to store absolute paths. But it also results in
more calls to lstat(3p) when calling `strbuf_realpath()` to resolve the
path of the primary object directory, as that syscall is made once per
path component.

Reorder the logic so that we perform the hash lookup first for an easy
win. Using the same benchmark as in the preceding commit:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)
    Time (mean ± σ):     282.5 ms ±   2.9 ms    [User: 100.0 ms, System: 178.4 ms]
    Range (min … max):   279.8 ms … 289.1 ms    10 runs

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd)
    Time (mean ± σ):     181.1 ms ±   1.7 ms    [User: 53.9 ms, System: 123.4 ms]
    Range (min … max):   178.2 ms … 183.6 ms    16 runs

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)
    Time (mean ± σ):     309.7 ms ±   2.7 ms    [User: 108.0 ms, System: 197.4 ms]
    Range (min … max):   305.4 ms … 315.1 ms    10 runs

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
    Time (mean ± σ):     263.4 ms ±   3.6 ms    [User: 92.2 ms, System: 167.1 ms]
    Range (min … max):   254.8 ms … 268.2 ms    11 runs

  Summary
    ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) ran
      1.45 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
      1.56 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)
      1.71 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)

So we're faster than the original, non-broken state now, but still
significantly slower compared to the regressed commit where we didn't
reload alternates at all.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 86f3fd1136..917f5e9d25 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -228,16 +228,6 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	struct hashmap_entry key;
 	bool usable = false;
 
-	strbuf_realpath(&normalized_objdir, files->dirs->abspath, 1);
-
-	/* Detect cases where alternate disappeared */
-	if (!is_directory(path)) {
-		error(_("object directory %s does not exist; "
-			"check .git/objects/info/alternates"),
-		      path);
-		goto out;
-	}
-
 	/*
 	 * Prevent the common mistake of listing the same
 	 * thing twice, or object directory itself.
@@ -249,12 +239,21 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 		hashmap_add(&files->dirs_by_path, &files->dirs->by_path_entry);
 	}
 
+	hashmap_entry_init(&key, strihash(path));
+	if (hashmap_get(&files->dirs_by_path, &key, path))
+		goto out;
+
+	strbuf_realpath(&normalized_objdir, files->dirs->abspath, 1);
 	if (!odb_files_dir_paths_cmp(files, path, normalized_objdir.buf))
 		goto out;
 
-	hashmap_entry_init(&key, strihash(path));
-	if (hashmap_get(&files->dirs_by_path, &key, path))
+	/* Detect cases where alternate disappeared */
+	if (!is_directory(path)) {
+		error(_("object directory %s does not exist; "
+			"check .git/objects/info/alternates"),
+		      path);
 		goto out;
+	}
 
 	usable = true;
 

-- 
2.56.0.269.g68944adf51.dirty

[PATCH 4/4] odb/source-files: only reload alternates when the file has changed

[Patrick Steinhardt]

In the preceding commit we have fixed a regression where we didn't
re-read the alternates file anymore when we hit a missing object. But
while correct, this change significantly hampers performance, even
though we're roughly at the same level compared to before the
regression.

But the checks are unnecessarily expensive, as we always read and parse
the alternates file even in the case where it has not changed. This of
course translates into a lot of busywork for the common case, where the
"objects/info/alternates" is still the exact same.

Improve this by using a `struct stat_validity`. Like this we can easily
short-circuit reading the alternates file in case we can determine via a
stat(3p) call that the file hasn't changed. This significantly improves
performance:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)
    Time (mean ± σ):     280.7 ms ±   2.1 ms    [User: 95.7 ms, System: 181.0 ms]
    Range (min … max):   277.9 ms … 285.0 ms    10 runs

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd)
    Time (mean ± σ):     179.5 ms ±   1.7 ms    [User: 60.5 ms, System: 115.6 ms]
    Range (min … max):   177.6 ms … 183.2 ms    16 runs

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)
    Time (mean ± σ):     257.2 ms ±   3.5 ms    [User: 94.8 ms, System: 158.3 ms]
    Range (min … max):   252.6 ms … 262.1 ms    11 runs

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
    Time (mean ± σ):     197.4 ms ±   3.8 ms    [User: 66.9 ms, System: 126.7 ms]
    Range (min … max):   192.3 ms … 203.3 ms    14 runs

  Summary
    ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) ran
      1.10 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
      1.43 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)
      1.56 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)

We're significantly faster compared to the original state now and are
almost as fast as the broken state where we don't reload alternates at
all. This is probably good enough for now.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 31 ++++++++++++++++++++++---------
 odb/source-files.h |  4 ++++
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 917f5e9d25..8ac4f7f380 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -73,6 +73,7 @@ static void odb_source_files_free(struct odb_source *source)
 		files->dirs = next;
 	}
 	hashmap_clear(&files->dirs_by_path);
+	stat_validity_clear(&files->alternates_validity);
 
 	odb_source_release(&files->base);
 	free(files);
@@ -329,22 +330,33 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static int read_alternates(const char *object_dir, struct strvec *out)
+static void read_alternates(const char *object_dir, struct strvec *out,
+			    struct stat_validity *validity)
 {
 	struct strbuf buf = STRBUF_INIT;
 	char *path;
 
 	path = xstrfmt("%s/info/alternates", object_dir);
-	if (strbuf_read_file(&buf, path, 1024) < 0) {
-		warn_on_fopen_errors(path);
-		free(path);
-		return 0;
+	if (!validity || !stat_validity_check(validity, path)) {
+		int fd = open(path, O_RDONLY);
+		if (fd < 0) {
+			warn_on_fopen_errors(path);
+			if (validity)
+				stat_validity_clear(validity);
+		} else {
+			if (validity)
+				stat_validity_update(validity, fd);
+			if (strbuf_read(&buf, fd, 1024) < 0)
+				warning_errno(_("unable to read alternates file '%s'"),
+					      path);
+			else
+				parse_alternates(buf.buf, '\n', object_dir, out);
+			close(fd);
+		}
 	}
-	parse_alternates(buf.buf, '\n', object_dir, out);
 
 	strbuf_release(&buf);
 	free(path);
-	return 0;
 }
 
 static void add_alternate_recursively(struct odb_source_files *files,
@@ -370,7 +382,7 @@ static void add_alternate_recursively(struct odb_source_files *files,
 	hashmap_add(&files->dirs_by_path, &alternate->by_path_entry);
 
 	/* recursively add alternates */
-	read_alternates(alternate->abspath, &alternates);
+	read_alternates(alternate->abspath, &alternates, NULL);
 	if (alternates.nr && depth + 1 > 5) {
 		error(_("%s: ignoring alternate object stores, nesting too deep"),
 		      path);
@@ -389,7 +401,8 @@ static void prepare_alternates(struct odb_source_files *files,
 	struct strvec alternates = STRVEC_INIT;
 
 	parse_alternates(alternate_db, PATH_SEP, NULL, &alternates);
-	read_alternates(files->dirs->abspath, &alternates);
+	read_alternates(files->dirs->abspath, &alternates,
+			&files->alternates_validity);
 
 	for (size_t i = 0; i < alternates.nr; i++)
 		add_alternate_recursively(files, alternates.v[i], 0);
diff --git a/odb/source-files.h b/odb/source-files.h
index 837e890304..b0795cd384 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -2,6 +2,7 @@
 #define ODB_SOURCE_FILES_H
 
 #include "odb/source.h"
+#include "statinfo.h"
 
 struct odb_source_loose;
 struct odb_source_packed;
@@ -66,6 +67,9 @@ struct odb_source_files {
 	 * determined by "core.ignoreCase".
 	 */
 	int dirs_paths_icase;
+
+	/* Stat data of the "info/alternates" file. */
+	struct stat_validity alternates_validity;
 };
 
 /* Allocate and initialize a new object source. */

-- 
2.56.0.269.g68944adf51.dirty

Re: [PATCH 4/4] odb/source-files: only reload alternates when the file has changed

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> +	if (!validity || !stat_validity_check(validity, path)) {
> +		int fd = open(path, O_RDONLY);
> +		if (fd < 0) {
> +			warn_on_fopen_errors(path);
> +			if (validity)
> +				stat_validity_clear(validity);
> +		} else {
> +			if (validity)
> +				stat_validity_update(validity, fd);
> +			if (strbuf_read(&buf, fd, 1024) < 0)
> +				warning_errno(_("unable to read alternates file '%s'"),
> +					      path);
> +			else
> +				parse_alternates(buf.buf, '\n', object_dir, out);
> +			close(fd);

When we update validity and then fail to strbuf_read(), say due to a
transient I/O error or something that was caused by somebody else
mucking with our alternates list at the same time, we warn, but do
not roll back the validity update even though we failed to parse.  I
wonder if it is safer to run the validity update only after
successfully read the alternates file, and if the extra safety is
worth it?

Re: [PATCH 4/4] odb/source-files: only reload alternates when the file has changed

[Patrick Steinhardt]

On Tue, Oct 20, 2026 at 12:59:58PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > +	if (!validity || !stat_validity_check(validity, path)) {
> > +		int fd = open(path, O_RDONLY);
> > +		if (fd < 0) {
> > +			warn_on_fopen_errors(path);
> > +			if (validity)
> > +				stat_validity_clear(validity);
> > +		} else {
> > +			if (validity)
> > +				stat_validity_update(validity, fd);
> > +			if (strbuf_read(&buf, fd, 1024) < 0)
> > +				warning_errno(_("unable to read alternates file '%s'"),
> > +					      path);
> > +			else
> > +				parse_alternates(buf.buf, '\n', object_dir, out);
> > +			close(fd);
> 
> When we update validity and then fail to strbuf_read(), say due to a
> transient I/O error or something that was caused by somebody else
> mucking with our alternates list at the same time, we warn, but do
> not roll back the validity update even though we failed to parse.  I
> wonder if it is safer to run the validity update only after
> successfully read the alternates file, and if the extra safety is
> worth it?

Good question. I doubt that the extra safety would really be worth it in
the general case as it's rather unlikely that we can open the path but
then not read from it. But that doesn't feel like a good-enough argument
to be sloppy, because "unlikely" doesn't mean "impossible".

So we could do something like this instead:

	if (!validity || !stat_validity_check(validity, path)) {
		bool successful = false;
		int fd;

		fd = open(path, O_RDONLY);
		if (fd < 0) {
			warn_on_fopen_errors(path);
		} else if (strbuf_read(&buf, fd, 1024) < 0) {
			warning_errno(_("unable to read alternates file '%s'"),
				      path);
		} else {
			parse_alternates(buf.buf, '\n', object_dir, out);
			successful = true;
		}

		if (validity) {
			if (successful)
				stat_validity_update(validity, fd);
			else
				stat_validity_clear(validity);
		}

		if (fd >= 0)
			close(fd);
	}

I've queued that change locally for now.

Thanks!

Patrick