Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Oct 10 2018, Jonathan Nieder wrote:

> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> Perhaps this reporting could also print the message from a previous
>>> run, so you could write:
>>>
>>> 	git gc --detached-status || exit
>>> 	git gc --auto; # perhaps also passing --detach
>>>
>>> (Names still open for bikeshedding.)
>>
>> When the command is given --detached-exit-code/status option, what
>> does it do?  Does it perform the "did an earlier run left gc.log?"
>> and report the result and nothing else?  In other words, is it a
>> pure replacement for "test -e .git/gc.log"?
>
> My intent was the latter.  In other words, in the idiom
>
> 	do_something_async &
> 	... a lot of time passes ...
> 	wait
>
> it is something like the replacement for "wait".
>
> More precisely,
>
> 	git gc --detached-status || exit
>
> would mean something like
>
> 	if test -e .git/gc.log	# Error from previous gc --detach?
> 	then
> 		cat >&2 .git/gc.log	# Report the error.
> 		exit 1
> 	fi
>
>>                                              Or does it do some of
>> the "auto-gc" prep logic like guestimating loose object count and
>> have that also in its exit status (e.g. "from the gc.log left
>> behind, we know that we failed to reduce loose object count down
>> sufficiently after finding there are more than 6700 earlier, but now
>> we do not have that many loose object, so there is nothing to
>> complain about the presence of gc.log")?
>
> Depending on the use case, a user might want to avoid losing
> information about the results of a previous "git gc --detach" run,
> even if they no longer apply.  For example, a user might want to
> collect the error message for monitoring or later log analysis, to
> track down intermittent gc errors that go away on their own.
>
> A separate possible use case might be a
>
> 	git gc --needs-auto-gc
>
> command that detects whether an auto gc is needed.  With that, a
> caller that only wants to learn about errors if auto gc is needed
> could run
>
> 	if git gc --needs-auto-gc
> 	then
> 		git gc --detached-status || exit
> 	fi
>
>> I am bad at naming myself, but worse at guessing what others meant
>> with a new thing that was given a new name whose name is fuzzy,
>> so... ;-)
>
> No problem.  I'm mostly trying to tease out more details about the use
> case.

Likewise, so don't take the following as an assertion of fact, but more
of a fact-finding mission:

We could add something like this --detached-status / --needs-auto-gc,
but I don't need it, and frankly I can't think of a reason for why
anyone would want to use these.

The entire point of having gc --auto in the first place is that you
don't care when exactly GC happens, you're happy with whenever git
decides it's needed.

So why would anyone need a --needs-auto-gc? If your criteria for doing
GC exactly matches that of gc --auto then ... you just run gc --auto, if
it isn't (e.g. if you're using Microsoft's Windows repo) you're not
using gc --auto in the first place, and neither --needs-auto-gc nor
--auto is useful to you.

So maybe I'm missing something here, but a --needs-auto-gc just seems
like a gratuitous exposure of an internal implementation detail whose
only actionable result is doing what we're doing with "gc --auto" now,
i.e. just run gc.

Which is what I'm doing by running "gc --auto" across a set of servers
and looking at the exit code. If it's been failing I get an error, if
there's no need to gc nothing happens, and if it hasn't been failing and
it just so happens that it's time to GC then fine, now was as good a
time as any.

So if we assume that for the sake of argument there's no point in a
--detached-status either. My only reason for ever caring about that
status is when I run "gc --auto" and it says it can't fork() itself so
it fails. Since I'm using "gc --auto" I have zero reason to even ask
that question unless I'm OK with kicking off a gc run as a side-effect,
so why split up the two? It just introduces a race condition for no
benefit.