Re: [PATCH v2 3/5] backfill: add --min-batch-size=<n> option

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 16, 2025, 10:01 UTC
Message-ID: <Z4jY34vmK1yj0Mzi@pks.im>
In-Reply-To: <3fa32822dabb64266a0625c50530ae2ae0797de6.1734712193.git.gitgitgadget@gmail.com>

On Fri, Dec 20, 2024 at 04:29:51PM +0000, Derrick Stolee via GitGitGadget wrote:

Show 12 quoted lines

> From: Derrick Stolee <derrickstolee@github.com>
> 
> Users may want to specify a minimum batch size for their needs. This is only
> a minimum: the path-walk API provides a list of OIDs that correspond to the
> same path, and thus it is optimal to allow delta compression across those
> objects in a single server request.
> 
> We could consider limiting the request to have a maximum batch size in the
> future. For now, we let the path-walk API batches determine the
> boundaries.
> 
> To get a feeling for the value of specifying the --batch-size parameter,

This should say `--min-batch-size`.

Show 13 quoted lines

> diff --git a/Documentation/git-backfill.txt b/Documentation/git-backfill.txt
> index ece887831f6..e392517869c 100644
> --- a/Documentation/git-backfill.txt
> +++ b/Documentation/git-backfill.txt
> @@ -9,7 +9,7 @@ git-backfill - Download missing objects in a partial clone
>  SYNOPSIS
>  --------
>  [verse]
> -'git backfill' [<options>]
> +'git backfill' [--batch-size=<n>]
>  
>  DESCRIPTION
>  -----------

Here, as well.

Show 10 quoted lines

> diff --git a/builtin/backfill.c b/builtin/backfill.c
> index 177fd4286c7..ddccececc36 100644
> --- a/builtin/backfill.c
> +++ b/builtin/backfill.c
> @@ -21,14 +21,14 @@
>  #include "path-walk.h"
>  
>  static const char * const builtin_backfill_usage[] = {
> -	N_("git backfill [<options>]"),
> +	N_("git backfill [--batch-size=<n>]"),

And here.

Show 7 quoted lines

> @@ -111,9 +111,11 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
>  	struct backfill_context ctx = {
>  		.repo = repo,
>  		.current_batch = OID_ARRAY_INIT,
> -		.batch_size = 50000,
> +		.min_batch_size = 50000,
>  	};

Nit: it would be nice to adjust the name of this variable in the
preceding commit already so that it doesn't have to change again over
here.

Patrick