Re: RLIMIT_NOFILE fallback

From: Jeff King <peff@peff.net>
Date: Dec 20, 2013, 09:12 UTC
Message-ID: <20131220091232.GA9637@sigill.intra.peff.net>
In-Reply-To: <xmqqmwjwg2ok.fsf@gitster.dls.corp.google.com>

On Thu, Dec 19, 2013 at 09:39:55AM -0800, Junio C Hamano wrote:

Show 16 quoted lines

> Torsten Bögershausen <tboegi@web.de> writes:
> 
> > Thanks for an interesting reading,
> > please allow a side question:
> > Could it be, that "-1 == unlimited" is Linux specific?
> > And therefore not 100% portable ?
> >
> > And doesn't "unlimited" number of files call for trouble,
> > having the risk to starve the machine ?
> >
> > BTW: cygwin returns 256.
> 
> If you look at the caller, you will see that we do cap the value
> returned from this helper function down to a more reasonable and not
> so selfish maximum, exactly for the purpose of avoiding the risk of
> starving other processes.

I am not sure you are reading the capping in the right direction. We do not cap at 25, but rather keep 25 open for "other stuff". So at unlimited, we are consuming a mere UINT_MAX-25 descriptors. :)

I think that 25 is not for the benefit of the rest of the system, but rather for _us_ to avoid running out of descriptors for normal operations. I do not think we need to be careful about starving other processes at all. That is the job of the ulimit in the first place, and we respect it. If the sysadmin turns off the limit, then we are just following their instructions.

In practice, I'd be shocked if git behaved reasonably above about 500 packs anyway, so that puts a practical cap on our fd use. :)

None of that impacts the patch under discussion, though. The only thing I was trying to bring up earlier is that on a system with:

  1. No (or broken) getrlimit

  2. No OPEN_MAX defined

  3. sysconf that works, and returns -1 for unlimited

  4. a sysadmin who has set the descriptor limit to "unlimited"

We will end up at "1". Which is not great, but I am skeptical that a system matching the above 4 constraints actually exists. So I think the patch is fine in practice.

-Peff