Re: [PATCH 2/2] object-file: flush transaction packfile before migrating objects
From: Karthik Nayak <hidden>
Date: 2026-09-15 04:46:22
Justin Tobler [off-list ref] writes:
A "files" ODB transaction creates a temporary directory to stage newly written objects in when configured to batch fsync loose objects. Once the temporary directory is created, it is configured as the primary ODB and all object are written to it accordingly. This also includes packfiles containing blobs that exceed `core.bigFileThreshold` written via `odb_transaction_files_write_object_stream()`. If a "large" blob packfile is written to the ODB transaction temporary directory after other loose objects, the ODB transaction fails to commit as a result of the temporary directory being migrated prior to the packfile being flushed. Fix this bug by always flushing the packfile transaction before objects are migrated to the main ODB.
Okay this makes sense.
quoted hunk ↗ jump to hunk
Signed-off-by: Justin Tobler <redacted> --- object-file.c | 4 ++-- t/t1050-large.sh | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-)diff --git a/object-file.c b/object-file.c index 0f123b79fad1..210984f82532 100644 --- a/object-file.c +++ b/object-file.c@@ -1262,6 +1262,8 @@ static int odb_transaction_files_commit(struct odb_transaction *base) container_of(base, struct odb_transaction_files, base); int have_packfile = !!transaction->packfile.f; + flush_packfile_transaction(transaction); + if (transaction->objdir) { struct strbuf temp_path = STRBUF_INIT; struct tempfile *temp;@@ -1292,8 +1294,6 @@ static int odb_transaction_files_commit(struct odb_transaction *base) transaction->objdir = NULL; } - flush_packfile_transaction(transaction); - if (have_packfile) odb_reprepare(transaction->base.source->odb);diff --git a/t/t1050-large.sh b/t/t1050-large.sh index d295c265c75c..fb83c8fba619 100755 --- a/t/t1050-large.sh +++ b/t/t1050-large.sh@@ -87,6 +87,22 @@ test_expect_success 'add a large file or two' ' test $count = 1 ' +test_expect_success 'add large file with loose object in batch fsync' ' + test_when_finished "rm -rf batch" && + git init batch && + + git -C batch config core.bigFileThreshold 5 &&
Nit: we have `test_config` which automatically unsets after the test. Perhaps not really needed here, as we drop 'batch' anyways.
+ echo foo >batch/1-small && + echo foobar >batch/2-large && + + git -C batch -c core.fsync=loose-object -c core.fsyncMethod=batch \ + add 1-small 2-large && + + # Neither object may be left behind in a temporary location. + git -C batch cat-file -e :1-small && + git -C batch cat-file -e :2-large +'
Looks good.
+ test_expect_success 'checkout a large file' ' large1=$(git rev-parse :large1) && git update-index --add --cacheinfo 100644 $large1 another && -- 2.55.0
Attachments
- signature.asc [application/pgp-signature] 690 bytes