From: Derrick Stolee <stolee@gmail.com>
To: Patrick Steinhardt <ps@pks.im>, git@vger.kernel.org
Subject: Re: [PATCH 0/7] builtin/maintenance: fix auto-detach with non-standard tasks
Date: Thu, 15 Aug 2024 10:04:10 -0400 [thread overview]
Message-ID: <de298972-a359-46dd-b808-4edaa8701d40@gmail.com> (raw)
In-Reply-To: <cover.1723533091.git.ps@pks.im>
On 8/13/24 3:17 AM, Patrick Steinhardt wrote:
> I recently configured git-maintenance(1) to not use git-gc(1) anymore,
> but instead to use git-multi-pack-index(1). I quickly noticed that the
> behaviour here is somewhat broken because instead of auto-detaching when
> `git maintenance run --auto` executes, we wait for the process to run to
> completion.
>
> The root cause is that git-maintenance(1), probably by accident,
> continues to rely on the auto-detaching mechanism in git-gc(1). So
> instead of having git-maintenance(1) detach, it is git-gc(1) that
> detaches and thus causes git-maintenance(1) to exit early. That of
> course falls flat once any maintenance task other than git-gc(1)
> executes, because these won't detach.
>
> Despite being a usability issue, this may also cause git-gc(1) to run
> concurrently with any other enabled maintenance tasks. This shouldn't
> lead to data loss, but it can certainly lead to processes stomping on
> each others feet.
>
> This patch series fixes this by wiring up new `--detach` flags for both
> git-gc(1) and git-maintenance(1). Like this, git-maintenance(1) now
> knows to execute `git gc --auto --no-detach`, while our auto-maintenance
> will execute `git mainteance run --auto --detach`.
Thank you for noticing this behavior, which is essentially an unintended
regression from when the maintenance command was first introduced. It
worked for most users because of the accidental detachment of the GC
task, but now users can correctly customize their automatic maintenance
to run in the background.
This was my oversight, as I was focused on scheduled maintenance as
being the primary way that users would customize their maintenance tasks.
Thank you for unifying the concepts.
I sprinkled in commentary, and most of it was just things I noticed
while reading the series in order but then later patches or a careful
read made my comments non-actionable.
This v1 looks good to me.
Thanks,
-Stolee
next prev parent reply other threads:[~2024-08-15 14:04 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-13 7:17 [PATCH 0/7] builtin/maintenance: fix auto-detach with non-standard tasks Patrick Steinhardt
2024-08-13 7:17 ` [PATCH 1/7] config: fix constness of out parameter for `git_config_get_expiry()` Patrick Steinhardt
2024-08-13 7:17 ` [PATCH 2/7] builtin/gc: refactor to read config into structure Patrick Steinhardt
2024-08-15 5:24 ` James Liu
2024-08-15 8:18 ` Patrick Steinhardt
2024-08-15 13:46 ` Derrick Stolee
2024-08-13 7:17 ` [PATCH 3/7] builtin/gc: fix leaking config values Patrick Steinhardt
2024-08-15 5:22 ` James Liu
2024-08-15 8:18 ` Patrick Steinhardt
2024-08-15 13:50 ` Derrick Stolee
2024-08-13 7:17 ` [PATCH 4/7] builtin/gc: stop processing log file on signal Patrick Steinhardt
2024-08-15 6:01 ` James Liu
2024-08-13 7:17 ` [PATCH 5/7] builtin/gc: add a `--detach` flag Patrick Steinhardt
2024-08-13 7:17 ` [PATCH 6/7] builtin/maintenance: " Patrick Steinhardt
2024-08-13 7:18 ` [PATCH 7/7] builtin/maintenance: fix auto-detach with non-standard tasks Patrick Steinhardt
2024-08-13 11:29 ` Phillip Wood
2024-08-13 11:59 ` Patrick Steinhardt
2024-08-13 13:19 ` Phillip Wood
2024-08-14 4:15 ` Patrick Steinhardt
2024-08-14 15:13 ` Phillip Wood
2024-08-15 5:30 ` Patrick Steinhardt
2024-08-15 6:40 ` James Liu
2024-08-15 8:17 ` Patrick Steinhardt
2024-08-15 14:00 ` Derrick Stolee
2024-08-15 6:42 ` [PATCH 0/7] " James Liu
2024-08-15 9:12 ` [PATCH v2 " Patrick Steinhardt
2024-08-15 9:12 ` [PATCH v2 1/7] config: fix constness of out parameter for `git_config_get_expiry()` Patrick Steinhardt
2024-08-15 9:12 ` [PATCH v2 2/7] builtin/gc: refactor to read config into structure Patrick Steinhardt
2024-08-15 9:12 ` [PATCH v2 3/7] builtin/gc: fix leaking config values Patrick Steinhardt
2024-08-15 9:12 ` [PATCH v2 4/7] builtin/gc: stop processing log file on signal Patrick Steinhardt
2024-08-15 9:12 ` [PATCH v2 5/7] builtin/gc: add a `--detach` flag Patrick Steinhardt
2024-08-15 19:11 ` Junio C Hamano
2024-08-15 22:29 ` Junio C Hamano
2024-08-16 8:06 ` Patrick Steinhardt
2024-08-15 9:12 ` [PATCH v2 6/7] builtin/maintenance: " Patrick Steinhardt
2024-08-15 9:12 ` [PATCH v2 7/7] run-command: fix detaching when running auto maintenance Patrick Steinhardt
2024-08-15 16:13 ` Junio C Hamano
2024-08-16 8:06 ` Patrick Steinhardt
2024-08-15 14:04 ` Derrick Stolee [this message]
2024-08-15 15:37 ` [PATCH 0/7] builtin/maintenance: fix auto-detach with non-standard tasks Junio C Hamano
2024-08-16 8:06 ` Patrick Steinhardt
2024-08-16 10:44 ` [PATCH v3 " Patrick Steinhardt
2024-08-16 10:44 ` [PATCH v3 1/7] config: fix constness of out parameter for `git_config_get_expiry()` Patrick Steinhardt
2024-08-16 10:45 ` [PATCH v3 2/7] builtin/gc: refactor to read config into structure Patrick Steinhardt
2024-08-16 10:45 ` [PATCH v3 3/7] builtin/gc: fix leaking config values Patrick Steinhardt
2024-08-16 10:45 ` [PATCH v3 4/7] builtin/gc: stop processing log file on signal Patrick Steinhardt
2024-08-16 10:45 ` [PATCH v3 5/7] builtin/gc: add a `--detach` flag Patrick Steinhardt
2024-08-16 10:45 ` [PATCH v3 6/7] builtin/maintenance: " Patrick Steinhardt
2024-08-17 7:09 ` Jeff King
2024-08-17 7:14 ` Jeff King
2024-08-19 6:17 ` Patrick Steinhardt
2024-08-16 10:45 ` [PATCH v3 7/7] run-command: fix detaching when running auto maintenance Patrick Steinhardt
2024-08-17 12:14 ` Jeff King
2024-08-19 6:17 ` Patrick Steinhardt
2024-08-19 7:47 ` [PATCH 0/3] Fixups for git-maintenance(1) tests Patrick Steinhardt
2024-08-19 7:47 ` [PATCH 1/3] t7900: fix flaky test due to leaking background job Patrick Steinhardt
2024-08-19 8:49 ` Jeff King
2024-08-19 8:55 ` Patrick Steinhardt
2024-08-19 9:12 ` Jeff King
2024-08-19 9:17 ` Patrick Steinhardt
2024-08-19 7:48 ` [PATCH 2/3] t7900: exercise detaching via trace2 regions Patrick Steinhardt
2024-08-19 8:51 ` Jeff King
2024-08-19 8:56 ` Patrick Steinhardt
2024-08-21 18:38 ` Junio C Hamano
2024-08-22 5:41 ` Patrick Steinhardt
2024-08-22 17:22 ` Junio C Hamano
2024-08-19 7:48 ` [PATCH 3/3] builtin/maintenance: fix loose objects task emitting pack hash Patrick Steinhardt
2024-08-19 8:55 ` Jeff King
2024-08-19 9:07 ` Patrick Steinhardt
2024-08-19 9:17 ` Jeff King
2024-08-19 9:26 ` Patrick Steinhardt
2024-08-19 10:26 ` Jeff King
2024-08-20 7:39 ` Patrick Steinhardt
2024-08-20 15:58 ` Junio C Hamano
2024-08-19 17:05 ` Junio C Hamano
2024-08-19 8:46 ` [PATCH v3 7/7] run-command: fix detaching when running auto maintenance Jeff King
2024-08-19 9:04 ` Patrick Steinhardt
2024-08-19 10:49 ` Patrick Steinhardt
2024-08-19 15:41 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=de298972-a359-46dd-b808-4edaa8701d40@gmail.com \
--to=stolee@gmail.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).