git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] fetch-pack: don't resend known-common refs in find_common

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 21, 2014, 17:56 UTC
Message-ID
<xmqqd29l1f3p.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20141021144838.GA11589@seahawk>
Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
Show 9 quoted lines
> By not clearing the request buffer in stateless-rpc mode, fetch-pack
> would keep sending already known-common commits, leading to ever bigger
> http requests, eventually getting too large for git-http-backend to
> handle properly without filling up the pipe buffer in inflate_request.
> ---
> I'm still not quite sure whether this is the right thing to do, but make
> test still passes :) The new testcase demonstrates the problem, when
> running t5551 with EXPENSIVE, this test will hang without the patch to
> fetch-pack.c and succeed otherwise.

IIUC, because "stateless" is just that, i.e. the server-end does not keep track of what is already known, not telling what is known to be common in each request would fundamentally break the protocol. Am I mistaken?

Show 58 quoted lines
>  fetch-pack.c                |  1 -
>  t/t5551-http-fetch-smart.sh | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 655ee64..258245c 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -410,7 +410,6 @@ static int find_common(struct fetch_pack_args *args,
>  						 */
>  						const char *hex = sha1_to_hex(result_sha1);
>  						packet_buf_write(&req_buf, "have %s\n", hex);
> -						state_len = req_buf.len;
>  					}
>  					mark_common(commit, 0, 1);
>  					retval = 0;
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index 6cbc12d..2aac237 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -245,5 +245,37 @@ test_expect_success EXPENSIVE 'clone the 50,000 tag repo to check OS command lin
>  	)
>  '
>  
> +test_expect_success EXPENSIVE 'create 50,000 more tags' '
> +	(
> +	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> +	for i in `test_seq 50001 100000`
> +	do
> +		echo "commit refs/heads/too-many-refs-again"
> +		echo "mark :$i"
> +		echo "committer git <git@example.com> $i +0000"
> +		echo "data 0"
> +		echo "M 644 inline bla.txt"
> +		echo "data 4"
> +		echo "bla"
> +		# make every commit dangling by always
> +		# rewinding the branch after each commit
> +		echo "reset refs/heads/too-many-refs-again"
> +		echo "from :50001"
> +	done | git fast-import --export-marks=marks &&
> +
> +	# now assign tags to all the dangling commits we created above
> +	tag=$(perl -e "print \"bla\" x 30") &&
> +	sed -e "s|^:\([^ ]*\) \(.*\)$|\2 refs/tags/$tag-\1|" <marks >>packed-refs
> +	)
> +'
> +
> +test_expect_success EXPENSIVE 'fetch the new tags' '
> +	(
> +		cd too-many-refs &&
> +		git fetch --tags &&
> +		test $(git for-each-ref refs/tags | wc -l) = 100000
> +	)
> +'
> +
>  stop_httpd
>  test_done
Previous: Dennis KaarsemakerNext: Dennis Kaarsemaker
Message 4 of 10 in “git fetch (http) hanging/failing on one specific repository, http only”
  1. Dennis KaarsemakerOct 20, 2014
  2. Bug in fetch-pack causing ever-growing http requests. (Was: git fetch (http) hanging/failing on one specific repository, http only)Dennis Kaarsemaker, Oct 21, 2014
  3. fetch-pack: don't resend known-common refs in find_commonDennis Kaarsemaker, Oct 21, 2014
  4. Junio C HamanoOct 21, 2014
  5. Dennis KaarsemakerOct 22, 2014
  6. Duy NguyenOct 22, 2014
  7. Dennis KaarsemakerOct 26, 2014
  8. Junio C HamanoOct 22, 2014
  9. Dennis KaarsemakerOct 26, 2014
  10. Shawn PearceDec 6, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.