Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remote: prefetch config #1779

Conversation

pastelsky
Copy link

@pastelsky pastelsky commented Sep 4, 2024

CC: Patrick Steinhardt ps@pks.im, Junio C Hamano gitster@pobox.com
cc: Derrick Stolee stolee@gmail.com

@pastelsky pastelsky force-pushed the sk/maintenance-prefetch-remote branch 2 times, most recently from aedcfd8 to 21bccac Compare September 4, 2024 17:02
@pastelsky
Copy link
Author

/preview

Copy link

gitgitgadget bot commented Sep 4, 2024

Preview email sent as pull.1779.git.1725469826836.gitgitgadget@gmail.com

@pastelsky pastelsky force-pushed the sk/maintenance-prefetch-remote branch from 21bccac to 667553b Compare September 4, 2024 17:32
@pastelsky
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Sep 4, 2024

Submitted as pull.1779.git.1725472799637.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1779/pastelsky/sk/maintenance-prefetch-remote-v1

To fetch this version to local tag pr-1779/pastelsky/sk/maintenance-prefetch-remote-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1779/pastelsky/sk/maintenance-prefetch-remote-v1

Copy link

gitgitgadget bot commented Sep 4, 2024

On the Git mailing list, Derrick Stolee wrote (reply to this):

On 9/4/24 1:59 PM, Shubham Kanodia via GitGitGadget wrote:
> From: Shubham Kanodia <shubham.kanodia10@gmail.com>
> > Large repositories often contain numerous branches and refs, many of
> which individual users may not need. This commit introduces a new
> configuration option (`remote.<remote>.prefetch`) to allow
> users to specify which remotes to prefetch during
> the maintenance task.
> > Key behaviors:
> 1. If `remote.<remote>.prefetch` is unset or true, running
>     `git-maintenance` will prefetch all refs for the remote.
> 2. If `remote.<remote>.prefetch` is set to false, the remote
>     will be ignored for prefetching.

Thanks for this contribution. I think this is a good idea for extra
flexibility of the prefetch task.

> In a future change, we could also allow restricting the refs that are
> prefetched per remote using the `prefetchref` config option per remote.

I agree that this would also be of interest, but more complicated. Thanks
for starting with this simpler modification.

> +	if (remote->prefetch == 0)
> +		return 0;

In the Git codebase, this would normally be written as

	if (!remote->prefetch)
		return 0;

> +		# Run maintenance prefetch task
> +		GIT_TRACE2_EVENT="$(pwd)/prefetch.txt" git maintenance run --task=prefetch 2>/dev/null &&
> +
> +		# Check that remote1 was not fetched (prefetch=false)
> +		test_subcommand ! git fetch remote1 --prefetch --prune --no-tags \
> +			--no-write-fetch-head --recurse-submodules=no --quiet \
> +			<prefetch.txt &&

I'm happy to see this use of test_subcommand to validate the behavior
of this patch!

This is a very good patch and I only have the one style nit.

Thanks,
-Stolee

Copy link

gitgitgadget bot commented Sep 4, 2024

User Derrick Stolee <stolee@gmail.com> has been added to the cc: list.

Copy link

gitgitgadget bot commented Sep 4, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

Derrick Stolee <stolee@gmail.com> writes:

> I agree that this would also be of interest, but more complicated. Thanks
> for starting with this simpler modification.
>
>> +	if (remote->prefetch == 0)
>> +		return 0;
>
> In the Git codebase, this would normally be written as
>
> 	if (!remote->prefetch)
> 		return 0;

;-)

>> +		# Run maintenance prefetch task
>> +		GIT_TRACE2_EVENT="$(pwd)/prefetch.txt" git maintenance run --task=prefetch 2>/dev/null &&
>> +
>> +		# Check that remote1 was not fetched (prefetch=false)
>> +		test_subcommand ! git fetch remote1 --prefetch --prune --no-tags \
>> +			--no-write-fetch-head --recurse-submodules=no --quiet \
>> +			<prefetch.txt &&
>
> I'm happy to see this use of test_subcommand to validate the behavior
> of this patch!

I found it a bit disturbing that the pattern is overly specific.

The only thing we are interested in is that we are not fetching from
remote1, so it _should_ suffice if we could write

	test_subcommand ! git fetch remote1 <prefetch.txt &&

to avoid being tied to how the current version of Git happens to
pass these command line option flags and the order it does so.

