All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com, mhagger@alum.mit.edu
Cc: git@vger.kernel.org, Stefan Beller <sbeller@google.com>
Subject: [PATCH 3/3] refs.c: enable large transactions
Date: Thu, 16 Apr 2015 16:17:39 -0700	[thread overview]
Message-ID: <1429226259-21622-4-git-send-email-sbeller@google.com> (raw)
In-Reply-To: <1429226259-21622-1-git-send-email-sbeller@google.com>

This is another attempt on enabling large transactions
(large in terms of open file descriptors). We keep track of how many
lock files are opened by the ref_transaction_commit function.
When more than a reasonable amount of files is open, we close
the file descriptors to make sure the transaction can continue.

Another idea I had during implementing this was to move this file
closing into the lock file API, such that only a certain amount of
lock files can be open at any given point in time and we'd be 'garbage
collecting' open fds when necessary in any relevant call to the lock
file API. This would have brought the advantage of having such
functionality available in other users of the lock file API as well.
The downside however is the over complication, you really need to always
check for (lock->fd != -1) all the time, which may slow down other parts
of the code, which did not ask for such a feature.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 refs.c                | 13 +++++++++++++
 t/t1400-update-ref.sh |  4 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 4f495bd..44e0b15 100644
--- a/refs.c
+++ b/refs.c
@@ -3041,6 +3041,8 @@ static int write_ref_sha1(struct ref_lock *lock,
 		errno = EINVAL;
 		return -1;
 	}
+	if (lock->lk->fd == -1)
+		reopen_lock_file(lock->lk);
 	if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
 	    write_in_full(lock->lk->fd, &term, 1) != 1 ||
 	    close_ref(lock) < 0) {
@@ -3719,6 +3721,12 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 {
 	int ret = 0, i;
 	int n = transaction->nr;
+	/*
+	 * We may want to open many files in a large transaction, so come up with
+	 * a reasonable maximum, keep some spares for stdin/out and other open
+	 * files.
+	 */
+	int remaining_fds = get_max_fd_limit() - 8;
 	struct ref_update **updates = transaction->updates;
 	struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
 	struct string_list_item *ref_to_delete;
@@ -3762,6 +3770,11 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 				    update->refname);
 			goto cleanup;
 		}
+		if (remaining_fds > 0) {
+			remaining_fds--;
+		} else {
+			close_lock_file(update->lock->lk);
+		}
 	}
 
 	/* Perform updates first so live commits remain referenced */
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7a69f1a..636d3a1 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1071,7 +1071,7 @@ run_with_limited_open_files () {
 
 test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
 
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches does not burst open file limit' '
 (
 	for i in $(test_seq 33)
 	do
@@ -1082,7 +1082,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches
 )
 '
 
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting branches does not burst open file limit' '
 (
 	for i in $(test_seq 33)
 	do
-- 
2.3.0.81.gc37f363

  parent reply	other threads:[~2015-04-16 23:18 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-16 23:17 [PATCH 0/3] Another approach to large transactions Stefan Beller
2015-04-16 23:17 ` [PATCH 1/3] refs.c: remove lock_fd from struct ref_lock Stefan Beller
2015-04-16 23:17 ` [PATCH 2/3] Move get_max_fd_limit(void) to git_compat_util.h Stefan Beller
2015-04-16 23:17 ` Stefan Beller [this message]
2015-04-17 17:09 ` [PATCH 0/3] Another approach to large transactions Junio C Hamano
2015-04-17 22:12   ` Junio C Hamano
2015-04-17 22:17     ` Stefan Beller
2015-04-17 23:31       ` Stefan Beller
2015-04-20 22:26         ` Stefan Beller
2015-04-20 22:51           ` Junio C Hamano
2015-04-20 23:07             ` Stefan Beller
2015-04-21  0:31               ` Stefan Beller
2015-04-21  0:35                 ` [PATCH] refs.c: enable " Stefan Beller
2015-04-21 17:16                   ` Junio C Hamano
2015-04-21 17:24                     ` Stefan Beller
2015-04-21 18:00                       ` Junio C Hamano
2015-04-21 19:06                         ` [PATCHv2] " Stefan Beller
2015-04-21 19:56                           ` Stefan Beller
2015-04-22 14:11                           ` Michael Haggerty
2015-04-22 19:09                             ` Stefan Beller
2015-04-22 20:12                               ` Michael Haggerty
2015-04-21 17:22                   ` [PATCH] " Junio C Hamano
2015-04-21 23:21                 ` [PATCH 0/3] Another approach to " Jeff King
2015-04-22 19:14                   ` Stefan Beller
2015-04-22 20:11                     ` Jeff King
2015-04-21 17:19               ` Junio C Hamano
2015-04-21 17:31                 ` Stefan Beller
2015-04-21 12:37             ` Michael Haggerty

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=1429226259-21622-4-git-send-email-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    /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.