Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file

From: R. Tyler Ballance <tyler@slide.com>
Date: Jan 8, 2009, 00:49 UTC
Message-ID: <1231375780.8870.629.camel@starfruit>
In-Reply-To: <alpine.LFD.2.00.0901071621340.3283@localhost.localdomain>

On Wed, 2009-01-07 at 16:37 -0800, Linus Torvalds wrote:

Show 21 quoted lines

> 
> On Wed, 7 Jan 2009, Linus Torvalds wrote:
> >
> > >         > limit ~1.5GB -> corrupt file
> > >         > limit ~3GB -> magically no longer corrupt.
> > 
> > That is interesting, although I also worry that there might be other 
> > issues going on (ie since you've reported thigns magically fixing 
> > themselves, maybe the ulimit tests just _happened_ to show that, even if 
> > it wasn't the core reason).
> > 
> > BUT! This is definitely worth looking at.
> > 
> > For example, we do have some cases where we try to do "mmap()", and if it 
> > fails, we try to free some memory and try again. In particular, in 
> > xmmap(), if an mmap() fails - which may be due to running out of virtual 
> > address space - we'll actually try to release some pack-file memory and 
> > try again. Maybe there's a bug there - and it would be one that seldom 
> > triggers for others.
> 
> Ho humm. We really do have some interesting things there.

Always enjoyable when these mail threads get this deep ;)

> 
> Is this a 64-bit machine? I didn't think OS X did that, but if there is 
> some limited 64-bit support there, maybe "sizeof(void *)" is 8, then we 
> default the default git pack-window to a pretty healthy 1GB.

I was only mentioning OS X with regards to the Samba/NFS red herring, the rest of our operations are on 64-bit Linux machines.

The machine I reproduced this on ("Public repo case!") is the following:
        tyler@grapefruit:~> uname -a
        Linux grapefruit.corp.slide.com 2.6.27.7-9-default #1 SMP
        2008-12-04 18:10:04 +0100 x86_64 x86_64 x86_64 GNU/Linux
        tyler@grapefruit:~> cat /etc/issue
        Welcome to openSUSE 11.1   - Kernel \r (\l).
        
The machines we're experiencing this issue on "in the wild" are:
        xdev3 (master)% uname -a 
        Linux xdev3 2.6.24-22-server #1 SMP Mon Nov 24 20:06:28 UTC 2008
        x86_64 GNU/Linux
        xdev3 (master)% cat /etc/issue
        Ubuntu 8.04.1 \n \l

> 
> I could easily see that if you have a virtual memory size limit of 1.5GB, 
> and the pack window size is 1GB, we might have trouble. Because we could 
> only keep one such pack window in memory at a time.

The DEFAULT_PACKED_GIT_WINDOW_SIZE in our local builds is 256M, FWIW

Show 14 quoted lines

> 
> I have _not_ looked at the code, though. I'd have expected a SIGSEGV if we 
> really had issues with the window handling.
> 
> Anyway, _if_ your system has 64-bit pointers, then _maybe_ something the 
> default 1GB pack window causes problem.
> 
> If so, then adding a
> 
> 	[core]
> 		packedgitwindowsize = 64M
> 
> might make a difference. It would certainly be very interesting to hear if 
> there's any impact.

I can try this still if you'd like, but it doesn't seem like that'd be the issue since we're already lowering the window size system-wide

Cheers

-- 
-R. Tyler Ballance
Slide, Inc.