Re: [PATCH 0/5] Avoid file descriptor exhaustion in ref_transaction_commit()

[Stefan Beller <sbeller@google.com>]

On Fri, Apr 24, 2015 at 10:26 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Apr 24, 2015 at 01:35:44PM +0200, Michael Haggerty wrote:
>
>> In ref_transaction_commit(), close the reference lockfiles immediately
>> to avoid keeping too many file descriptors open at a time. This is
>> pretty easy, because in the first loop (where we create the locks) we
>> already know what, if anything, has to be written into the lockfile.
>> So write it and close the lockfile immediately. In the second loop,
>> rename the lockfiles for reference updates into place, and in the
>> cleanup loop, unlock any references that are still locked (i.e., those
>> that were only being verified or deleted).
>>
>> I think this is a cleaner solution than Stefan's approach [1] of
>> closing and reopening fds based on an estimate of how many fds we can
>> afford to waste--we only need a single file descriptor at a time, and
>> we never have to close then reopen a lockfile. But it is a bit more
>> intrusive, so it might still be preferable to use Stefan's approach
>> for release 2.4.0, if indeed any fix for this problem is still being
>> considered for that release.
>
> I like this approach much better. It seems like the best of all worlds
> (same performance, and we don't have to worry about whether and when to
> close lockfiles).

I would have guessed this approach to take more work to do it right.
Thanks Michael for tackling the problem in an elegant way!

>
> Stefan's patch is just in pu at this point, right? I do not think there
> is any rushing/release concern.

Yeah it's in pu, so it's easy to remove.

> It is too late for either to be in
> v2.4.0, so the only decision is whether to aim for "master" or "maint".
> To me, they both seem to be in the same ballpark as far as risking a
> regression.
>
> -Peff


Thanks,
Stefan