From 5a817d648bb96cf97e2530c2de4ea4ad7a9a81e9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 15 Oct 2026 07:27:57 +0200 Subject: [PATCH] odb/source-files: make the second read an implementation detail When looking up an object fails we perform a second read in all sources with the `OBJECT_INFO_SECOND_READ` flag set. This flag tells sources that the first lookup has failed and that they should reload any on-disk state that may have changed in the meantime so that the object may be surfaced after all. The "packed" source uses this to reprepare its packfiles, whereas the "loose" source short-circuits the second read because there are no caches to flush. This logic lives in the generic object database layer even though it is highly specific to how the "files" backend handles its reads. But now that alternates are an implementation detail of that source, we can easily move that logic into the backend, as well. Move the logic into the "files" source and drop the flag. Note that the "packed" source also used this flag to figure out whether it should try to surface an object via a packfile that should have the object as determined by a multi-pack index, but where the packfile has vanished. The multi-pack index is stale in that case, and that scenario should really only happen when the process is racing with a concurrent maintenance task. This code is adapted to instead use `OBJECT_INFO_QUICK`, which means that the fallback may trigger on the first read already, but not in a case where callers know to handle false negatives already. In the worst case this could cause another scan through all packfiles covered by the multi-pack index. But that scan should be fast enough anyway, and it only triggers when we hit the race. So the effect thereof should be limited. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 15 --------------- odb.h | 12 ------------ odb/source-files.c | 13 +++++++++++++ odb/source-loose.c | 10 ---------- odb/source-packed.c | 15 +++------------ odb/source.h | 7 ------- 6 files changed, 16 insertions(+), 56 deletions(-) diff --git a/odb.c b/odb.c index 98c5c862f5..69a4c203bc 100644 --- a/odb.c +++ b/odb.c @@ -285,21 +285,6 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database * if (ret != ODB_READ_NOT_FOUND) corrupt = true; - /* - * When the object hasn't been found we try a second read and - * tell the sources so. This may cause them to invalidate - * caches or reload on-disk state. - */ - if (!(flags & OBJECT_INFO_QUICK)) { - ret = odb_source_read_object_info(odb->source, real, oi, - flags | OBJECT_INFO_SECOND_READ, - corrupt_err.len ? NULL : &corrupt_err); - if (!ret) - goto out; - if (ret != ODB_READ_NOT_FOUND) - corrupt = true; - } - /* Check if it is a missing object */ if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) && !already_retried && diff --git a/odb.h b/odb.h index 0ccc47ee73..dd85b880a7 100644 --- a/odb.h +++ b/odb.h @@ -383,18 +383,6 @@ enum object_info_flags { /* Die if object corruption (not just an object being missing) was detected. */ OBJECT_INFO_DIE_IF_CORRUPT = (1 << 3), - /* - * We have already tried reading the object, but it couldn't be found - * via any of the attached sources, and are now doing a second read. - * This second read asks the individual sources to also evaluate - * whether any on-disk state may have changed that may have caused the - * object to appear. - * - * This flag is for internal use, only. The second read only occurs - * when `OBJECT_INFO_QUICK` was not passed. - */ - OBJECT_INFO_SECOND_READ = (1 << 4), - /* * This is meant for bulk prefetching of missing blobs in a partial * clone. Implies OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK. diff --git a/odb/source-files.c b/odb/source-files.c index 54235821d5..9788a5adf4 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -427,7 +427,9 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source struct odb_source_files *files = odb_source_files_downcast(source); enum odb_read_status status = ODB_READ_NOT_FOUND; enum odb_read_status ret; + bool retry = false; +retry: /* * Reading an object may fail even though the object exists, for * example because it is corrupt. Report this failure to the caller in @@ -435,6 +437,9 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source * prefer the first such error in case multiple reads have failed. */ for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) { + if (retry) + odb_source_prepare(&dir->packed->base, ODB_PREPARE_FLUSH_CACHES); + ret = odb_source_read_object_info(&dir->packed->base, oid, oi, flags, status == ODB_READ_NOT_FOUND ? errmsg : NULL); if (!ret) @@ -444,6 +449,9 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source } for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) { + if (retry) + break; + ret = odb_source_read_object_info(&dir->loose->base, oid, oi, flags, status == ODB_READ_NOT_FOUND ? errmsg : NULL); if (!ret) @@ -452,6 +460,11 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source status = ret; } + if (!retry && !(flags & OBJECT_INFO_QUICK)) { + retry = true; + goto retry; + } + return status; } diff --git a/odb/source-loose.c b/odb/source-loose.c index 0664634938..180ce7403f 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -222,16 +222,6 @@ static enum odb_read_status odb_source_loose_read_object_info(struct odb_source { struct odb_source_loose *loose = odb_source_loose_downcast(source); static struct strbuf buf = STRBUF_INIT; - - /* - * The second read shouldn't cause new loose objects to show up, unless - * there was a race condition with a secondary process. We don't care - * about this case though, so we simply skip reading loose objects a - * second time. - */ - if (flags & OBJECT_INFO_SECOND_READ) - return ODB_READ_NOT_FOUND; - odb_loose_path(loose, &buf, oid); return read_object_info_from_path(loose, buf.buf, oid, oi, flags, errmsg); } diff --git a/odb/source-packed.c b/odb/source-packed.c index 2226fbd313..48fe00c2bc 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -52,12 +52,11 @@ static int find_pack_entry(struct odb_source_packed *store, * packs, and repreparing the on-disk pack set does not reload the * borrowed, cached MIDX, so scan its packs directly for the survivor. * - * Do this only on the second read, by which point repreparing packs has - * already had a chance to find an object merely relocated into a new, - * uncovered pack; only a genuine hidden duplicate reaches here. + * Skip the scan for quick lookups, which are expected to accept false + * negatives in favor of speed. */ if (midx_result == MIDX_FILL_OWNER_UNAVAILABLE && - (flags & OBJECT_INFO_SECOND_READ)) { + !(flags & OBJECT_INFO_QUICK)) { struct multi_pack_index *m = store->midx; uint32_t i; @@ -86,14 +85,6 @@ static enum odb_read_status odb_source_packed_read_object_info(struct odb_source struct pack_entry e; int ret; - /* - * In case the first read didn't surface the object, we have to reload - * packfiles. This may cause us to discover new packfiles that have - * been added since the last time we have prepared the packfile store. - */ - if (flags & OBJECT_INFO_SECOND_READ) - odb_source_prepare(source, ODB_PREPARE_FLUSH_CACHES); - if (!find_pack_entry(packed, oid, &e, flags, &bad_pack)) { /* * The lookup may have failed because the object is known to be diff --git a/odb/source.h b/odb/source.h index 1867347dfc..0cfe762376 100644 --- a/odb/source.h +++ b/odb/source.h @@ -119,13 +119,6 @@ struct odb_source { * - `OBJECT_INFO_QUICK` indicates it is fine to use caches without * re-verifying the data. * - * - `OBJECT_INFO_SECOND_READ` indicates that the initial object - * lookup has failed and that the object sources should check - * whether any of its on-disk state has changed that may have - * caused the object to appear. Sources are free to ignore the - * second read in case they know that the first read would have - * already surfaced the object without reloading any on-disk state. - * * The callback is expected to return an `enum odb_read_status`. Please * refer to the individual values that can be returned. In case reading * the object has failed with a generic error and `errmsg` is non-NULL,