* [PATCH 1/2] odb/source-loose: extract logic to retrieve subdirectory bitmap
2026-11-04 12:16 [PATCH 0/2] odb/source-loose: fix failing existence checks caused by stale cache Patrick Steinhardt
@ 2026-11-04 12:16 ` Patrick Steinhardt
2026-11-04 12:16 ` [PATCH 2/2] odb/source-loose: fix existence checks for newly written objects Patrick Steinhardt
2026-11-04 12:57 ` [PATCH 0/2] odb/source-loose: fix failing existence checks caused by stale cache Kristofer Karlsson
2 siblings, 0 replies; 6+ messages in thread
From: Patrick Steinhardt @ 2026-11-04 12:16 UTC (permalink / raw)
To: git; +Cc: Patrick Ohly
The loose object cache tracks which sharding directories have already
been scanned via a bitmap. The logic to retrieve the bitmap and mask for
a given object ID is inlined into the function that populates the cache,
but we're about to add a second function that will need to perform the
same logic.
Extract the logic into a separate function to prepare for this.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-loose.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 50cb388c352..da8f9767257 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -26,29 +26,39 @@ static int append_loose_object(const struct object_id *oid,
return 0;
}
-static struct oidtree *odb_source_loose_cache(struct odb_source_loose *loose,
- const struct object_id *oid)
+static uint32_t *subdir_seen_bitmap(struct odb_source_loose *loose,
+ const struct object_id *oid,
+ uint32_t *mask)
{
int subdir_nr = oid->hash[0];
- struct strbuf buf = STRBUF_INIT;
size_t word_bits = bitsizeof(loose->subdir_seen[0]);
size_t word_index = subdir_nr / word_bits;
- size_t mask = (size_t)1u << (subdir_nr % word_bits);
- uint32_t *bitmap;
if (subdir_nr < 0 ||
(size_t) subdir_nr >= bitsizeof(loose->subdir_seen))
BUG("subdir_nr out of range");
- bitmap = &loose->subdir_seen[word_index];
+ *mask = (size_t)1u << (subdir_nr % word_bits);
+
+ return &loose->subdir_seen[word_index];
+}
+
+static struct oidtree *odb_source_loose_cache(struct odb_source_loose *loose,
+ const struct object_id *oid)
+{
+ struct strbuf buf = STRBUF_INIT;
+ uint32_t *bitmap, mask;
+
+ bitmap = subdir_seen_bitmap(loose, oid, &mask);
if (*bitmap & mask)
return loose->cache;
+
if (!loose->cache) {
ALLOC_ARRAY(loose->cache, 1);
oidtree_init(loose->cache);
}
strbuf_addstr(&buf, loose->base.path);
- for_each_file_in_obj_subdir(subdir_nr, &buf,
+ for_each_file_in_obj_subdir(oid->hash[0], &buf,
loose->base.odb->repo->hash_algo,
append_loose_object,
NULL, NULL,
--
2.56.0.500.g5d06dc02cd.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] odb/source-loose: fix existence checks for newly written objects
2026-11-04 12:16 [PATCH 0/2] odb/source-loose: fix failing existence checks caused by stale cache Patrick Steinhardt
2026-11-04 12:16 ` [PATCH 1/2] odb/source-loose: extract logic to retrieve subdirectory bitmap Patrick Steinhardt
@ 2026-11-04 12:16 ` Patrick Steinhardt
2026-11-04 17:47 ` Junio C Hamano
2026-11-04 12:57 ` [PATCH 0/2] odb/source-loose: fix failing existence checks caused by stale cache Kristofer Karlsson
2 siblings, 1 reply; 6+ messages in thread
From: Patrick Steinhardt @ 2026-11-04 12:16 UTC (permalink / raw)
To: git; +Cc: Patrick Ohly
The loose object source maintains a cache of object IDs that it uses to
answer existence checks fast. This cache is lazily populated: whenever
we are asked about a specific object ID, we scan the whole sharding
directory it would live in and populate the cache with the objects we
find. Subsequent existence checks are then answered from the cache
without hitting the filesystem.
But when writing a new loose object we don't update that cache. That's
fine in the case where the cache for its sharding directory has not yet
been populated, because we would then populate it on a subsequent read
and thus surface the newly written object. But when it _is_ populated
already, then any subsequent existence check will claim that the object
does not exist at all until the cache is flushed.
This issue can cause git-merge(1) to die in a partial clone because we
try to fetch an object that we have just written, as reported in [1]:
1. `prefetch_for_content_merges()` performs quick existence checks for
all blobs that need a content merge to figure out which ones need
to be fetched from the promisor remote. These lookups populate the
loose object cache for the respective sharding directories.
2. We lazily fetch the missing blobs, which writes a new packfile.
3. We perform the merge and write the resulting blobs as loose
objects. The cache is not updated, so for all blobs whose sharding
directory was populated in (1), the object existence checks will
fail.
4. When checking out the merge result, `prefetch_cache_entries()`
performs existence checks for all index entries that need to be
written to the worktree. But due to the stale cache, the merged
blobs are reported as missing, and consequently we try to fetch
them from the promisor remote. This fetch fails because the remote
will most likely not have them.
This issue bisects to 5946a564cdd (odb/source: make `read_object_info()`
function pluggable, 2026-03-05), because in that commit we stopped
flushing the loose object cache when we try to read a missing object.
That was done intentionally though: a failing read is only retried when
it is a non-QUICK read, whereas the cache only affects QUICK reads. So
flushing the cache for a non-QUICK read wouldn't ever cause us to find
that object.
So the logic described here only ever worked by chance, as the cache was
incoherent after writes even before that commit. It only used to work
because of the interleaving sequence of QUICK and non-QUICK reads: when
we read the lazily-fetched blobs in (3) via a non-QUICK read, the first
read fails because the new packfile has not yet been added to the
"packed" source. On the retry we then used to flush the loose object
cache, and that in turn means that the subsequent existence check in (4)
rescanned the sharding directories and found the merged blobs.
We could in theory fix this by flushing all the loose object caches
again, like we used to do before 5946a564cdd. But that wouldn't address
the root cause: the cache would still be stale unless somebody does a
non-QUICK read for a missing object. Instead, fix the bug by updating
the cache whenever we write a new loose object.
[1]: <yrjh1p92qc7w.fsf@pohly-mobl1.fritz.box>
Reported-by: Patrick Ohly <patrick.ohly@intel.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-loose.c | 30 +++++++++++++++++++++++++++---
t/t5616-partial-clone.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+), 3 deletions(-)
diff --git a/odb/source-loose.c b/odb/source-loose.c
index da8f9767257..bcbd29ce3a0 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -68,6 +68,19 @@ static struct oidtree *odb_source_loose_cache(struct odb_source_loose *loose,
return loose->cache;
}
+/*
+ * Mark an object as seen in the loose cache. Note that we only do this in case
+ * the particular subdirectory was scanned already. If it wasn't, then the next
+ * read would populate it anyway.
+ */
+static void loose_cache_mark_seen(struct odb_source_loose *loose,
+ const struct object_id *oid)
+{
+ uint32_t mask, *bitmap = subdir_seen_bitmap(loose, oid, &mask);
+ if (*bitmap & mask)
+ oidtree_insert(loose->cache, oid, NULL);
+}
+
static int quick_has_loose(struct odb_source_loose *loose,
const struct object_id *oid)
{
@@ -824,8 +837,13 @@ static int write_loose_object(struct odb_source_loose *loose,
warning_errno(_("failed utime() on %s"), tmp_file.buf);
}
- return finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
- FOF_SKIP_COLLISION_CHECK);
+ ret = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
+ FOF_SKIP_COLLISION_CHECK);
+ if (ret < 0)
+ return ret;
+
+ loose_cache_mark_seen(loose, oid);
+ return 0;
}
static int odb_source_loose_write_object(struct odb_source *source,
@@ -967,8 +985,14 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,
err = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
FOF_SKIP_COLLISION_CHECK);
- if (!err && compat)
+ if (err < 0)
+ goto cleanup;
+
+ loose_cache_mark_seen(loose, oid);
+
+ if (compat)
err = repo_add_loose_object_map(loose, oid, &compat_oid);
+
cleanup:
strbuf_release(&tmp_file);
strbuf_release(&filename);
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 7cc6f58a31f..9636652362f 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -747,6 +747,49 @@ test_expect_success 'after fetching descendants of non-promisor commits, gc work
git -C partial gc --prune=now
'
+write_many_mergeable_files () {
+ for i in $(test_seq 50)
+ do
+ {
+ echo "$1 $i" &&
+ test_seq 8 | sed "s/^/line /" &&
+ echo "$2 $i"
+ } >"$3/file-$i" || return 1
+ done
+}
+
+test_expect_success 'merge does not lazy-fetch locally written objects' '
+ test_when_finished "rm -rf server client merged" &&
+ git init server &&
+ (
+ cd server &&
+ git config set uploadpack.allowfilter 1 &&
+
+ write_many_mergeable_files top-A bottom-A . &&
+ git add . &&
+ git commit -m A &&
+
+ write_many_mergeable_files top-B bottom-A . &&
+ git commit -a -m B &&
+
+ git checkout -b C main~1 &&
+ write_many_mergeable_files top-A bottom-C . &&
+ echo C >only-in-C &&
+ git add . &&
+ git commit -m C
+ ) &&
+
+ git clone --filter=blob:none --branch=main "file://$(pwd)/server" client &&
+ git -C client merge origin/C &&
+
+ mkdir merged &&
+ write_many_mergeable_files top-B bottom-C merged &&
+ for m in merged/*
+ do
+ test_cmp "$m" client/"$(basename "$m")" || return 1
+ done &&
+ test_cmp server/only-in-C client/only-in-C
+'
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
--
2.56.0.500.g5d06dc02cd.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread