Re: [PATCH] Wrap inflateInit to retry allocation after releasing pack memory

["R. Tyler Ballance" <tyler@slide.com>]

[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]

On Thu, 2009-01-08 at 12:22 -0800, Linus Torvalds wrote:
> 
> On Thu, 8 Jan 2009, R. Tyler Ballance wrote:
> > > 
> > > Tyler - does this make the corruption errors go away, and be replaced by 
> > > hard failures with "out of memory" reporting?
> > 
> > Yeah, looks like it:
> 
> Well, I was hoping that you'd have a confirmation from your own huge repo, 
> but I do suspect it's all the same thing, so I guess this counts as 
> confirmation too.

I never got a real solid "consistent" reproduction case with our
repository, just a lot of users that experienced the issue. I think the
Linux repro case is a far better example, and yeah, it's sorta
confirmation (waiting for operations here to deploy the patched 1.6.1 to
dev machines).

> 
> > > This patch is potentially pretty noisy, on purpose. I didn't remove the 
> > > reporting from places that already do so - some of them have stricter 
> > > errors than this.
> > 
> > I'm assuming this patch is going to be reworked, if so, I'll back it out
> > of our internal 1.6.1 build and anxiously await The Real Deal(tm)
> 
> Oh, it shouldn't be any noisier under _normal_ load - it's more that 
> certain real corruption cases will now report the error twice. That said, 
> the new errors should actually be more informative than the old ones, so 
> even that isn't necessarily all bad.
> 
> Junio - I think we should apply this, and likely to the stable branch too. 
> Add the re-trying the inflateInit() after shrinking pack windows on top of 
> it.

I really appreciate this guys, this is one of the longer threads I've
participated (spanning over a month) and I'm glad you guys were finally
able to track the issue down.

From now moving forward, I'll try to get a reproduction case with the
kernel tree or something equally big since I know it's frustrating to
play the game of telephone with a proprietary code base ("try this? what
does that do? okay, then this?").

Linus, I'll have a chance to look at your comments on my "variable
packed git window size" patch this weekend, and I'll follow-up in the
appropriate thread.


I'm relatively certain that after this witch hunt, I can get Slide to
cover a round of beers at LinuxWorld or the nearest GitTogether ;)


Cheers
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]