Looking at the implementation of test_subcommand, it seems that we
cannot quite do that (it assumes that the pattern it assembles out
of the parameters are to match the full argument list used in
invocation, enclosing them in a single [] pair and without giving
the caller an easy way to sneak wildcards like ".*" in), which is
sad.


So, the expected command line being too strit is *not* a fault of
this patch, and with the style fix, I think this half of the
solution is a good one.

Thanks.

@pastelsky pastelsky force-pushed the sk/maintenance-prefetch-remote branch from 667553b to 330a3d5 Compare September 5, 2024 01:59
Copy link

gitgitgadget bot commented Sep 5, 2024

On the Git mailing list, Derrick Stolee wrote (reply to this):

On 9/4/24 4:55 PM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:

>>> +		# Run maintenance prefetch task
>>> +		GIT_TRACE2_EVENT="$(pwd)/prefetch.txt" git maintenance run --task=prefetch 2>/dev/null &&
>>> +
>>> +		# Check that remote1 was not fetched (prefetch=false)
>>> +		test_subcommand ! git fetch remote1 --prefetch --prune --no-tags \
>>> +			--no-write-fetch-head --recurse-submodules=no --quiet \
>>> +			<prefetch.txt &&
>>
>> I'm happy to see this use of test_subcommand to validate the behavior
>> of this patch!
> > I found it a bit disturbing that the pattern is overly specific.
> > The only thing we are interested in is that we are not fetching from
> remote1, so it _should_ suffice if we could write
> > 	test_subcommand ! git fetch remote1 <prefetch.txt &&
> > to avoid being tied to how the current version of Git happens to
> pass these command line option flags and the order it does so.
> > Looking at the implementation of test_subcommand, it seems that we
> cannot quite do that (it assumes that the pattern it assembles out
> of the parameters are to match the full argument list used in
> invocation, enclosing them in a single [] pair and without giving
> the caller an easy way to sneak wildcards like ".*" in), which is
> sad.
I agree the ergonomics of the test_subcommand helper is a bit poor
(and not this patch author's fault). The trickiest part is the
negative case, as in this highlighted one. It's hard to read from
this if the subcommand wasn't found because the argument list is
too specific and doesn't match the exact arguments.

It helps that the same options are given for the other, positive
tests. But maybe that could be a hint as to how to make this test
a bit cleaner: make a variable describing the "uninteresting"
arguments. Something like...

	args="--prefetch --prune --no-tags \
 	      --no-write-fetch-head --recurse-submodules=no --quiet" &&

	test_subcommand ! git fetch remote1 $args <prefetch.txt &&
	test_subcommand git fetch remote2 $args <prefetch.txt &&
	test_subcommand git fetch remote3 $args <prefetch.txt &&

Thanks,
-Stolee

@pastelsky pastelsky force-pushed the sk/maintenance-prefetch-remote branch 3 times, most recently from 5d33b4e to 1d58b78 Compare September 5, 2024 02:30
@pastelsky
Copy link
Author

Agree with both the suggestions. Sending an updated patch with the feedback incorporated.

@pastelsky
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Sep 5, 2024

Submitted as pull.1779.v2.git.1725504725976.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1779/pastelsky/sk/maintenance-prefetch-remote-v2

To fetch this version to local tag pr-1779/pastelsky/sk/maintenance-prefetch-remote-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1779/pastelsky/sk/maintenance-prefetch-remote-v2

Copy link

gitgitgadget bot commented Sep 5, 2024

On the Git mailing list, Shubham Kanodia wrote (reply to this):

On Thu, Sep 5, 2024 at 7:38 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 9/4/24 4:55 PM, Junio C Hamano wrote:
> > Derrick Stolee <stolee@gmail.com> writes:
>
> >>> +           # Run maintenance prefetch task
> >>> +           GIT_TRACE2_EVENT="$(pwd)/prefetch.txt" git maintenance run --task=prefetch 2>/dev/null &&
> >>> +
> >>> +           # Check that remote1 was not fetched (prefetch=false)
> >>> +           test_subcommand ! git fetch remote1 --prefetch --prune --no-tags \
> >>> +                   --no-write-fetch-head --recurse-submodules=no --quiet \
> >>> +                   <prefetch.txt &&
> >>
> >> I'm happy to see this use of test_subcommand to validate the behavior
> >> of this patch!
> >
> > I found it a bit disturbing that the pattern is overly specific.
> >
> > The only thing we are interested in is that we are not fetching from
> > remote1, so it _should_ suffice if we could write
> >
> >       test_subcommand ! git fetch remote1 <prefetch.txt &&
> >
> > to avoid being tied to how the current version of Git happens to
> > pass these command line option flags and the order it does so.
> >
> > Looking at the implementation of test_subcommand, it seems that we
> > cannot quite do that (it assumes that the pattern it assembles out
> > of the parameters are to match the full argument list used in
> > invocation, enclosing them in a single [] pair and without giving
> > the caller an easy way to sneak wildcards like ".*" in), which is
> > sad.
> I agree the ergonomics of the test_subcommand helper is a bit poor
> (and not this patch author's fault). The trickiest part is the
> negative case, as in this highlighted one. It's hard to read from
> this if the subcommand wasn't found because the argument list is
> too specific and doesn't match the exact arguments.
>
> It helps that the same options are given for the other, positive
> tests. But maybe that could be a hint as to how to make this test
> a bit cleaner: make a variable describing the "uninteresting"
> arguments. Something like...
>
>         args="--prefetch --prune --no-tags \
>               --no-write-fetch-head --recurse-submodules=no --quiet" &&
>
>         test_subcommand ! git fetch remote1 $args <prefetch.txt &&
>         test_subcommand git fetch remote2 $args <prefetch.txt &&
>         test_subcommand git fetch remote3 $args <prefetch.txt &&
>
> Thanks,
> -Stolee
>

Agree with both the suggestions here. Updated my patch.

Copy link

gitgitgadget bot commented Sep 5, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

Derrick Stolee <stolee@gmail.com> writes:

> On 9/4/24 4:55 PM, Junio C Hamano wrote:
>> Derrick Stolee <stolee@gmail.com> writes:
>
>>>> +		# Run maintenance prefetch task
>>>> +		GIT_TRACE2_EVENT="$(pwd)/prefetch.txt" git maintenance run --task=prefetch 2>/dev/null &&
>>>> +
>>>> +		# Check that remote1 was not fetched (prefetch=false)
>>>> +		test_subcommand ! git fetch remote1 --prefetch --prune --no-tags \
>>>> +			--no-write-fetch-head --recurse-submodules=no --quiet \
>>>> +			<prefetch.txt &&
>>>
>>> I'm happy to see this use of test_subcommand to validate the behavior
>>> of this patch!
>> I found it a bit disturbing that the pattern is overly specific.
>> The only thing we are interested in is that we are not fetching from
>> remote1, so it _should_ suffice if we could write
>> 	test_subcommand ! git fetch remote1 <prefetch.txt &&
>> to avoid being tied to how the current version of Git happens to
>> pass these command line option flags and the order it does so.
>> Looking at the implementation of test_subcommand, it seems that we
>> cannot quite do that (it assumes that the pattern it assembles out
>> of the parameters are to match the full argument list used in
>> invocation, enclosing them in a single [] pair and without giving
>> the caller an easy way to sneak wildcards like ".*" in), which is
>> sad.
> I agree the ergonomics of the test_subcommand helper is a bit poor
> (and not this patch author's fault).

I suspect that we could do

    test_subcommand ! git fetch remote1 --prefetch '.*' <prefetch.txt

which would be rewritten to this pattern

	\["git", "fetch", "remote1", "--prefetch", ".*"\]

if I am reading how the expr given to grep is built by the
test_subcommand implementation.  As long a there is at least one
actual argument after the "--prefetch" one, .* would slurp
everything.

But it is ugly.  In any case, this is a tangent unrelated to the
topic of the patch on this thread.

Copy link

gitgitgadget bot commented Sep 5, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Shubham Kanodia via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/builtin/gc.c b/builtin/gc.c
> index 427faf1cfe1..2ca3a3e7d6a 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1027,6 +1027,9 @@ static int fetch_remote(struct remote *remote, void *cbdata)
>  	if (remote->skip_default_update)
>  		return 0;
>  
> +	if (!remote->prefetch)
> +		return 0;

This, while better than ane xplicit comparison with "== 0", is a bit
tricky in this patch, as it is not saying "if we are told to prefetch,
fall through to the rest of the function".  It is saying "leave if
and only if we are explicitly configured not to prefetch".

It might warrant a comment.

> diff --git a/remote.c b/remote.c
> index 8f3dee13186..05edb3a5f40 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -140,6 +140,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
>  	CALLOC_ARRAY(ret, 1);
>  	ret->prune = -1;  /* unspecified */
>  	ret->prune_tags = -1;  /* unspecified */
> +	ret->prefetch = -1;  /* unspecified */

Or, we can just assign "1" (and drop "unspecified" comment).

	ret->prefetch = 1; /* enabled by default */

If I understand it correctly, we want this to default to true...

>  	ret->name = xstrndup(name, len);
>  	refspec_init(&ret->push, REFSPEC_PUSH);
>  	refspec_init(&ret->fetch, REFSPEC_FETCH);
> @@ -456,6 +457,8 @@ static int handle_config(const char *key, const char *value,
>  		remote->prune = git_config_bool(key, value);
>  	else if (!strcmp(subkey, "prunetags"))
>  		remote->prune_tags = git_config_bool(key, value);
> +	else if (!strcmp(subkey, "prefetch"))
> +		remote->prefetch = git_config_bool(key, value);

... with a way for the user to turn it off.

> diff --git a/remote.h b/remote.h
> index b901b56746d..4522fdec354 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -77,6 +77,15 @@ struct remote {
>  
>  	struct refspec fetch;
>  
> +	/*
> +	 * The setting for whether to prefetch from a remote
> +	 * when a fetch is invoked with a prefetch flag.
> +	 *  -1 = unset
> +	 *   0 = don't prefetch from this remote
> +	 *   1 = prefetch from this remote
> +	 */
> +	int prefetch;

And then we can get rid of "-1 unset" from this list.  The comment
can become a lot more brief, as such a change would make it a simple
Boolean flag that everybody would understand immediately.

"prefetch" in the comment is superfluous, as that is the name of the
member anyway.  "from this remote" is superfluous, as that is the
point of having the member in "struct remote" that gives settings
that are per-remote.

	int prefetch; /* is prefetch enabled? */

If we really want to have "unspecified yet" state, what we commonly
do is

 * to initialize the variable to -1 to signal "unspecified yet",
   which you did in this patch.

 * after the configuration reader returns, check if the variable is
   still -1, and then explicitly reset it to the default value,
   which your patch does not do.

 * the code that uses the variable assumes it is either 0 or 1 and
   there shoudl be no "unspecified yet" value.  It indeed is a bug
   that the ariable is left unspecified as it is a sign that the
   code to do previous step was somehow skipped.

But I do not think it is needed in this case; initializing the
.prefetch member to whichever is the default should be sufficient.

Thanks.

@pastelsky pastelsky force-pushed the sk/maintenance-prefetch-remote branch from 1d58b78 to c348f8e Compare September 5, 2024 16:41
@pastelsky
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Sep 5, 2024

Submitted as pull.1779.v3.git.1725554758463.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1779/pastelsky/sk/maintenance-prefetch-remote-v3

To fetch this version to local tag pr-1779/pastelsky/sk/maintenance-prefetch-remote-v3:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1779/pastelsky/sk/maintenance-prefetch-remote-v3

Copy link

gitgitgadget bot commented Sep 5, 2024

On the Git mailing list, Shubham Kanodia wrote (reply to this):

On Thu, Sep 5, 2024 at 9:36 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Shubham Kanodia via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index 427faf1cfe1..2ca3a3e7d6a 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -1027,6 +1027,9 @@ static int fetch_remote(struct remote *remote, void *cbdata)
> >       if (remote->skip_default_update)
> >               return 0;
> >
> > +     if (!remote->prefetch)
> > +             return 0;
>
> This, while better than ane xplicit comparison with "== 0", is a bit
> tricky in this patch, as it is not saying "if we are told to prefetch,
> fall through to the rest of the function".  It is saying "leave if
> and only if we are explicitly configured not to prefetch".
>
> It might warrant a comment.
>
> > diff --git a/remote.c b/remote.c
> > index 8f3dee13186..05edb3a5f40 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -140,6 +140,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
> >       CALLOC_ARRAY(ret, 1);
> >       ret->prune = -1;  /* unspecified */
> >       ret->prune_tags = -1;  /* unspecified */
> > +     ret->prefetch = -1;  /* unspecified */
>
> Or, we can just assign "1" (and drop "unspecified" comment).
>
>         ret->prefetch = 1; /* enabled by default */
>
> If I understand it correctly, we want this to default to true...
>
> >       ret->name = xstrndup(name, len);
> >       refspec_init(&ret->push, REFSPEC_PUSH);
> >       refspec_init(&ret->fetch, REFSPEC_FETCH);
> > @@ -456,6 +457,8 @@ static int handle_config(const char *key, const char *value,
> >               remote->prune = git_config_bool(key, value);
> >       else if (!strcmp(subkey, "prunetags"))
> >               remote->prune_tags = git_config_bool(key, value);
> > +     else if (!strcmp(subkey, "prefetch"))
> > +             remote->prefetch = git_config_bool(key, value);
>
> ... with a way for the user to turn it off.
>
> > diff --git a/remote.h b/remote.h
> > index b901b56746d..4522fdec354 100644
> > --- a/remote.h
> > +++ b/remote.h
> > @@ -77,6 +77,15 @@ struct remote {
> >
> >       struct refspec fetch;
> >
> > +     /*
> > +      * The setting for whether to prefetch from a remote
> > +      * when a fetch is invoked with a prefetch flag.
> > +      *  -1 = unset
> > +      *   0 = don't prefetch from this remote
> > +      *   1 = prefetch from this remote
> > +      */
> > +     int prefetch;
>
> And then we can get rid of "-1 unset" from this list.  The comment
> can become a lot more brief, as such a change would make it a simple
> Boolean flag that everybody would understand immediately.
>
> "prefetch" in the comment is superfluous, as that is the name of the
> member anyway.  "from this remote" is superfluous, as that is the
> point of having the member in "struct remote" that gives settings
> that are per-remote.
>
>         int prefetch; /* is prefetch enabled? */
>
> If we really want to have "unspecified yet" state, what we commonly
> do is
>
>  * to initialize the variable to -1 to signal "unspecified yet",
>    which you did in this patch.
>
>  * after the configuration reader returns, check if the variable is
>    still -1, and then explicitly reset it to the default value,
>    which your patch does not do.
>
>  * the code that uses the variable assumes it is either 0 or 1 and
>    there shoudl be no "unspecified yet" value.  It indeed is a bug
>    that the ariable is left unspecified as it is a sign that the
>    code to do previous step was somehow skipped.
>
> But I do not think it is needed in this case; initializing the
> .prefetch member to whichever is the default should be sufficient.
>
> Thanks.

Fair. I kept the initial value as `unset` as that could be interpreted
as a special case to do something else in the future — but I agree that keeping
it initialized to default keeps things clearer for now since such a
case doesn't arise.

Updating my patch — please let me know if there's anything else I can
improve here.

Copy link

gitgitgadget bot commented Sep 5, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

Shubham Kanodia <shubham.kanodia10@gmail.com> writes:

>>
>>         int prefetch; /* is prefetch enabled? */
>>
> ...
> Updating my patch — please let me know if there's anything else I can
> improve here.

Renaming the .prefetch member to .prefetch_enabled would eliminate
the need to add any comment on the member in the header file.

Thanks.

Copy link

gitgitgadget bot commented Sep 5, 2024

On the Git mailing list, Shubham Kanodia wrote (reply to this):

On Thu, Sep 5, 2024 at 10:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Shubham Kanodia <shubham.kanodia10@gmail.com> writes:
>
> >>
> >>         int prefetch; /* is prefetch enabled? */
> >>
> > ...
> > Updating my patch — please let me know if there's anything else I can
> > improve here.
>
> Renaming the .prefetch member to .prefetch_enabled would eliminate
> the need to add any comment on the member in the header file.

Do you mean for the struct member here or also the config? For the
config, it'll probably be clearer
to keep `prefetch` still as it aligns nicely with the boolean
`--prefetch` command line flag.

I can name the struct member `prefetch_enabled` — though I don't see
other boolean remote properties (`prune`, `prune_tags`) add suffixes
to indicate
they are booleans.

> Thanks.

Copy link

gitgitgadget bot commented Sep 5, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

Shubham Kanodia <shubham.kanodia10@gmail.com> writes:

> On Thu, Sep 5, 2024 at 10:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Shubham Kanodia <shubham.kanodia10@gmail.com> writes:
>>
>> >>
>> >>         int prefetch; /* is prefetch enabled? */
>> >>
>> > ...
>> > Updating my patch — please let me know if there's anything else I can
>> > improve here.
>>
>> Renaming the .prefetch member to .prefetch_enabled would eliminate
>> the need to add any comment on the member in the header file.
>
> Do you mean for the struct member here or also the config?

I do not think I mentioned anything about the name of the
configuration variable, but if I did that was a mistake.

End-user facing configuration variables are often named after a
feature that it enables or disables, so it can use the name without
"enable".  An int variable on the other hand can mean many other
things, ranging from "how many times have we prefetched from here"
to "does this remote allow prefetching?", so a more explicit name
would often help.

Thanks.

Copy link

gitgitgadget bot commented Sep 5, 2024

This patch series was integrated into seen via git@f497e39.

@gitgitgadget gitgitgadget bot added the seen label Sep 5, 2024
Large repositories often contain numerous branches and refs, many of
which individual users may not need. This commit introduces a new
configuration option (`remote.<remote>.prefetch`) to allow
users to specify which remotes to prefetch during
the maintenance task.

Key behaviors:
1. If `remote.<remote>.prefetch` is unset or true, running
   `git-maintenance` will prefetch all refs for the remote.
2. If `remote.<remote>.prefetch` is set to false, the remote
   will be ignored for prefetching.

In a future change, we could also allow restricting the refs that are
prefetched per remote using the `prefetchref` config option per remote.

Both of these options in unison would allow users to optimize their
prefetch operations, reducing network traffic and disk usage.

Signed-off-by: Shubham Kanodia <shubham.kanodia10@gmail.com>
@pastelsky pastelsky force-pushed the sk/maintenance-prefetch-remote branch from c348f8e to 80af121 Compare September 5, 2024 19:19
@pastelsky
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Sep 5, 2024

Submitted as pull.1779.v4.git.1725565398681.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1779/pastelsky/sk/maintenance-prefetch-remote-v4

To fetch this version to local tag pr-1779/pastelsky/sk/maintenance-prefetch-remote-v4:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1779/pastelsky/sk/maintenance-prefetch-remote-v4

Copy link

gitgitgadget bot commented Sep 5, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Shubham Kanodia via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Shubham Kanodia <shubham.kanodia10@gmail.com>
> ...
> In a future change, we could also allow restricting the refs that are
> prefetched per remote using the `prefetchref` config option per remote.
>
> Both of these options in unison would allow users to optimize their
> prefetch operations, reducing network traffic and disk usage.
>
> Signed-off-by: Shubham Kanodia <shubham.kanodia10@gmail.com>
> ---

Looking good.  Thanks.

Copy link

gitgitgadget bot commented Sep 5, 2024

This patch series was integrated into seen via git@e3530b2.

Copy link

gitgitgadget bot commented Sep 6, 2024

On the Git mailing list, Shubham Kanodia wrote (reply to this):

On Fri, Sep 6, 2024 at 2:28 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Shubham Kanodia via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Shubham Kanodia <shubham.kanodia10@gmail.com>
> > ...
> > In a future change, we could also allow restricting the refs that are
> > prefetched per remote using the `prefetchref` config option per remote.
> >
> > Both of these options in unison would allow users to optimize their
> > prefetch operations, reducing network traffic and disk usage.
> >
> > Signed-off-by: Shubham Kanodia <shubham.kanodia10@gmail.com>
> > ---
>
> Looking good.  Thanks.

How long do you reckon changes like this typically remain in "seen"
until merged upstream?
I'm preparing part-2 of this change separately — so would be good to
know when I can submit that.

Copy link

gitgitgadget bot commented Sep 6, 2024

This branch is now known as sk/enable-prefetch-per-remote.

Copy link

gitgitgadget bot commented Sep 6, 2024

This patch series was integrated into seen via git@069bc89.

Copy link

gitgitgadget bot commented Sep 6, 2024

This patch series was integrated into seen via git@04afd77.

Copy link

gitgitgadget bot commented Sep 7, 2024

This patch series was integrated into seen via git@9c67b13.

Copy link

gitgitgadget bot commented Sep 7, 2024

This patch series was integrated into seen via git@bee35cf.

Copy link

gitgitgadget bot commented Sep 9, 2024

There was a status update in the "Discarded" section about the branch sk/enable-prefetch-per-remote on the Git mailing list:

The prefetch task of "git maintenance" learned to honor the
"remote.<name>.prefetch" configuration variable, which can be used
to selectively disable prefetching from selected remote
repositories.

Retracted.
cf. <CAG=Um+0X3Umt-2TQ-BGeefqdGxfVoy2Ug0tGKLycrX=_pj=oJw@mail.gmail.com>
source: <pull.1779.v4.git.1725565398681.gitgitgadget@gmail.com>

@pastelsky pastelsky closed this Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant