{"thread":{"id":"66767","subject":"[PATCH 0/2] odb/source-loose: fix failing existence checks caused by stale cache","startedAt":"2026-11-04T12:16:33Z","lastAt":"2026-11-05T00:09:11Z","messageCount":7,"participants":["Patrick Steinhardt","Kristofer Karlsson","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"557428","messageId":"20261104-pks-odb-loose-stale-cache-v1-0-a00d5cdb5e7d@pks.im","threadId":"66767","inReplyTo":null,"subject":"[PATCH 0/2] odb/source-loose: fix failing existence checks caused by stale cache","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-04T12:16:33Z","receivedAt":"2026-11-04T12:16:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series fixes the bug reported in [1].\n\nIn summary, when git-merge(1) writes merged blobs it may be unable to\nread them back because the loose object cache does not get updated.\nConsequently, we may think that the blobs do not exist locally, and thus\nin a promisor remote we may perform a backfill fetch. But as the remote\ndoesn't know those objects we'll fail.\n\nThanks!\n\nPatrick\n\n[1]: yrjh1p92qc7w.fsf@pohly-mobl1.fritz.box\n\n---\nPatrick Steinhardt (2):\n odb/source-loose: extract logic to retrieve subdirectory bitmap\n odb/source-loose: fix existence checks for newly written objects\n\n odb/source-loose.c | 54 +++++++++++++++++++++++++++++++++++++++---------\n t/t5616-partial-clone.sh | 43 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 87 insertions(+), 10 deletions(-)\n\n\n---\nbase-commit: 750f5de7dfe9a4062f95088212b063f189d50e91\nchange-id: 20261104-pks-odb-loose-stale-cache-c16107987290\n\n\n"},{"id":"557429","messageId":"20261104-pks-odb-loose-stale-cache-v1-1-a00d5cdb5e7d@pks.im","threadId":"66767","inReplyTo":"20261104-pks-odb-loose-stale-cache-v1-0-a00d5cdb5e7d@pks.im","subject":"[PATCH 1/2] odb/source-loose: extract logic to retrieve subdirectory bitmap","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-04T12:16:34Z","receivedAt":"2026-11-04T12:16:34Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The loose object cache tracks which sharding directories have already\nbeen scanned via a bitmap. The logic to retrieve the bitmap and mask for\na given object ID is inlined into the function that populates the cache,\nbut we're about to add a second function that will need to perform the\nsame logic.\n\nExtract the logic into a separate function to prepare for this.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb/source-loose.c | 24 +++++++++++++++++-------\n 1 file changed, 17 insertions(+), 7 deletions(-)\n\ndiff --git a/odb/source-loose.c b/odb/source-loose.c\nindex 50cb388c352..da8f9767257 100644\n--- a/odb/source-loose.c\n+++ b/odb/source-loose.c\n@@ -26,29 +26,39 @@ static int append_loose_object(const struct object_id *oid,\n \treturn 0;\n }\n \n-static struct oidtree *odb_source_loose_cache(struct odb_source_loose *loose,\n-\t\t\t\t\t const struct object_id *oid)\n+static uint32_t *subdir_seen_bitmap(struct odb_source_loose *loose,\n+\t\t\t\t const struct object_id *oid,\n+\t\t\t\t uint32_t *mask)\n {\n \tint subdir_nr = oid->hash[0];\n-\tstruct strbuf buf = STRBUF_INIT;\n \tsize_t word_bits = bitsizeof(loose->subdir_seen[0]);\n \tsize_t word_index = subdir_nr / word_bits;\n-\tsize_t mask = (size_t)1u << (subdir_nr % word_bits);\n-\tuint32_t *bitmap;\n \n \tif (subdir_nr < 0 ||\n \t (size_t) subdir_nr >= bitsizeof(loose->subdir_seen))\n \t\tBUG(\"subdir_nr out of range\");\n \n-\tbitmap = &loose->subdir_seen[word_index];\n+\t*mask = (size_t)1u << (subdir_nr % word_bits);\n+\n+\treturn &loose->subdir_seen[word_index];\n+}\n+\n+static struct oidtree *odb_source_loose_cache(struct odb_source_loose *loose,\n+\t\t\t\t\t const struct object_id *oid)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tuint32_t *bitmap, mask;\n+\n+\tbitmap = subdir_seen_bitmap(loose, oid, &mask);\n \tif (*bitmap & mask)\n \t\treturn loose->cache;\n+\n \tif (!loose->cache) {\n \t\tALLOC_ARRAY(loose->cache, 1);\n \t\toidtree_init(loose->cache);\n \t}\n \tstrbuf_addstr(&buf, loose->base.path);\n-\tfor_each_file_in_obj_subdir(subdir_nr, &buf,\n+\tfor_each_file_in_obj_subdir(oid->hash[0], &buf,\n \t\t\t\t loose->base.odb->repo->hash_algo,\n \t\t\t\t append_loose_object,\n \t\t\t\t NULL, NULL,\n\n-- \n2.56.0.500.g5d06dc02cd.dirty\n\n\n"},{"id":"557430","messageId":"20261104-pks-odb-loose-stale-cache-v1-2-a00d5cdb5e7d@pks.im","threadId":"66767","inReplyTo":"20261104-pks-odb-loose-stale-cache-v1-0-a00d5cdb5e7d@pks.im","subject":"[PATCH 2/2] odb/source-loose: fix existence checks for newly written objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-04T12:16:35Z","receivedAt":"2026-11-04T12:16:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The loose object source maintains a cache of object IDs that it uses to\nanswer existence checks fast. This cache is lazily populated: whenever\nwe are asked about a specific object ID, we scan the whole sharding\ndirectory it would live in and populate the cache with the objects we\nfind. Subsequent existence checks are then answered from the cache\nwithout hitting the filesystem.\n\nBut when writing a new loose object we don't update that cache. That's\nfine in the case where the cache for its sharding directory has not yet\nbeen populated, because we would then populate it on a subsequent read\nand thus surface the newly written object. But when it _is_ populated\nalready, then any subsequent existence check will claim that the object\ndoes not exist at all until the cache is flushed.\n\nThis issue can cause git-merge(1) to die in a partial clone because we\ntry to fetch an object that we have just written, as reported in [1]:\n\n 1. `prefetch_for_content_merges()` performs quick existence checks for\n all blobs that need a content merge to figure out which ones need\n to be fetched from the promisor remote. These lookups populate the\n loose object cache for the respective sharding directories.\n\n 2. We lazily fetch the missing blobs, which writes a new packfile.\n\n 3. We perform the merge and write the resulting blobs as loose\n objects. The cache is not updated, so for all blobs whose sharding\n directory was populated in (1), the object existence checks will\n fail.\n\n 4. When checking out the merge result, `prefetch_cache_entries()`\n performs existence checks for all index entries that need to be\n written to the worktree. But due to the stale cache, the merged\n blobs are reported as missing, and consequently we try to fetch\n them from the promisor remote. This fetch fails because the remote\n will most likely not have them.\n\nThis issue bisects to 5946a564cdd (odb/source: make `read_object_info()`\nfunction pluggable, 2026-03-05), because in that commit we stopped\nflushing the loose object cache when we try to read a missing object.\nThat was done intentionally though: a failing read is only retried when\nit is a non-QUICK read, whereas the cache only affects QUICK reads. So\nflushing the cache for a non-QUICK read wouldn't ever cause us to find\nthat object.\n\nSo the logic described here only ever worked by chance, as the cache was\nincoherent after writes even before that commit. It only used to work\nbecause of the interleaving sequence of QUICK and non-QUICK reads: when\nwe read the lazily-fetched blobs in (3) via a non-QUICK read, the first\nread fails because the new packfile has not yet been added to the\n\"packed\" source. On the retry we then used to flush the loose object\ncache, and that in turn means that the subsequent existence check in (4)\nrescanned the sharding directories and found the merged blobs.\n\nWe could in theory fix this by flushing all the loose object caches\nagain, like we used to do before 5946a564cdd. But that wouldn't address\nthe root cause: the cache would still be stale unless somebody does a\nnon-QUICK read for a missing object. Instead, fix the bug by updating\nthe cache whenever we write a new loose object.\n\n[1]: \n\nReported-by: Patrick Ohly \nSigned-off-by: Patrick Steinhardt \n---\n odb/source-loose.c | 30 +++++++++++++++++++++++++++---\n t/t5616-partial-clone.sh | 43 +++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 70 insertions(+), 3 deletions(-)\n\ndiff --git a/odb/source-loose.c b/odb/source-loose.c\nindex da8f9767257..bcbd29ce3a0 100644\n--- a/odb/source-loose.c\n+++ b/odb/source-loose.c\n@@ -68,6 +68,19 @@ static struct oidtree *odb_source_loose_cache(struct odb_source_loose *loose,\n \treturn loose->cache;\n }\n \n+/*\n+ * Mark an object as seen in the loose cache. Note that we only do this in case\n+ * the particular subdirectory was scanned already. If it wasn't, then the next\n+ * read would populate it anyway.\n+ */\n+static void loose_cache_mark_seen(struct odb_source_loose *loose,\n+\t\t\t\t const struct object_id *oid)\n+{\n+\tuint32_t mask, *bitmap = subdir_seen_bitmap(loose, oid, &mask);\n+\tif (*bitmap & mask)\n+\t\toidtree_insert(loose->cache, oid, NULL);\n+}\n+\n static int quick_has_loose(struct odb_source_loose *loose,\n \t\t\t const struct object_id *oid)\n {\n@@ -824,8 +837,13 @@ static int write_loose_object(struct odb_source_loose *loose,\n \t\t\twarning_errno(_(\"failed utime() on %s\"), tmp_file.buf);\n \t}\n \n-\treturn finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,\n-\t\t\t\t\t FOF_SKIP_COLLISION_CHECK);\n+\tret = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,\n+\t\t\t\t\t FOF_SKIP_COLLISION_CHECK);\n+\tif (ret < 0)\n+\t\treturn ret;\n+\n+\tloose_cache_mark_seen(loose, oid);\n+\treturn 0;\n }\n \n static int odb_source_loose_write_object(struct odb_source *source,\n@@ -967,8 +985,14 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,\n \n \terr = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,\n \t\t\t\t\t FOF_SKIP_COLLISION_CHECK);\n-\tif (!err && compat)\n+\tif (err < 0)\n+\t\tgoto cleanup;\n+\n+\tloose_cache_mark_seen(loose, oid);\n+\n+\tif (compat)\n \t\terr = repo_add_loose_object_map(loose, oid, &compat_oid);\n+\n cleanup:\n \tstrbuf_release(&tmp_file);\n \tstrbuf_release(&filename);\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex 7cc6f58a31f..9636652362f 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -747,6 +747,49 @@ test_expect_success 'after fetching descendants of non-promisor commits, gc work\n \tgit -C partial gc --prune=now\n '\n \n+write_many_mergeable_files () {\n+\tfor i in $(test_seq 50)\n+\tdo\n+\t\t{\n+\t\t\techo \"$1 $i\" &&\n+\t\t\ttest_seq 8 | sed \"s/^/line /\" &&\n+\t\t\techo \"$2 $i\"\n+\t\t} >\"$3/file-$i\" || return 1\n+\tdone\n+}\n+\n+test_expect_success 'merge does not lazy-fetch locally written objects' '\n+\ttest_when_finished \"rm -rf server client merged\" &&\n+\tgit init server &&\n+\t(\n+\t\tcd server &&\n+\t\tgit config set uploadpack.allowfilter 1 &&\n+\n+\t\twrite_many_mergeable_files top-A bottom-A . &&\n+\t\tgit add . &&\n+\t\tgit commit -m A &&\n+\n+\t\twrite_many_mergeable_files top-B bottom-A . &&\n+\t\tgit commit -a -m B &&\n+\n+\t\tgit checkout -b C main~1 &&\n+\t\twrite_many_mergeable_files top-A bottom-C . &&\n+\t\techo C >only-in-C &&\n+\t\tgit add . &&\n+\t\tgit commit -m C\n+\t) &&\n+\n+\tgit clone --filter=blob:none --branch=main \"file://$(pwd)/server\" client &&\n+\tgit -C client merge origin/C &&\n+\n+\tmkdir merged &&\n+\twrite_many_mergeable_files top-B bottom-C merged &&\n+\tfor m in merged/*\n+\tdo\n+\t\ttest_cmp \"$m\" client/\"$(basename \"$m\")\" || return 1\n+\tdone &&\n+\ttest_cmp server/only-in-C client/only-in-C\n+'\n \n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n\n-- \n2.56.0.500.g5d06dc02cd.dirty\n\n\n"},{"id":"557435","messageId":"CAL71e4ONN4OQUZ2S==0TRWqZgA8ogT0smRL3Rk46voTm+0Tihw@mail.gmail.com","threadId":"66767","inReplyTo":"20261104-pks-odb-loose-stale-cache-v1-0-a00d5cdb5e7d@pks.im","subject":"Re: [PATCH 0/2] odb/source-loose: fix failing existence checks caused by stale cache","fromName":"Kristofer Karlsson","fromEmail":"krka@spotify.com","sentAt":"2026-11-04T12:57:52Z","receivedAt":"2026-11-04T12:57:52Z","isPatch":true,"sender":{"key":"krka@spotify.com","avatar":"https://avatars.githubusercontent.com/u/5349266?v=4"},"body":"On Wed, 4 Nov 2026 at 13:17, Patrick Steinhardt wrote:\n>\n> Hi,\n>\n> this small patch series fixes the bug reported in [1].\n>\n> In summary, when git-merge(1) writes merged blobs it may be unable to\n> read them back because the loose object cache does not get updated.\n> Consequently, we may think that the blobs do not exist locally, and thus\n> in a promisor remote we may perform a backfill fetch. But as the remote\n> doesn't know those objects we'll fail.\n>\n> Thanks!\n>\n> Patrick\n\nThe fix looks good to me, and I verified that the test fails\nwithout it. My own local fix is logically equivalent,\nwhich is a good sign, though perhaps that biased my review.\n\nThanks,\nKristofer\n\n"},{"id":"557460","messageId":"55e14977-dfc2-4acc-bc71-a91915fa251f@gmail.com","threadId":"66767","inReplyTo":"CAL71e4ONN4OQUZ2S==0TRWqZgA8ogT0smRL3Rk46voTm+0Tihw@mail.gmail.com","subject":"Re: [PATCH 0/2] odb/source-loose: fix failing existence checks caused by stale cache","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-11-04T16:27:52Z","receivedAt":"2026-11-04T16:27:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/4/2026 7:57 AM, Kristofer Karlsson wrote:\n> On Wed, 4 Nov 2026 at 13:17, Patrick Steinhardt wrote:\n>>\n>> Hi,\n>>\n>> this small patch series fixes the bug reported in [1].\n>>\n>> In summary, when git-merge(1) writes merged blobs it may be unable to\n>> read them back because the loose object cache does not get updated.\n>> Consequently, we may think that the blobs do not exist locally, and thus\n>> in a promisor remote we may perform a backfill fetch. But as the remote\n>> doesn't know those objects we'll fail.\n>>\n>> Thanks!\n>>\n>> Patrick\n> \n> The fix looks good to me, and I verified that the test fails\n> without it. My own local fix is logically equivalent,\n> which is a good sign, though perhaps that biased my review.\nI also think the fix looks good. (I haven't verified it locally\nlike Kristofer did, but the test is good.)\n\nThanks,\n-Stolee\n\n\n"},{"id":"557471","messageId":"xmqqfqxg32uy.fsf@gitster.g","threadId":"66767","inReplyTo":"20261104-pks-odb-loose-stale-cache-v1-2-a00d5cdb5e7d@pks.im","subject":"Re: [PATCH 2/2] odb/source-loose: fix existence checks for newly written objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-11-04T17:47:49Z","receivedAt":"2026-11-04T17:47:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> @@ -824,8 +837,13 @@ static int write_loose_object(struct odb_source_loose *loose,\n> \t\t\twarning_errno(_(\"failed utime() on %s\"), tmp_file.buf);\n> \t}\n> \n> -\treturn finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,\n> -\t\t\t\t\t FOF_SKIP_COLLISION_CHECK);\n> +\tret = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,\n> +\t\t\t\t\t FOF_SKIP_COLLISION_CHECK);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tloose_cache_mark_seen(loose, oid);\n> +\treturn 0;\n> }\n> \n> static int odb_source_loose_write_object(struct odb_source *source,\n> @@ -967,8 +985,14 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,\n> \n> \terr = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,\n> \t\t\t\t\t FOF_SKIP_COLLISION_CHECK);\n> -\tif (!err && compat)\n> +\tif (err < 0)\n> +\t\tgoto cleanup;\n> +\n> +\tloose_cache_mark_seen(loose, oid);\n> +\n> +\tif (compat)\n> \t\terr = repo_add_loose_object_map(loose, oid, &compat_oid);\n> +\n> cleanup:\n> \tstrbuf_release(&tmp_file);\n> \tstrbuf_release(&filename);\n\nNot about this change but the lack of \"if (compat)\" stuff in the\nprevious hunk puzzled me. It turns out that the logic to write a\nsingle loose object is so big that write_loose_object(), which is\nthe subject of the previous hunk, helps another function,\nodb_source_loose_write_object(), by being its helper function, and\nthis caller has the corresponding \"if (compat)\" logic to produce\nthe hash interoperability data.\n\nLooking very good.\n\n"},{"id":"557519","messageId":"xmqqqzh0xhp4.fsf@gitster.g","threadId":"66767","inReplyTo":"CAL71e4ONN4OQUZ2S==0TRWqZgA8ogT0smRL3Rk46voTm+0Tihw@mail.gmail.com","subject":"Re: [PATCH 0/2] odb/source-loose: fix failing existence checks caused by stale cache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-11-05T00:09:11Z","receivedAt":"2026-11-05T00:09:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristofer Karlsson writes:\n\n> On Wed, 4 Nov 2026 at 13:17, Patrick Steinhardt wrote:\n>>\n>> Hi,\n>>\n>> this small patch series fixes the bug reported in [1].\n>>\n>> In summary, when git-merge(1) writes merged blobs it may be unable to\n>> read them back because the loose object cache does not get updated.\n>> Consequently, we may think that the blobs do not exist locally, and thus\n>> in a promisor remote we may perform a backfill fetch. But as the remote\n>> doesn't know those objects we'll fail.\n>>\n>> Thanks!\n>>\n>> Patrick\n>\n> The fix looks good to me, and I verified that the test fails\n> without it. My own local fix is logically equivalent,\n> which is a good sign, though perhaps that biased my review.\n\nThanks, both.\n\n\n\n"}]}