> -----Original Message----- > From: Jeff King [mailto:p...@peff.net] > Sent: Tuesday, April 18, 2017 1:50 PM > To: David Turner <david.tur...@twosigma.com> > Cc: git@vger.kernel.org; christian.cou...@gmail.com; mf...@codeaurora.org; > jacob.kel...@gmail.com > Subject: Re: [PATCH] repack: respect gc.pid lock > > On Tue, Apr 18, 2017 at 05:43:29PM +0000, David Turner wrote: > > > > A lock can catch the racy cases where both run at the same time. But > > > I think that > > > even: > > > > > > git -c repack.writeBitmaps=true repack -Ad > > > [...wait...] > > > git gc > > > > > > is questionable, because that gc will erase your bitmaps. How does > > > git-gc know that it's doing a bad thing by repacking without > > > bitmaps, and that you didn't simply change your configuration or want to > > > get > rid of them? > > > > Sorry, the gc in Gitlab does keep bitmaps. The one I quoted in a > > previous message doesn't, because the person typing the command was > > just doing some manual testing and I guess didn't realize that > > bitmaps were important. Or perhaps he knew that repack.writeBitmaps was > already set in the config. > > Sure, but I guess I'd just wonder what _else_ is different between the > commands > (and if nothing, why are both running).
Presumably, repack is faster, and they're not intended to run concurrently (but there's a Gitlab bug causing them to do so). But you'll have to ask the Gitlab folks for more details. > > So given that the lock will catch the races, might it be a good idea > > (if Implemented to avoid locking on repack -d)? > > I'm mildly negative just because it increases complexity, and I don't think > it's > actually buying very much. It's not clear to me which invocations of repack > would want to lock and which ones wouldn't. > > Is "-a" or "-A" the key factor? Are there current callers who prefer the > current > behavior of "possibly duplicate some work, but never report failure" versus > "do > not duplicate work, but sometimes fail due to lock contention"? One problem with failing is that it can leave a temp pack behind. I think the correct fix is to change the default code.packedGitLimit on 64-bit machines to 32 terabytes (2**45 bytes). That's because on modern Intel processors, there are 48 bits of address space actually available, but the kernel is going to probably reserve a few bits. My machine claims to have 2**46 bytes of virtual address space available. It's also several times bigger than any repo that I know of or can easily imagine. Does that seem reasonable to you?
