Re: [PATCH] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

From: Karthik Nayak <karthik.188@gmail.com>
Date: Feb 11, 2025, 06:09 UTC
Message-ID: <CAOLa=ZTGTr_3Xe2mLZ9wYB6oo++TuLUN-OQ2Cpv9Fokgc77UcQ@mail.gmail.com>
In-Reply-To: <llz32al6tgghs73odbryc2324vks2pf3uyawp5dls5wgop7or2@bumahbjibtgz>

Justin Tobler <jltobler@gmail.com> writes:

Show 7 quoted lines

> On 25/02/07 12:57PM, Karthik Nayak wrote:
>> The 'git-refs(1)' migrate subcommand, which transfers repositories
>> between reference backends, currently migrates reflogs by default as of
>> In 246cebe320 (refs: add support for migrating reflogs, 2024-12-16).
>
> s/In 246cebe320/246cebe320/
>

Thanks.

Show 14 quoted lines

>> While this behavior is desirable for most client-side repositories,
>> server-side repositories typically don't use reflogs and the migration
>> of these entries is unnecessary overhead.
>>
>> Add a '--skip-reflog' flag to the migrate subcommand to make reflog
>> migration optional. This is particularly useful for server-side
>> migrations where reflogs are not needed, improving migration performance
>> in these scenarios.
>
> Just to clarify, does a repository already without reflogs see improved
> migration performance with this `--skip-reflog` flag? Or is the improved
> performance soley due to repositories with reflogs skipping that part of
> the migration?
>

Since we iterate over all reflogs and add them, the perf gain would only be for repositories which already have reflogs. Will modify accordingly.

Show 44 quoted lines

>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> ---
>>  builtin/refs.c          |  3 +++
>>  refs.c                  |  8 +++++---
>>  refs.h                  |  5 ++++-
>>  t/t1460-refs-migrate.sh | 17 +++++++++++++++--
>>  4 files changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/refs.c b/builtin/refs.c
>> index a29f19583474518ee0942ea53c39cbdf9661c5e2..30be0254c14dd3d07693d70c25dddc9990756e9c 100644
>> --- a/builtin/refs.c
>> +++ b/builtin/refs.c
>> @@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
>>  		OPT_BIT(0, "dry-run", &flags,
>>  			N_("perform a non-destructive dry-run"),
>>  			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
>> +		OPT_BIT(0, "skip-reflog", &flags,
>> +			N_("skip migrating reflogs"),
>> +			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
>>  		OPT_END(),
>>  	};
>>  	struct strbuf errbuf = STRBUF_INIT;
>> diff --git a/refs.c b/refs.c
>> index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
>>  	if (ret < 0)
>>  		goto done;
>>
>> -	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
>> -	if (ret < 0)
>> -		goto done;
>> +	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
>> +		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
>> +		if (ret < 0)
>> +			goto done;
>> +	}
>
> When the `REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG` flag is set, we
> now skip over all the logs to perform the reflog migration. Makes sense.
>

Yup, thanks for the review.

> -Justin
>