@matheusbsbs / matheus-bagliano / commits / 98077d06b2

run-command: fix detaching when running auto maintenance

In the past, we used to execute `git gc --auto` as part of our automatic housekeeping routines. As git-gc(1) may require quite some time to perform the housekeeping, it knows to detach itself and run in the background so that the user can continue their work. Eventually, we refactored our automatic housekeeping to instead use the more flexible git-maintenance(1) command. The upside of this new infra is that the user can configure which maintenance tasks are performed, at least to a certain degree. So while it continues to run git-gc(1) by default, it can also be adapted to e.g. use git-multi-pack-index(1) for maintenance of the object database. The auto-detach of the new infra is somewhat broken though once the user configures non-standard tasks. The problem is essentially that we detach at the wrong level in the process hierarchy: git-maintenance(1) never detaches itself, but instead it continues to be git-gc(1) which does. When configured to only run the git-gc(1) maintenance task, then the result is basically the same as before. But when configured to run other tasks, then git-maintenance(1) will wait for these to run to completion. Even worse, it may be that git-gc(1) runs concurrently with other housekeeping tasks, stomping on each others feet. Fix this bug by asking git-gc(1) to not detach when it is being invoked via git-maintenance(1). Instead, git-maintenance(1) now respects a new config "maintenance.autoDetach", the equivalent of "gc.autoDetach", and detaches itself into the background when running as part of our auto maintenance. This should continue to behave the same for all users which use the git-gc(1) task, only. For others though, it means that we now properly perform all tasks in the background. The default behaviour of git-maintenance(1) when executed by the user does not change, it will remain in the foreground unless they pass the `--detach` option. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Aug 16, 2024 at 12:45 UTC 98077d06b28b97d508c389886ee5014056707a5e
6 files changed +62 -14
Documentation/config/gc.txt
+2 -1
@@ -40,7 +40,8 @@ use, it'll affect how the auto pack limit works.
40
41 gc.autoDetach::
42 Make `git gc --auto` return immediately and run in the background
43 - if the system supports it. Default is true.
43 + if the system supports it. Default is true. This config variable acts
44 + as a fallback in case `maintenance.autoDetach` is not set.
45
46 gc.bigPackThreshold::
47 If non-zero, all non-cruft packs larger than this limit are kept
Documentation/config/maintenance.txt
+11
@@ -3,6 +3,17 @@ maintenance.auto::
3 `git maintenance run --auto` after doing their normal work. Defaults
4 to true.
5
6 +maintenance.autoDetach::
7 + Many Git commands trigger automatic maintenance after they have
8 + written data into the repository. This boolean config option
9 + controls whether this automatic maintenance shall happen in the
10 + foreground or whether the maintenance process shall detach and
11 + continue to run in the background.
12 ++
13 +If unset, the value of `gc.autoDetach` is used as a fallback. Defaults
14 +to true if both are unset, meaning that the maintenance process will
15 +detach.
16 +
17 maintenance.strategy::
18 This string config option provides a way to specify one of a few
19 recommended schedules for background maintenance. This only affects
builtin/gc.c
+1
@@ -1063,6 +1063,7 @@ static int maintenance_task_gc(struct maintenance_run_opts *opts,
1063 strvec_push(&child.args, "--quiet");
1064 else
1065 strvec_push(&child.args, "--no-quiet");
1066 + strvec_push(&child.args, "--no-detach");
1067
1068 return run_command(&child);
1069 }
run-command.c
+11 -1
@@ -1808,16 +1808,26 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
1808
1809 int prepare_auto_maintenance(int quiet, struct child_process *maint)
1810 {
1811 - int enabled;
1811 + int enabled, auto_detach;
1812
1813 if (!git_config_get_bool("maintenance.auto", &enabled) &&
1814 !enabled)
1815 return 0;
1816
1817 + /*
1818 + * When `maintenance.autoDetach` isn't set, then we fall back to
1819 + * honoring `gc.autoDetach`. This is somewhat weird, but required to
1820 + * retain behaviour from when we used to run git-gc(1) here.
1821 + */
1822 + if (git_config_get_bool("maintenance.autodetach", &auto_detach) &&
1823 + git_config_get_bool("gc.autodetach", &auto_detach))
1824 + auto_detach = 1;
1825 +
1826 maint->git_cmd = 1;
1827 maint->close_object_store = 1;
1828 strvec_pushl(&maint->args, "maintenance", "run", "--auto", NULL);
1829 strvec_push(&maint->args, quiet ? "--quiet" : "--no-quiet");
1830 + strvec_push(&maint->args, auto_detach ? "--detach" : "--no-detach");
1831
1832 return 1;
1833 }
t/t5616-partial-clone.sh
+3 -3
@@ -229,7 +229,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
229
230 GIT_TRACE2_EVENT="$PWD/trace1.event" \
231 git -C pc1 fetch --refetch origin &&
232 - test_subcommand git maintenance run --auto --no-quiet <trace1.event &&
232 + test_subcommand git maintenance run --auto --no-quiet --detach <trace1.event &&
233 grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace1.event &&
234 grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace1.event &&
235
@@ -238,7 +238,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
238 -c gc.autoPackLimit=0 \
239 -c maintenance.incremental-repack.auto=1234 \
240 -C pc1 fetch --refetch origin &&
241 - test_subcommand git maintenance run --auto --no-quiet <trace2.event &&
241 + test_subcommand git maintenance run --auto --no-quiet --detach <trace2.event &&
242 grep \"param\":\"gc.autopacklimit\",\"value\":\"0\" trace2.event &&
243 grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace2.event &&
244
@@ -247,7 +247,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
247 -c gc.autoPackLimit=1234 \
248 -c maintenance.incremental-repack.auto=0 \
249 -C pc1 fetch --refetch origin &&
250 - test_subcommand git maintenance run --auto --no-quiet <trace3.event &&
250 + test_subcommand git maintenance run --auto --no-quiet --detach <trace3.event &&
251 grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace3.event &&
252 grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"0\" trace3.event
253 '
t/t7900-maintenance.sh
+34 -9
@@ -49,22 +49,47 @@ test_expect_success 'run [--auto|--quiet]' '
49 git maintenance run --auto 2>/dev/null &&
50 GIT_TRACE2_EVENT="$(pwd)/run-no-quiet.txt" \
51 git maintenance run --no-quiet 2>/dev/null &&
52 - test_subcommand git gc --quiet <run-no-auto.txt &&
53 - test_subcommand ! git gc --auto --quiet <run-auto.txt &&
54 - test_subcommand git gc --no-quiet <run-no-quiet.txt
52 + test_subcommand git gc --quiet --no-detach <run-no-auto.txt &&
53 + test_subcommand ! git gc --auto --quiet --no-detach <run-auto.txt &&
54 + test_subcommand git gc --no-quiet --no-detach <run-no-quiet.txt
55 '
56
57 test_expect_success 'maintenance.auto config option' '
58 GIT_TRACE2_EVENT="$(pwd)/default" git commit --quiet --allow-empty -m 1 &&
59 - test_subcommand git maintenance run --auto --quiet <default &&
59 + test_subcommand git maintenance run --auto --quiet --detach <default &&
60 GIT_TRACE2_EVENT="$(pwd)/true" \
61 git -c maintenance.auto=true \
62 commit --quiet --allow-empty -m 2 &&
63 - test_subcommand git maintenance run --auto --quiet <true &&
63 + test_subcommand git maintenance run --auto --quiet --detach <true &&
64 GIT_TRACE2_EVENT="$(pwd)/false" \
65 git -c maintenance.auto=false \
66 commit --quiet --allow-empty -m 3 &&
67 - test_subcommand ! git maintenance run --auto --quiet <false
67 + test_subcommand ! git maintenance run --auto --quiet --detach <false
68 +'
69 +
70 +for cfg in maintenance.autoDetach gc.autoDetach
71 +do
72 + test_expect_success "$cfg=true config option" '
73 + test_when_finished "rm -f trace" &&
74 + test_config $cfg true &&
75 + GIT_TRACE2_EVENT="$(pwd)/trace" git commit --quiet --allow-empty -m 1 &&
76 + test_subcommand git maintenance run --auto --quiet --detach <trace
77 + '
78 +
79 + test_expect_success "$cfg=false config option" '
80 + test_when_finished "rm -f trace" &&
81 + test_config $cfg false &&
82 + GIT_TRACE2_EVENT="$(pwd)/trace" git commit --quiet --allow-empty -m 1 &&
83 + test_subcommand git maintenance run --auto --quiet --no-detach <trace
84 + '
85 +done
86 +
87 +test_expect_success "maintenance.autoDetach overrides gc.autoDetach" '
88 + test_when_finished "rm -f trace" &&
89 + test_config maintenance.autoDetach false &&
90 + test_config gc.autoDetach true &&
91 + GIT_TRACE2_EVENT="$(pwd)/trace" git commit --quiet --allow-empty -m 1 &&
92 + test_subcommand git maintenance run --auto --quiet --no-detach <trace
93 '
94
95 test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' '
@@ -129,9 +154,9 @@ test_expect_success 'run --task=<task>' '
154 git maintenance run --task=commit-graph 2>/dev/null &&
155 GIT_TRACE2_EVENT="$(pwd)/run-both.txt" \
156 git maintenance run --task=commit-graph --task=gc 2>/dev/null &&
132 - test_subcommand ! git gc --quiet <run-commit-graph.txt &&
133 - test_subcommand git gc --quiet <run-gc.txt &&
134 - test_subcommand git gc --quiet <run-both.txt &&
157 + test_subcommand ! git gc --quiet --no-detach <run-commit-graph.txt &&
158 + test_subcommand git gc --quiet --no-detach <run-gc.txt &&
159 + test_subcommand git gc --quiet --no-detach <run-both.txt &&
160 test_subcommand git commit-graph write --split --reachable --no-progress <run-commit-graph.txt &&
161 test_subcommand ! git commit-graph write --split --reachable --no-progress <run-gc.txt &&
162 test_subcommand git commit-graph write --split --reachable --no-progress <run-both.txt