diff mbox series

[v2,6/7] maintenance: recommended schedule in register/start

Message ID f609c1bde27558db2a9601ac34f8a51ce86f0e2c.1599846561.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Maintenance III: Background maintenance | expand

Commit Message

Linus Arver via GitGitGadget Sept. 11, 2020, 5:49 p.m. UTC
From: Derrick Stolee <dstolee@microsoft.com>

The 'git maintenance (register|start)' subcommands add the current
repository to the global Git config so maintenance will operate on that
repository. It does not specify what maintenance should occur or how
often.

If a user sets any 'maintenance.<task>.schedule' config value, then
they have chosen a specific schedule for themselves and Git should
respect that.

However, in an effort to recommend a good schedule for repositories of
all sizes, set new config values for recommended tasks that are safe to
run in the background while users run foreground Git commands. These
commands are generally everything but the 'gc' task.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/git-maintenance.txt |  6 ++++
 builtin/gc.c                      | 46 +++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh            | 16 +++++++++++
 3 files changed, 68 insertions(+)

Comments

Martin Ågren Sept. 29, 2020, 7:48 p.m. UTC | #1
Hi Stolee,

On Fri, 11 Sep 2020 at 19:53, Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> If a user sets any 'maintenance.<task>.schedule' config value, then
> they have chosen a specific schedule for themselves and Git should
> respect that.
>
> However, in an effort to recommend a good schedule for repositories of
> all sizes, set new config values for recommended tasks that are safe to
> run in the background while users run foreground Git commands. These
> commands are generally everything but the 'gc' task.

If there aren't any "schedule" configurations, we'll go ahead and
sprinkle in quite a few of them. I suppose that another approach would
be that later, much later, when we go look for these configuration
items, we could go "there is not a single one set, let's act as if
*these* were configured".

The advantage there would be that we can tweak those defaults over time.
Whereas with the approach of this patch, v2.29.0 will give the user a
snapshot of 2020's best practices. If they want to catch up, they will
need to drop all their "schedule" config and re-"register", or use a
future `git maintenance reregister`. ;-)

Anyway, this is a convenience thing. There's a chance that "convenience"
interferes with "perfect" and "optimal". I guess that's to be expected.

> +If your repository has no 'maintenance.<task>.schedule' configuration

Thank you for going above and beyond with marking config items et cetera
for rendering in `monospace`. I just noticed that this is slightly
mis-marked-upped. If you end up rerolling this patch series for some
reason, you might want to switch from 'single quotes' to `backticks` in
this particular instance.

While I'm commenting anyway...

> +static int has_schedule_config(void)
> +{
> +       int i, found = 0;
> +       struct strbuf config_name = STRBUF_INIT;
> +       size_t prefix;
> +
> +       strbuf_addstr(&config_name, "maintenance.");
> +       prefix = config_name.len;
> +
> +       for (i = 0; !found && i < TASK__COUNT; i++) {
> +               char *value;
> +
> +               strbuf_setlen(&config_name, prefix);
> +               strbuf_addf(&config_name, "%s.schedule", tasks[i].name);
> +
> +               if (!git_config_get_string(config_name.buf, &value)) {
> +                       found = 1;
> +                       FREE_AND_NULL(value);
> +               }
> +       }
> +
> +       strbuf_release(&config_name);
> +       return found;
> +}

That `FREE_AND_NULL()` caught me off-guard. The pointer is on the stack.
I suppose it doesn't *hurt*, but being careful to set it to NULL made me
go "huh".

I suppose you could drop the `!found` check in favour of `break`-ing
precisely when you get a hit.

And I do wonder how much the reuse of the "maintenance." part of the
buffer helps performance.

In the end, you could use something like the following (not compiled):

  static int has_schedule_config(void)
  {
         int i, found = 0;
         struct strbuf config_name = STRBUF_INIT;

         for (i = 0; i < TASK__COUNT; i++) {
                 const char *value;

                 strbuf_reset(&config_name);
                 strbuf_addf(&config_name, "maintenance.%s.schedule",
tasks[i].name);

                 if (!git_config_get_value(config_name.buf, &value)) {
                         found = 1;
                         break;
                 }
         }

         strbuf_release(&config_name);
         return found;
  }

