Re: Diagnosing stray/stale .keep files -- explore what is in a pack?

From: Jeff King <peff@peff.net>
Date: Jan 21, 2014, 05:19 UTC
Message-ID: <20140121051956.GA5878@sigill.intra.peff.net>
In-Reply-To: <CACPiFC+koU1Fan+tbE2YgOstWGsDtDihpK-7CMOct7XAEpwJ2A@mail.gmail.com>

On Wed, Jan 15, 2014 at 06:50:33PM -0500, Martin Langhoff wrote:

Show 7 quoted lines

> On Wed, Jan 15, 2014 at 12:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > As long as we can reliably determine that it is safe to do so
> > without risking races, automatically cleaning .lock files is a good
> > thing to do.
> 
> If the .lock file is a day old, it seems to me that it should be safe
> to call it stale.

Probably. The way our "lease" system works, nobody should be holding a ref lock for more than a few milliseconds.

That being said, we do lock other things, like the index. Generally I think the index lock should be quick, too. And similar for config file rewrites, and shallow files. And rerere files, it looks like. My, "git grep commit_lock_file" turns up a lot of hits. :)

So I think all of the existing uses are fine, and I suppose that most new cases should be fine, too, because git processes tend not to last a long time.

You asked earlier if I had a script for cleaning locks. No code worth sharing, but I'll give an outline of what we do at GitHub. We basically do:

  find -name *.lock -mmin +60 | xargs rm

I.e., we give only an hour. For keep files, we give a day (since things like hooks may run for a while under the lock, though a day is probably excessive). And we check that it begins with "^receive-pack".

As far as I know, neither of these has ever caused any problems. Of course, any problems might not be immediately obvious.

> Can anyone "take the lock" if there is already a lock file?

Git never takes an existing lock. It expects you to clean it up yourself.

Show 6 quoted lines

> For the keep files, I already drafted a script that looks inside the
> keep file, if it reads 'receive-pack [pid] [host]' it checks whether
> the hostname matches, and if so whether the pid matches a running
> process.
> 
> Only if the host matches and the pid is dead we call it stale.

That sounds reasonable.

> Seems fairly conservative to me. Are there scenarios where we think
> this can misfire?

I cannot think of any.

-Peff