git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Brandon Casey <bcasey@nvidia.com>
To: <git@vger.kernel.org>
Cc: <gitster@pobox.com>, <spearce@spearce.org>,
	Brandon Casey <drafnel@gmail.com>
Subject: [PATCH] sha1_file: introduce close_one_pack() to close packs on fd pressure
Date: Mon, 29 Jul 2013 21:05:13 -0700	[thread overview]
Message-ID: <1375157113-608-1-git-send-email-bcasey@nvidia.com> (raw)

From: Brandon Casey <drafnel@gmail.com>

When the number of open packs exceeds pack_max_fds, unuse_one_window()
is called repeatedly to attempt to release the least-recently-used
pack windows, which, as a side-effect, will also close a pack file
after closing its last open window.  If a pack file has been opened,
but no windows have been allocated into it, it will never be selected
by unuse_one_window() and hence its file descriptor will not be
closed.  When this happens, git may exceed the number of file
descriptors permitted by the system.

This latter situation can occur in show-ref or receive-pack during ref
advertisement.  During ref advertisement, receive-pack will iterate
over every ref in the repository and advertise it to the client after
ensuring that the ref exists in the local repository.  If the ref is
located inside a pack, then the pack is opened to ensure that it
exists, but since the object is not actually read from the pack, no
mmap windows are allocated.  When the number of open packs exceeds
pack_max_fds, unuse_one_window() will not able to find any windows to
free and will not be able to close any packs.  Once the per-process
file descriptor limit is exceeded, receive-pack will produce a warning,
not an error, for each pack it cannot open, and will then most likely
fail with an error to spawn rev-list or index-pack like:

   error: cannot create standard input pipe for rev-list: Too many open files
   error: Could not run 'git rev-list'

This is not likely to occur during upload-pack since upload-pack
reads each object from the pack so that it can peel tags and
advertise the exposed object.  So during upload-pack, mmap windows
will be allocated for each pack that is opened and unuse_one_window()
will eventually be able to close unused packs after freeing all of
their windows.

When we have file descriptor pressure, in contrast to memory pressure,
we need to free all windows and close the pack file descriptor so that
a new pack can be opened.  Let's introduce a new function
close_one_pack() designed specifically for this purpose to search
for and close the least-recently-used pack, where LRU is defined as

   * pack with oldest mtime and no allocated mmap windows or
   * pack with the least-recently-used windows, i.e. the pack
     with the oldest most-recently-used window

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 sha1_file.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index 8e27db1..7731ab1 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -682,6 +682,67 @@ void close_pack_windows(struct packed_git *p)
 	}
 }
 
+/*
+ * The LRU pack is the one with the oldest MRU window or the oldest mtime
+ * if it has no windows allocated.
+ */
+static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p, struct pack_window **mru_w)
+{
+	struct pack_window *w, *this_mru_w;
+
+	/*
+	 * Reject this pack if it has windows and the previously selected
+	 * one does not.  If this pack does not have windows, reject
+	 * it if the pack file is newer than the previously selected one.
+	 */
+	if (*lru_p && !*mru_w && (p->windows || p->mtime > (*lru_p)->mtime))
+		return;
+
+	for (w = this_mru_w = p->windows; w; w = w->next) {
+		/* Reject this pack if any of its windows are in use */
+		if (w->inuse_cnt)
+			return;
+		/*
+		 * Reject this pack if it has windows that have been
+		 * used more recently than the previously selected pack.
+		 */
+		if (*mru_w && w->last_used > (*mru_w)->last_used)
+			return;
+		if (w->last_used > this_mru_w->last_used)
+			this_mru_w = w;
+	}
+
+	/*
+	 * Select this pack.
+	 */
+	*mru_w = this_mru_w;
+	*lru_p = p;
+}
+
+static int close_one_pack(void)
+{
+	struct packed_git *p, *lru_p = NULL;
+	struct pack_window *mru_w = NULL;
+
+	for (p = packed_git; p; p = p->next) {
+		if (p->pack_fd == -1)
+			continue;
+		find_lru_pack(p, &lru_p, &mru_w);
+	}
+
+	if (lru_p) {
+		close_pack_windows(lru_p);
+		close(lru_p->pack_fd);
+		pack_open_fds--;
+		lru_p->pack_fd = -1;
+		if (lru_p == last_found_pack)
+			last_found_pack = NULL;
+		return 1;
+	}
+
+	return 0;
+}
+
 void unuse_pack(struct pack_window **w_cursor)
 {
 	struct pack_window *w = *w_cursor;
@@ -777,7 +838,7 @@ static int open_packed_git_1(struct packed_git *p)
 			pack_max_fds = 1;
 	}
 
-	while (pack_max_fds <= pack_open_fds && unuse_one_window(NULL, -1))
+	while (pack_max_fds <= pack_open_fds && close_one_pack())
 		; /* nothing */
 
 	p->pack_fd = git_open_noatime(p->pack_name);
-- 
1.8.3.1.440.gc2bf105


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

             reply	other threads:[~2013-07-30  4:05 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-30  4:05 Brandon Casey [this message]
2013-07-30  7:51 ` [PATCH] sha1_file: introduce close_one_pack() to close packs on fd pressure Eric Sunshine
2013-07-30 15:39 ` Junio C Hamano
2013-07-30 19:52   ` Jeff King
2013-07-30 22:59     ` Brandon Casey
2013-07-31 19:51       ` [PATCH v2 1/2] " Brandon Casey
2013-07-31 19:51         ` [PATCH 2/2] Don't close pack fd when free'ing pack windows Brandon Casey
2013-07-31 21:08           ` Antoine Pelisse
2013-07-31 21:21             ` Fredrik Gustafsson
2013-07-31 21:31               ` Brandon Casey
2013-07-31 21:44                 ` Fredrik Gustafsson
2013-07-31 21:23             ` Brandon Casey
2013-07-31 21:28             ` Thomas Rast
2013-08-01 17:12         ` [PATCH v2 1/2] sha1_file: introduce close_one_pack() to close packs on fd pressure Junio C Hamano
2013-08-01 18:01           ` Brandon Casey
2013-08-01 18:39             ` Junio C Hamano
2013-08-01 19:16               ` Brandon Casey
2013-08-01 19:23                 ` Brandon Casey
2013-08-01 20:02                 ` Junio C Hamano
2013-08-01 20:37                   ` Brandon Casey
2013-08-02  5:36                     ` [PATCH v3] " Brandon Casey
2013-08-02 16:26                       ` Junio C Hamano
2013-08-02 17:12                         ` Brandon Casey

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=1375157113-608-1-git-send-email-bcasey@nvidia.com \
    --to=bcasey@nvidia.com \
    --cc=drafnel@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=spearce@spearce.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).