Anyway, that's just microniting, obviously, but maybe in the sum it has
some value.


Martin
Derrick Stolee Sept. 30, 2020, 8:11 p.m. UTC | #2
On 9/29/2020 3:48 PM, Martin Ågren wrote:
> Hi Stolee,
> 
> On Fri, 11 Sep 2020 at 19:53, Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> If a user sets any 'maintenance.<task>.schedule' config value, then
>> they have chosen a specific schedule for themselves and Git should
>> respect that.
>>
>> However, in an effort to recommend a good schedule for repositories of
>> all sizes, set new config values for recommended tasks that are safe to
>> run in the background while users run foreground Git commands. These
>> commands are generally everything but the 'gc' task.
> 
> If there aren't any "schedule" configurations, we'll go ahead and
> sprinkle in quite a few of them. I suppose that another approach would
> be that later, much later, when we go look for these configuration
> items, we could go "there is not a single one set, let's act as if
> *these* were configured".

I do like this alternative.

> The advantage there would be that we can tweak those defaults over time.
> Whereas with the approach of this patch, v2.29.0 will give the user a
> snapshot of 2020's best practices. If they want to catch up, they will
> need to drop all their "schedule" config and re-"register", or use a
> future `git maintenance reregister`. ;-)

This is a significant advantage! Great idea.

It might be a bit difficult to slide this in, but I bet it would work
out OK if we have a "initialize_schedule()" option that is only run
when the "--schedule=<...>" option is given? The trickiest part is
actually setting the ".enabled" configs to "true" as well. The condition
for using the "default" schedule might get a bit complicated. I do think
it is worth some effort to do, as adjusting defaults in code is certainly
easier than modifying config values.

> Anyway, this is a convenience thing. There's a chance that "convenience"
> interferes with "perfect" and "optimal". I guess that's to be expected.
> 
>> +If your repository has no 'maintenance.<task>.schedule' configuration
> 
> Thank you for going above and beyond with marking config items et cetera
> for rendering in `monospace`. I just noticed that this is slightly
> mis-marked-upped. If you end up rerolling this patch series for some
> reason, you might want to switch from 'single quotes' to `backticks` in
> this particular instance.

Thanks! Yeah that was a mis-type.

> While I'm commenting anyway...
> 
>> +static int has_schedule_config(void)
>> +{
>> +       int i, found = 0;
>> +       struct strbuf config_name = STRBUF_INIT;
>> +       size_t prefix;
>> +
>> +       strbuf_addstr(&config_name, "maintenance.");
>> +       prefix = config_name.len;
>> +
>> +       for (i = 0; !found && i < TASK__COUNT; i++) {
>> +               char *value;
>> +
>> +               strbuf_setlen(&config_name, prefix);
>> +               strbuf_addf(&config_name, "%s.schedule", tasks[i].name);
>> +
>> +               if (!git_config_get_string(config_name.buf, &value)) {
>> +                       found = 1;
>> +                       FREE_AND_NULL(value);
>> +               }
>> +       }
>> +
>> +       strbuf_release(&config_name);
>> +       return found;
>> +}
> 
> That `FREE_AND_NULL()` caught me off-guard. The pointer is on the stack.
> I suppose it doesn't *hurt*, but being careful to set it to NULL made me
> go "huh".
> 
> I suppose you could drop the `!found` check in favour of `break`-ing
> precisely when you get a hit.
> 
> And I do wonder how much the reuse of the "maintenance." part of the
> buffer helps performance.

All valid points.

> In the end, you could use something like the following (not compiled):
> 
>   static int has_schedule_config(void)
>   {
>          int i, found = 0;
>          struct strbuf config_name = STRBUF_INIT;
> 
>          for (i = 0; i < TASK__COUNT; i++) {
>                  const char *value;
> 
>                  strbuf_reset(&config_name);
>                  strbuf_addf(&config_name, "maintenance.%s.schedule",
> tasks[i].name);
> 
>                  if (!git_config_get_value(config_name.buf, &value)) {
>                          found = 1;
>                          break;
>                  }
>          }
> 
>          strbuf_release(&config_name);
>          return found;
>   }
> 
> Anyway, that's just microniting, obviously, but maybe in the sum it has
> some value.

