Re: [PATCH 2/2] object-file: flush transaction packfile before migrating objects
Justin Tobler <jltobler@gmail.com> writes:
Show 12 quoted lines
> 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.
Show 42 quoted lines
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
> 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.
Show 11 quoted lines
> + 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
> +'
>
Show 6 quoted lines
> +
> test_expect_success 'checkout a large file' '
> large1=$(git rev-parse :large1) &&
> git update-index --add --cacheinfo 100644 $large1 another &&
> --
> 2.55.0
Karthik Nayak <karthik.188@gmail.com>