Re: [PATCH v4] gc: reject if another gc is running, unless --force is given

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

Andres Perera <andres.p@zoho.com> writes:

>> +/* return NULL on success, else hostname running the gc */
>> +static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
>> +{
>> +       static struct lock_file lock;
>> +       static char locking_host[128];
>> +       char my_host[128];
>> +       struct strbuf sb = STRBUF_INIT;
>> +       struct stat st;
>> +       uintmax_t pid;
>
> pid_t is always an signed type, therefore unintmax_t does not make
> sense as a catch all value

Good eyes.

>> +                       !strcmp(locking_host, my_host) &&
>> +                       !kill(pid, 0);
>> +               if (fp != NULL)
>> +                       fclose(fp);
>> +               if (should_exit) {
>> +                       if (fd >= 0)
>> +                               rollback_lock_file(&lock);
>> +                       *ret_pid = pid;
>> +                       return locking_host;
>
> why not exponential backoff?


If the other guy is doing a GC, and we decide that we should exit,
it is *not* because we want to wait until the other guy is done.  It
is because we know we do not have to do the work --- the other guy
is doing what we were about to do, and it will do it for us anyway.

So I do not think it makes any sense to do exponential backoff if
"gc --auto" is asking "should we exit" to this logic.

An explicit "gc", on the other hand, may benefit from backoff, but
then the user can choose to do so himself, and more importantly, the
user can see "ah, another one is running so enough cruft will be
cleaned up anyway" and choose not to run it.