Sounds good to me. I'll work on a new version that makes your
recommendations.

Thanks,
-Stolee
Derrick Stolee Oct. 1, 2020, 8:38 p.m. UTC | #3
On 9/30/2020 4:11 PM, Derrick Stolee wrote:
> On 9/29/2020 3:48 PM, Martin Ågren wrote:
>> If there aren't any "schedule" configurations, we'll go ahead and
>> sprinkle in quite a few of them. I suppose that another approach would
>> be that later, much later, when we go look for these configuration
>> items, we could go "there is not a single one set, let's act as if
>> *these* were configured".
> 
> I do like this alternative.
> 
>> The advantage there would be that we can tweak those defaults over time.
>> Whereas with the approach of this patch, v2.29.0 will give the user a
>> snapshot of 2020's best practices. If they want to catch up, they will
>> need to drop all their "schedule" config and re-"register", or use a
>> future `git maintenance reregister`. ;-)
> 
> This is a significant advantage! Great idea.

Thank you for giving me the idea to pursue this direction. The
replacement patch isn't too bad, and I think this is a much better
approach to satisfy the situation.

What do you think?

Thanks,
-Stolee

-- >8 --

From 7c7698da17327d17485ba1b23a16a0a8d54efaad Mon Sep 17 00:00:00 2001
From: Derrick Stolee <dstolee@microsoft.com>
Date: Tue, 18 Aug 2020 15:15:02 -0400
Subject: [PATCH] maintenance: recommended schedule in register/start
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The 'git maintenance (register|start)' subcommands add the current
repository to the global Git config so maintenance will operate on that
repository. It does not specify what maintenance should occur or how
often.

If a user sets any 'maintenance.<task>.schedule' config value, then
they have chosen a specific schedule for themselves and Git should
respect that.

To make this process extremely simple for users, assume a default
schedule when no 'maintenance.<task>.schedule' or '...enabled' config
settings are concretely set. This is only an in-process assumption, so
future versions of Git could adjust this expected schedule.

Helped-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/git-maintenance.txt | 15 ++++++++
 builtin/gc.c                      | 58 +++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh            | 11 +++---
 3 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index 7628a6d157..52fff86844 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -37,6 +37,21 @@ register::
 	`maintenance.<task>.schedule`. The tasks that are enabled are safe
 	for running in the background without disrupting foreground
 	processes.
++
+If your repository has no `maintenance.<task>.schedule` configuration
+values set, then Git will use a recommended default schedule that performs
+background maintenance that will not interrupt foreground commands. The
+default schedule is as follows:
++
+* `gc`: disabled.
+* `commit-graph`: hourly.
+* `prefetch`: hourly.
+* `loose-objects`: daily.
+* `incremental-repack`: daily.
++
+`git maintenance register` will also disable foreground maintenance by
+setting `maintenance.auto = false` in the current repository. This config
+setting will remain after a `git maintenance unregister` command.
 
 run::
 	Run one or more maintenance tasks. If one or more `--task` options
diff --git a/builtin/gc.c b/builtin/gc.c
index a387f46585..965690704b 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1251,6 +1251,59 @@ static int compare_tasks_by_selection(const void *a_, const void *b_)
 	return b->selected_order - a->selected_order;
 }
 
