Re: [PATCH v2] rerere: keep a background gc from killing a rebase

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Overall, this commit message is rather long and it would be helpful if 
> you could distill it to remove unnecessary and unrelated details.

Hear hear.

>> +rerere.lockTimeout::
>> +	The length of time, in milliseconds, to retry when trying to
>> +	take the rerere lock while another process holds it, typically
>> +	a background `git rerere gc`.  When the time is up, the command
>> +	warns and goes on without rerere.  Value 0 means not to retry
>> +	at all; -1 means to try indefinitely.  Default is 1000 (i.e.,
>> +	retry for 1 second).  `git rerere gc` does not retry, and
>> +	`git rerere`, `git rerere forget` and `git rerere clear` fail
>> +	instead of going on.
>
> Why do those commands fail rather than wait?

Isn't locktimeout about waiting?

After waiting enough, why should it not fail but proceed?

When there is somebody holding the lock, they acquired the lock
exactly because they did not want to see others (including
ourselves) to touch the rerere database until they are done.

The description "`git rerere gc` does not retry" is highly
questionable.  None of the others retries, either.

What makes `git rerere gc` different among all is not that it does
not retry.  It just does not insist doing a GC and instead leaves
without doing anything (and without failing).

I think this is justifyable as the actions visible to end-users of
"rerere gc" is a vague "discard old enough crufts to gain the
diskspace back" (as opposed to "I know this particular entry is old
enough and I want to see it gone right now").

Compared to that, with "git rerere forget", the end-user explicitly
says "I know the specific rerere entry i just saw reused is *wrong*
and I want to get rid of it".  If another process holding the lock
prevents it from being carried out, I'd prefer to see it fail loudly
and let me know that the entry I wanted to remove is still there (so
if I retried the same merge, I'll see the same mistaken resolution).

>> +		if (fd < 0) {
>> +			warning_errno(_("skipping rerere, unable to create '%s.lock'"),
>> +				      git_path_merge_rr(r));
>
> A background job that the user did not explicitly start printing to the 
> terminal is rather confusing as it is likely to get mixed in with the 
> output of whatever is running in the foreground.

Very good point.

Thanks.