All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Stefan Beller <sbeller@google.com>,
	git@vger.kernel.org, gitster@pobox.com, loic@dachary.org
Subject: Re: [PATCHv2 1/5] update-ref: test handling large transactions properly
Date: Thu, 22 Jan 2015 08:07:47 -0500	[thread overview]
Message-ID: <20150122130746.GD19681@peff.net> (raw)
In-Reply-To: <54C0D6F9.1000808@alum.mit.edu>

On Thu, Jan 22, 2015 at 11:54:49AM +0100, Michael Haggerty wrote:

> > +run_with_limited_open_files () {
> > +	(ulimit -n 32 && "$@")
> > +}
> 
> Regarding the choice of "32", I wonder what is the worst-case number of
> open file descriptors that are needed *before* counting the ones that
> are currently wasted on open loose-reference locks. On Linux it seems to
> be only 4 with my setup:
> 
>     $ (ulimit -n 3 && git update-ref --stdin </dev/null)
>     bash: /dev/null: Too many open files
>     $ (ulimit -n 4 && git update-ref --stdin </dev/null)
>     $
> 
> This number might depend a little bit on details of the repository, like
> whether config files import other config files. But as long as the
> "background" number of fds required is at least a few less than 32, then
> your number should be safe.
> 
> Does anybody know of a platform where file descriptors are eaten up
> gluttonously by, for example, each shared library that is in use or
> something? That's the only think I can think of that could potentially
> make your choice of 32 problematic.

It's not just choice of platform. There could be inherited descriptors
in the environment (e.g., the test suite is being run by a shell that
keeps a pipe to CI server open, or something). And the test suite itself
uses several extra descriptors for hiding and showing output.

I think this is the sort of thing that we have to determine with a mix
of guessing and empiricism.  4 is almost certainly too low. 32 looks
"pretty big" in practice but not so big that it will make the test slow.
I think our best bet is probably to ship it and see if anybody reports
problems while the patch cooks.  Then we can bump the number (or find a
new approach) as necessary.

-Peff

  reply	other threads:[~2015-01-22 13:07 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-22  2:32 [PATCHv2 0/5] Fix bug in large transactions Stefan Beller
2015-01-22  2:32 ` [PATCHv2 1/5] update-ref: test handling large transactions properly Stefan Beller
2015-01-22 10:54   ` Michael Haggerty
2015-01-22 13:07     ` Jeff King [this message]
2015-01-22  2:32 ` [PATCHv2 2/5] t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE Stefan Beller
2015-01-22  2:32 ` [PATCHv2 3/5] refs.c: remove lock_fd from struct ref_lock Stefan Beller
2015-01-22  2:32 ` [PATCHv2 4/5] refs.c: have a write_sha1_to_lock_file wrapper Stefan Beller
2015-01-22  2:32 ` [PATCHv2 5/5] refs.c: enable large transactions Stefan Beller
2015-01-22 11:24   ` Michael Haggerty
2015-01-22 13:10     ` Jeff King
2015-01-22 16:33       ` Michael Haggerty
2015-01-22 19:24         ` Stefan Beller
2015-01-22 23:11     ` [RFC PATCH 0/5] So you dislike the sequence of system calls? Stefan Beller
2015-01-22 23:11       ` [PATCH 1/5] fixup for "refs.c: enable large transactions" Stefan Beller
2015-01-22 23:11       ` [PATCH 2/5] refs.c: remove unlock_ref from write_ref_sha1 Stefan Beller
2015-01-22 23:11       ` [PATCH 3/5] refs.c: move static functions to close and commit refs Stefan Beller
2015-01-22 23:11       ` [PATCH 4/5] refs.c: remove committing the ref from write_ref_sha1 Stefan Beller
2015-01-22 23:11       ` [PATCH 5/5] refs.c: write values to lock files early for committing Stefan Beller
2015-01-22 12:59   ` [PATCHv2 5/5] refs.c: enable large transactions Ramsay Jones
2015-01-22 19:16     ` Stefan Beller
2015-01-22 19:51       ` Ramsay Jones
2015-01-22 20:13         ` Ramsay Jones
2015-01-22 20:20           ` Stefan Beller
2015-01-22 20:59             ` Ramsay Jones
2015-01-22 12:05 ` [PATCHv2 0/5] Fix bug in " Michael Haggerty
2015-01-23 20:03   ` [PATCHv3 0/6] " Stefan Beller
2015-01-23 20:03     ` [PATCHv3 1/6] update-ref: test handling large transactions properly Stefan Beller
2015-01-23 20:03     ` [PATCHv3 2/6] t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE Stefan Beller
2015-01-23 20:03     ` [PATCHv3 3/6] refs.c: remove lock_fd from struct ref_lock Stefan Beller
2015-01-23 20:04     ` [PATCHv3 4/6] refs.c: move static functions to close and commit refs Stefan Beller
2015-01-23 20:04     ` [PATCHv3 5/6] refs.c: remove unlock_ref and commit_ref from write_ref_sha1 Stefan Beller
2015-01-23 23:57       ` Junio C Hamano
2015-01-24  0:22         ` Stefan Beller
2015-01-24  0:39           ` Junio C Hamano
2015-01-24  1:04             ` Stefan Beller
2015-01-24  1:29               ` Junio C Hamano
2015-01-23 20:04     ` [PATCHv3 6/6] refs.c: enable large transactions Stefan Beller
2015-01-24  0:14       ` Junio C Hamano
2015-01-24  0:24         ` Stefan Beller
2015-01-24  0:38           ` Junio C Hamano
2015-01-26 19:30             ` Stefan Beller
2015-01-26 21:10               ` [PATCH] refs.c: clean up write_ref_sha1 returns Stefan Beller
2015-01-27  3:22                 ` Junio C Hamano
2015-01-28 21:35                   ` Stefan Beller
2015-01-27  3:17               ` [PATCHv3 6/6] refs.c: enable large transactions Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150122130746.GD19681@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=loic@dachary.org \
    --cc=mhagger@alum.mit.edu \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.