+static int has_schedule_config(void)
+{
+	int i, found = 0;
+	struct strbuf config_name = STRBUF_INIT;
+	size_t prefix;
+
+	strbuf_addstr(&config_name, "maintenance.");
+	prefix = config_name.len;
+
+	for (i = 0; !found && i < TASK__COUNT; i++) {
+		char *value;
+
+		strbuf_setlen(&config_name, prefix);
+		strbuf_addf(&config_name, "%s.schedule", tasks[i].name);
+
+		if (!git_config_get_string(config_name.buf, &value)) {
+			found = 1;
+			FREE_AND_NULL(value);
+		}
+
+		strbuf_setlen(&config_name, prefix);
+		strbuf_addf(&config_name, "%s.enabled", tasks[i].name);
+
+		if (!git_config_get_string(config_name.buf, &value)) {
+			found = 1;
+			FREE_AND_NULL(value);
+		}
+	}
+
+	strbuf_release(&config_name);
+	return found;
+}
+
+static void set_recommended_schedule(void)
+{
+	if (has_schedule_config())
+		return;
+
+	tasks[TASK_GC].enabled = 0;
+
+	tasks[TASK_PREFETCH].enabled = 1;
+	tasks[TASK_PREFETCH].schedule = SCHEDULE_HOURLY;
+
+	tasks[TASK_COMMIT_GRAPH].enabled = 1;
+	tasks[TASK_COMMIT_GRAPH].schedule = SCHEDULE_HOURLY;
+
+	tasks[TASK_LOOSE_OBJECTS].enabled = 1;
+	tasks[TASK_LOOSE_OBJECTS].schedule = SCHEDULE_DAILY;
+
+	tasks[TASK_INCREMENTAL_REPACK].enabled = 1;
+	tasks[TASK_INCREMENTAL_REPACK].schedule = SCHEDULE_DAILY;
+}
+
 static int maintenance_run_tasks(struct maintenance_run_opts *opts)
 {
 	int i, found_selected = 0;
@@ -1280,6 +1333,8 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts)
 
 	if (found_selected)
 		QSORT(tasks, TASK__COUNT, compare_tasks_by_selection);
