[BUG] detached auto-gc does not respect lock for 'reflog expire', was Re: Flurries of 'git reflog expire'
- Date: Tue, 11 Jul 2017 03:25:37 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: [BUG] detached auto-gc does not respect lock for 'reflog expire', was Re: Flurries of 'git reflog expire'
[Updating the subject since I think this really is a bug].
On Tue, Jul 11, 2017 at 06:45:53AM +0200, Andreas Krey wrote:
> > I also want to add that Bitbucket Server 5.x includes totally
> > rewritten GC handling. 5.0.x automatically disables auto GC in all
> > repositories and manages it explicitly, and 5.1.x fully removes use of
> > "git gc" in favor of running relevant plumbing commands directly.
> That's the part that irks me. This shouldn't be necessary - git itself
> should make sure auto GC isn't run in parallel. Now I probably can't
> evaluate whether a git upgrade would fix this, but given that you
> are going the do-gc-ourselves route I suppose it wouldn't.
It's _supposed_ to take a lock, even in older versions. See 64a99eb47
(gc: reject if another gc is running, unless --force is given,
But it looks like before we take that lock, we sometimes run pack-refs
and reflog expire. This is due to 62aad1849 (gc --auto: do not lock refs
in the background, 2014-05-25). IMHO this is buggy; it should be
checking the lock before calling gc_before_repack() and daemonizing.
Annoyingly, the lock code interacts badly with daemonizing because that
latter will fork to a new process. So the simple solution like:
diff --git a/builtin/gc.c b/builtin/gc.c
index 2ba50a287..79480124a 100644
@@ -414,6 +414,9 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
+ if (lock_repo_for_gc(force, &pid))
+ return 0;
means that anybody looking at the lockfile will report the wrong pid
(and thus think the lock is invalid). I guess we'd need to update it in
place after daemonizing.