Subject: Re: [PATCH] refs.c: enable large transactions

From: Junio C Hamano <[email-masked]>
Date: Tue, 21 Apr 2015 10:16:26 -0700

Stefan Beller <[email-masked]> writes:

> +	/*
> +	 * 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() - 32;

Can this go negative?  If it does so, does it matter?  I think the
code doesn't barf, but starting from a negative "remaining" feels
cryptic, compared to starting from a zero "remaining".

>  	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);
> +		}

Any plan to add more code to these blocks in future updates?

Thanks.

> 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