+	else if (opts->schedule != SCHEDULE_NONE)
+		set_recommended_schedule();
 
 	for (i = 0; i < TASK__COUNT; i++) {
 		if (found_selected && tasks[i].selected_order < 0)
@@ -1417,6 +1472,9 @@ static int maintenance_register(void)
 	if (!the_repository || !the_repository->gitdir)
 		return 0;
 
+	/* Disable foreground maintenance */
+	git_config_set("maintenance.auto", "false");
+
 	config_get.git_cmd = 1;
 	strvec_pushl(&config_get.args, "config", "--global", "--get", "maintenance.repo",
 		     the_repository->worktree ? the_repository->worktree
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index c8d7e65d3d..fae2ef81bd 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -305,11 +305,14 @@ test_expect_success 'register and unregister' '
 	git config --global --add maintenance.repo /existing1 &&
 	git config --global --add maintenance.repo /existing2 &&
 	git config --global --get-all maintenance.repo >before &&
+
 	git maintenance register &&
-	git config --global --get-all maintenance.repo >actual &&
-	cp before after &&
-	pwd >>after &&
-	test_cmp after actual &&
+	test_cmp_config false maintenance.auto &&
+	git config --global --get-all maintenance.repo >between &&
+	cp before expect &&
+	pwd >>expect &&
+	test_cmp expect between &&
+
 	git maintenance unregister &&
 	git config --global --get-all maintenance.repo >actual &&
 	test_cmp before actual
Đoàn Trần Công Danh Oct. 2, 2020, 12:38 a.m. UTC | #4
On 2020-10-01 16:38:48-0400, Derrick Stolee <stolee@gmail.com> wrote:
> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
> index 7628a6d157..52fff86844 100644
> --- a/Documentation/git-maintenance.txt
> +++ b/Documentation/git-maintenance.txt
> @@ -37,6 +37,21 @@ register::
>  	`maintenance.<task>.schedule`. The tasks that are enabled are safe
>  	for running in the background without disrupting foreground
>  	processes.
> ++
> +If your repository has no `maintenance.<task>.schedule` configuration
> +values set, then Git will use a recommended default schedule that performs
> +background maintenance that will not interrupt foreground commands. The
> +default schedule is as follows:

I don't mind about using a default schedule (but someone else might).
I think some distributions will be paranoia with this change and shiped
with disable by default in system config.

> ++
> +* `gc`: disabled.
> +* `commit-graph`: hourly.
> +* `prefetch`: hourly.

However, no `prefetch` in default schedule, please.
IIUC, this is a network operation, if someone is on the go and paying
their internet based on their traffic, this will be a disaster.


> +* `loose-objects`: daily.
> +* `incremental-repack`: daily.

And I would say no incremental-repack, too.
Users don't want to a large operation of IO on some random time of the day,
be it when they open their PC in the morning, or when they want to close
their laptop to go home.

----------(Windows rant ahead)
I still remember those days that Windows 8 was introduced,
Back in that days, my computer still uses the old 7200rpm HDD.
I was super-angry that whenever Windows is started, it starts some IO
disk-caching, indexing that hung my computer for a good 10 minutes.
While that same computer can run Windows 7 and other OS fine.
I don't particularly care how much my computer is faster after that.
I want my computer usable at that time, instead of wasting a good 10
minutes on nothing.
---------(Windows rant end)

Either the users know what are they doing, or we don't do anything at
all. Let's them do it on their free time.

IOW, Please let users opt in instead of opt out of this features.
Derrick Stolee Oct. 2, 2020, 1:55 a.m. UTC | #5
On 10/1/2020 8:38 PM, Đoàn Trần Công Danh wrote:
> On 2020-10-01 16:38:48-0400, Derrick Stolee <stolee@gmail.com> wrote:
>> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
>> index 7628a6d157..52fff86844 100644
>> --- a/Documentation/git-maintenance.txt
>> +++ b/Documentation/git-maintenance.txt
>> @@ -37,6 +37,21 @@ register::
>>  	`maintenance.<task>.schedule`. The tasks that are enabled are safe
>>  	for running in the background without disrupting foreground
>>  	processes.
>> ++
>> +If your repository has no `maintenance.<task>.schedule` configuration
>> +values set, then Git will use a recommended default schedule that performs
>> +background maintenance that will not interrupt foreground commands. The
>> +default schedule is as follows:
> 
> I don't mind about using a default schedule (but someone else might).
> I think some distributions will be paranoia with this change and shiped
> with disable by default in system config.

If a user wants to prevent this schedule, then they can simply change
any one of the `.schedule` or `.enabled` configs in their --global config
and these defaults will not be used.

Of course, perhaps you are missing the fact that "git maintenance run
--schedule=<frequency>" is only run as a cron job if a user chose to
start background maintenance using "git maintenance start" (or "git
maintenance register" after running the 'start' subcommand in another
repo). So this is _not_ starting by default without some amount of
choosing to opt in.

>> ++
>> +* `gc`: disabled.
>> +* `commit-graph`: hourly.
>> +* `prefetch`: hourly.
> 
> However, no `prefetch` in default schedule, please.
> IIUC, this is a network operation, if someone is on the go and paying
> their internet based on their traffic, this will be a disaster.

It _is_ a network operation. You're right that we should make it clear
that network operations are being run in the background if the defaults
are being used.

Of course, this is why "git maintenance stop" exists. Background
maintenance can be halted while not being in a mode where this maintenance
is acceptable.

And further: these defaults are optimized for desktop machines that are
expected to always be on and connected to a non-metered network. Laptops
are not always on, not always connected, and sometimes are metered. Perhaps
a user should decide that they don't want to have background maintenance,
and then they can choose to not opt-in.

If this scenario is common enough, then we could extend the "prefetch"
task to somehow detect (on some platforms) that the network connection is
metered, and then not do any fetches.

>> +* `loose-objects`: daily.
>> +* `incremental-repack`: daily.
> 
> And I would say no incremental-repack, too.
> Users don't want to a large operation of IO on some random time of the day,
> be it when they open their PC in the morning, or when they want to close
> their laptop to go home.

But that's exactly why incremental-repack is an improvement over
a "random" instance of 'git gc --auto' going over an invisible threshold.
The "incremental" nature is intended to only do a reasonable amount of work 
instead of rewriting everything.
 
> ----------(Windows rant ahead)
> I still remember those days that Windows 8 was introduced,
> Back in that days, my computer still uses the old 7200rpm HDD.
> I was super-angry that whenever Windows is started, it starts some IO
> disk-caching, indexing that hung my computer for a good 10 minutes.
> While that same computer can run Windows 7 and other OS fine.
> I don't particularly care how much my computer is faster after that.
> I want my computer usable at that time, instead of wasting a good 10
> minutes on nothing.
> ---------(Windows rant end)
> 
> Either the users know what are they doing, or we don't do anything at
> all. Let's them do it on their free time.
> 
> IOW, Please let users opt in instead of opt out of this features.

But users _do_ opt-in to this feature. They need to start the maintenance
at all to have this run. We just need to give them something that actually
"maintains" their repository without being incredibly expensive or
possibly leading to data loss. That is exactly why "incremental-repack" is
chosen over the "gc" task. If there isn't enough work to be done, then
this task is very cheap to do.

Perhaps all of your concerns are satisfied with this reassurance that
background maintenance is completely opt-in and will not be set up
without a user explicitly enabling it.

Thanks,
-Stolee
Đoàn Trần Công Danh Oct. 5, 2020, 1:16 p.m. UTC | #6
On 2020-10-01 21:55:40-0400, Derrick Stolee <stolee@gmail.com> wrote:
> On 10/1/2020 8:38 PM, Đoàn Trần Công Danh wrote:
> > On 2020-10-01 16:38:48-0400, Derrick Stolee <stolee@gmail.com> wrote:
> >> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
> >> index 7628a6d157..52fff86844 100644
> >> --- a/Documentation/git-maintenance.txt
> >> +++ b/Documentation/git-maintenance.txt
> >> @@ -37,6 +37,21 @@ register::
> >>  	`maintenance.<task>.schedule`. The tasks that are enabled are safe
> >>  	for running in the background without disrupting foreground
> >>  	processes.
> >> ++
> >> +If your repository has no `maintenance.<task>.schedule` configuration
> >> +values set, then Git will use a recommended default schedule that performs
> >> +background maintenance that will not interrupt foreground commands. The
> >> +default schedule is as follows:
> > 
> > I don't mind about using a default schedule (but someone else might).
> > I think some distributions will be paranoia with this change and shiped
> > with disable by default in system config.
> 
> If a user wants to prevent this schedule, then they can simply change
> any one of the `.schedule` or `.enabled` configs in their --global config
> and these defaults will not be used.
> 
> Of course, perhaps you are missing the fact that "git maintenance run
> --schedule=<frequency>" is only run as a cron job if a user chose to
> start background maintenance using "git maintenance start" (or "git
> maintenance register" after running the 'start' subcommand in another
> repo). So this is _not_ starting by default without some amount of
> choosing to opt in.

Yes, I missed that fact. Sorry for the noise I generated.

Thanks,
-- Danh
Derrick Stolee Oct. 5, 2020, 6:17 p.m. UTC | #7
On 10/5/2020 9:16 AM, Đoàn Trần Công Danh wrote:
> On 2020-10-01 21:55:40-0400, Derrick Stolee <stolee@gmail.com> wrote:
>> On 10/1/2020 8:38 PM, Đoàn Trần Công Danh wrote:
>>> I don't mind about using a default schedule (but someone else might).
>>> I think some distributions will be paranoia with this change and shiped
>>> with disable by default in system config.
>>
>> If a user wants to prevent this schedule, then they can simply change
>> any one of the `.schedule` or `.enabled` configs in their --global config
>> and these defaults will not be used.
>>
>> Of course, perhaps you are missing the fact that "git maintenance run
>> --schedule=<frequency>" is only run as a cron job if a user chose to
>> start background maintenance using "git maintenance start" (or "git
>> maintenance register" after running the 'start' subcommand in another
>> repo). So this is _not_ starting by default without some amount of
>> choosing to opt in.
> 
> Yes, I missed that fact. Sorry for the noise I generated.

Thanks for confirming. And thank you for the interest in the
feature! It's my fault for not including the context properly.

Thanks,
-Stolee
diff mbox series

Patch

diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index 7f8c279fe8..364b3e32bf 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -37,6 +37,12 @@  register::
 	`maintenance.<task>.schedule`. The tasks that are enabled are safe
 	for running in the background without disrupting foreground
 	processes.
++
+If your repository has no 'maintenance.<task>.schedule' configuration
+values set, then Git will set configuration values to some recommended
+settings. These settings disable foreground maintenance while performing
+maintenance tasks in the background that will not interrupt foreground Git
+operations.
 
 run::
 	Run one or more maintenance tasks. If one or more `--task` options
diff --git a/builtin/gc.c b/builtin/gc.c
index 4f2d17c0c0..2ef4c0960c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1409,6 +1409,49 @@  static int maintenance_run(int argc, const char **argv, const char *prefix)
 	return maintenance_run_tasks(&opts);
 }
 
+static int has_schedule_config(void)
+{
+	int i, found = 0;
+	struct strbuf config_name = STRBUF_INIT;
+	size_t prefix;
+
+	strbuf_addstr(&config_name, "maintenance.");
+	prefix = config_name.len;
+
+	for (i = 0; !found && i < TASK__COUNT; i++) {
+		char *value;
+
+		strbuf_setlen(&config_name, prefix);
+		strbuf_addf(&config_name, "%s.schedule", tasks[i].name);
+
+		if (!git_config_get_string(config_name.buf, &value)) {
+			found = 1;
+			FREE_AND_NULL(value);
+		}
+	}
+
+	strbuf_release(&config_name);
+	return found;
+}
+
+static void set_recommended_schedule(void)
+{
+	git_config_set("maintenance.auto", "false");
+	git_config_set("maintenance.gc.enabled", "false");
+
+	git_config_set("maintenance.prefetch.enabled", "true");
+	git_config_set("maintenance.prefetch.schedule", "hourly");
+
+	git_config_set("maintenance.commit-graph.enabled", "true");
+	git_config_set("maintenance.commit-graph.schedule", "hourly");
+
+	git_config_set("maintenance.loose-objects.enabled", "true");
+	git_config_set("maintenance.loose-objects.schedule", "daily");
+
+	git_config_set("maintenance.incremental-repack.enabled", "true");
+	git_config_set("maintenance.incremental-repack.schedule", "daily");
+}
+
 static int maintenance_register(void)
 {
 	struct child_process config_set = CHILD_PROCESS_INIT;
@@ -1418,6 +1461,9 @@  static int maintenance_register(void)
 	if (!the_repository || !the_repository->gitdir)
 		return 0;
 
+	if (!has_schedule_config())
+		set_recommended_schedule();
+
 	config_get.git_cmd = 1;
 	strvec_pushl(&config_get.args, "config", "--global", "--get", "maintenance.repo",
 		     the_repository->worktree ? the_repository->worktree
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 8803fcf621..5a31f3925b 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -309,7 +309,23 @@  test_expect_success 'register and unregister' '
 	git config --global --add maintenance.repo /existing1 &&
 	git config --global --add maintenance.repo /existing2 &&
 	git config --global --get-all maintenance.repo >before &&
+
+	# We still have maintenance.<task>.schedule config set,
+	# so this does not update the local schedule
+	git maintenance register &&
+	test_must_fail git config maintenance.auto &&
+
+	# Clear previous maintenance.<task>.schedule values
+	for task in loose-objects commit-graph incremental-repack
+	do
+		git config --unset maintenance.$task.schedule || return 1
+	done &&
 	git maintenance register &&
+	test_cmp_config false maintenance.auto &&
+	test_cmp_config false maintenance.gc.enabled &&
+	test_cmp_config true maintenance.prefetch.enabled &&
+	test_cmp_config hourly maintenance.commit-graph.schedule &&
+	test_cmp_config daily maintenance.incremental-repack.schedule &&
 	git config --global --get-all maintenance.repo >actual &&
 	cp before after &&
 	pwd >>after &&