[PATCH] tmp-objdir: merge loose object index when migrating tmpdir
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Oct 22, 2026, 23:40 UTC
- Message-ID
- <20261022234007.3081825-1-jltobler@gmail.com>
When a repository is configured with a compatibility hash, each loose object write also records a mapping between the object and its compatibility hash in a "loose-object-idx" file. This index is located in the same object database the object is written to. If batch fsync of loose objects is also enabled, the loose objects and index file are instead written to a temporary object directory and expected to be migrated to the primary object directory when the batch transaction commits.
While migrating the loose objects works, the "loose-object-idx" file is unable to be migrated if an index already exists in the primary object directory. This occurs because `finalize_object_file_flags()` expects an already existing file to have the same contents. Otherwise, the following error is returned:
error: files '.git/objects/tmp_objdir-bulk-fsync-XXXXXX/loose-object-idx' and '.git/objects/loose-object-idx' differ in contents error: unable to migrate temporary objects
Furthermore, it is not sufficient to just replace the preexisting index with the new one as the new index only contains mapping entries for the newly written objects.
Update the temporary directory migration logic to handle "loose-object-idx" files explicitly by appending new mapping entries to the primary index.
Signed-off-by: Justin Tobler <jltobler@gmail.com> ---
Greetings,
While working on a separate patch series to convert the "files" ODB transaction to be eagerly created instead of lazily, I encountered this bug that causes an ODB transaction to fail on commit when a compatibility hash is also configured. This occurs because the temporary directory migration code does not know how to handle the loose object index file.
This patch addresses updating the migration logic to explicitly merge mapping entries in the transaction by appending them to the primary loose object index file.
Some other approaches I considered:
- Storing the uncommitted object mapping entries in the transaction
without writing a file and only writing on transaction commit. I
chose to keep writing the temp index into the transaction so that
child processes that write to the tempdir could still append entries.
- Also writing mapping entires to the main ODB loose object index.
This also didn't seem like a good apprach because it write entires
that may not ultimately get committed.Thanks for the review, -Justin
--- loose.c | 72 +++++++++++++++++++++++++++++++++++ loose.h | 2 + t/t1016-compatObjectFormat.sh | 27 +++++++++++++ tmp-objdir.c | 11 +++++- 4 files changed, 111 insertions(+), 1 deletion(-)
diff --git a/loose.c b/loose.c index d6a2bb5ba3..79d4c256e0 100644 --- a/loose.c +++ b/loose.c @@ -265,3 +265,75 @@ void loose_object_map_clear(struct loose_object_map **map) free(m); *map = NULL; } + +int loose_object_idx_merge(struct repository *repo, const char *src, + const char *dst) +{ + struct lock_file lock = LOCK_INIT; + struct strbuf buf = STRBUF_INIT; + struct stat st; + FILE *fp; + int fd = -1; + int ret; + + fp = fopen(src, "r"); + if (!fp) { + ret = error_errno(_("unable to open %s"), src); + goto out; + } + if (strbuf_getwholeline(&buf, fp, '\n') || strcmp(buf.buf, loose_object_header)) { + ret = error(_("invalid loose object index %s"), src); + goto out; + } + + if (repo_hold_lock_file_for_update_timeout(repo, &lock, dst, 0, -1) < 0) { + ret = error_errno(_("unable to lock %s"), dst); + goto out; + } + + fd = open(dst, O_WRONLY | O_CREAT | O_APPEND, 0666); + if (fd < 0) { + ret = error_errno(_("unable to open %s"), dst); + goto out; + } + if (fstat(fd, &st) < 0) { + ret = error_errno(_("unable to stat %s"), dst); + goto out; + } + + /* Create the loose-object-idx if one does not already exist. */ + if (!st.st_size && write_in_full(fd, loose_object_header, + strlen(loose_object_header)) < 0) { + ret = error_errno(_("failed to write loose object index header")); + goto out; + } + + while (!strbuf_getwholeline(&buf, fp, '\n')) { + if (write_in_full(fd, buf.buf, buf.len) < 0) { + ret = error_errno(_("failed to write loose object index %s"), + dst); + goto out; + } + } + + ret = close(fd); + fd = -1; + if (ret) { + ret = error_errno(_("unable to close %s"), dst); + goto out; + } + + adjust_shared_perm(repo, dst); + + ret = 0; + +out: + rollback_lock_file(&lock); + strbuf_release(&buf); + if (fd >= 0) + close(fd); + if (fp) + fclose(fp); + + return ret; +} diff --git a/loose.h b/loose.h index ed663ac550..35b46f4be5 100644 --- a/loose.h +++ b/loose.h @@ -23,5 +23,7 @@ int repo_add_loose_object_map(struct odb_source_loose *loose, const struct object_id *compat_oid); int repo_read_loose_object_map(struct repository *repo); int repo_write_loose_object_map(struct repository *repo); +int loose_object_idx_merge(struct repository *repo, const char *src, + const char *dst); #endif diff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh index 9cafcee509..bebf71bc29 100755 --- a/t/t1016-compatObjectFormat.sh +++ b/t/t1016-compatObjectFormat.sh @@ -305,4 +305,31 @@ compare_oids GPG2 'commit' signedcommit4 "$signedcommit4_sha1_oid" "$signedcommi compare_oids GPG2 'tag' signedtag3 "$signedtag3_sha1_oid" "$signedtag3_sha256_oid" compare_oids GPG2 'tag' signedtag4 "$signedtag4_sha1_oid" "$signedtag4_sha256_oid" +test_expect_success 'batched fsync migrates loose object index' ' + test_when_finished "rm -rf batch-fsync" && + git init --object-format=sha1 batch-fsync && + ( + cd batch-fsync && + git config core.repositoryFormatVersion 1 && + git config extensions.compatObjectFormat sha256 && + git config core.fsync loose-object && + git config core.fsyncMethod batch && + + # Write object to generate initial loose object index. + echo foo >foo && + git add foo && + test_grep "$(git rev-parse :foo)" .git/objects/loose-object-idx && + + # Write some additional objects and ensure the corresponding + # temporary loose object index entries stored in the transaction + # are merged into the primary ODB. + echo bar >bar && + echo baz >baz && + git add bar baz && + test_grep "$(git rev-parse :foo)" .git/objects/loose-object-idx && + test_grep "$(git rev-parse :bar)" .git/objects/loose-object-idx && + test_grep "$(git rev-parse :baz)" .git/objects/loose-object-idx + ) +' + test_done diff --git a/tmp-objdir.c b/tmp-objdir.c index 3debb60270..5307717354 100644 --- a/tmp-objdir.c +++ b/tmp-objdir.c @@ -4,6 +4,7 @@ #include "chdir-notify.h" #include "dir.h" #include "environment.h" +#include "loose.h" #include "object-file.h" #include "path.h" #include "string-list.h" @@ -294,6 +295,11 @@ static int is_loose_object_shard(const char *name) return strlen(name) == 2 && isxdigit(name[0]) && isxdigit(name[1]); } +static int is_loose_object_idx(const char *name) +{ + return !strcmp(name, "loose-object-idx"); +} + static int migrate_paths(struct tmp_objdir *t, struct strbuf *src, struct strbuf *dst, enum finalize_object_file_flags flags) @@ -317,7 +323,10 @@ static int migrate_paths(struct tmp_objdir *t, if (is_loose_object_shard(name)) flags_copy |= FOF_SKIP_COLLISION_CHECK; - ret |= migrate_one(t, src, dst, flags_copy); + if (is_loose_object_idx(name)) + ret |= loose_object_idx_merge(t->repo, src->buf, dst->buf); + else + ret |= migrate_one(t, src, dst, flags_copy); strbuf_setlen(src, src_len); strbuf_setlen(dst, dst_len); base-commit: 7e34d8174dcda3435d1404c9a30bef46afe6c13e
-- 2.56.0.173.gf57c9a7388