Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file
- From
Linus Torvalds <torvalds@linux-foundation.org>
- Date
- Jan 7, 2009, 23:29 UTC
- Message-ID
- <alpine.LFD.2.00.0901071520330.3057@localhost.localdomain>
- In-Reply-To
- <1231368935.8870.584.camel@starfruit>
On Wed, 7 Jan 2009, R. Tyler Ballance wrote:
Show 16 quoted lines
> > > git process - ie a wild pointer, or perhaps a race condition (but we > > don't really use threading in 1.6.0.4 unless you ask for it, and even > > then just for pack-file generation) > > I have a feeling it's something like this, one of our operations guys > did some research while I was looking at code and he came across this: > > On Wed, 2009-01-07 at 14:17 -0800, Ken Brownfield wrote: > git-merge is using too much RAM, and failing to malloc() but > NOT > > reporting it. This is all sorts of bad: > > > > A) using an unscalable amount of RAM > > B) failing to detect malloc() failure > > C) reporting file corruption instead
Well, I dont' think that's exactly it. git internally doesn't really use malloc at all, and uses xmalloc() instead which will die() if the malloc fails. So there's almost certainly no "failing to detect failures"
Yes, there's a few places that don't use the wrapper, but they should be safe (eg either they SIGSEGV, or they are like create_delta_index() and just create a sub-optimal pack with a warning).
HOWEVER:
> > I was able to reproduce this. > > > > 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.
Show 5 quoted lines
> I think you're correct insofar that our major site-specific alteration > has come up on the mailing list before (okay maybe two site-specific > things). > * Our Git repo is ~7.1GB > * ulimit -v is set to ~1.5G
It is certainly possible. It's too bad that it's private, because it makes it _much_ harder to try to pinpoint this.
Linus