[v5,0/1] no lazy fetch in lookup_commit_in_graph()

Message

Han Xin July 12, 2022, 6:50 a.m. UTC

This patch fixes the following issue:
When we found the commit in the graph in lookup_commit_in_graph(), 
but the commit is missing from the repository, we will try
promisor_remote_get_direct() and then enter another loop.

Then we will go into an endless loop:
  git fetch -> deref_without_lazy_fetch() ->
    lookup_commit_in_graph() -> repo_has_object_file() ->
      promisor_remote_get_direct() -> fetch_objects() ->
        git fetch (a new loop round)

Changes since v4:

* Remove run_with_limited_processses() as it can be catched by CI
  settings and developer workstation. Keeping it will make a trouble
  when there are too many prcesses or stress is used.

Han Xin (1):
  commit-graph.c: no lazy fetch in lookup_commit_in_graph()

 commit-graph.c                             |  2 +-
 t/t5330-no-lazy-fetch-with-commit-graph.sh | 47 ++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100755 t/t5330-no-lazy-fetch-with-commit-graph.sh

Range-diff against v4:
1:  96d4bb7150 ! 1:  3ffeed50de commit-graph.c: no lazy fetch in lookup_commit_in_graph()
    @@ t/t5330-no-lazy-fetch-with-commit-graph.sh (new)
     +
     +. ./test-lib.sh
     +
    -+run_with_limited_processses () {
    -+	# bash and ksh use "ulimit -u", dash uses "ulimit -p"
    -+	if test -n "$BASH_VERSION"
    -+	then
    -+		ulimit_max_process="-u"
    -+	elif test -n "$KSH_VERSION"
    -+	then
    -+		ulimit_max_process="-u"
    -+	fi
    -+	(ulimit ${ulimit_max_process-"-p"} 512 && "$@")
    -+}
    -+
    -+test_lazy_prereq ULIMIT_PROCESSES '
    -+	run_with_limited_processses true
    -+'
    -+
    -+if ! test_have_prereq ULIMIT_PROCESSES
    -+then
    -+	skip_all='skipping tests for no lazy fetch with the commit-graph, ulimit processes not available'
    -+	test_done
    -+fi
    -+
     +test_expect_success 'setup: prepare a repository with a commit' '
     +	git init with-commit &&
     +	test_commit -C with-commit the-commit &&
    @@ t/t5330-no-lazy-fetch-with-commit-graph.sh (new)
     +	git -C with-commit-graph config remote.origin.partialclonefilter blob:none &&
     +	test_commit -C with-commit any-commit &&
     +	anycommit=$(git -C with-commit rev-parse HEAD) &&
    -+
    -+	run_with_limited_processses env GIT_TRACE="$(pwd)/trace.txt" \
    ++	GIT_TRACE="$(pwd)/trace.txt" \
     +		git -C with-commit-graph fetch origin $anycommit 2>err &&
     +	! grep "fatal: promisor-remote: unable to fork off fetch subprocess" err &&
     +	grep "git fetch origin" trace.txt >actual &&

Comments

Jeff King July 12, 2022, 6:58 a.m. UTC | #1

On Tue, Jul 12, 2022 at 02:50:46PM +0800, Han Xin wrote:

> Changes since v4:
> 
> * Remove run_with_limited_processses() as it can be catched by CI
>   settings and developer workstation. Keeping it will make a trouble
>   when there are too many prcesses or stress is used.

Your v4 is already in 'next', so I think rather than replacing the
patch, we'd want a commit on top to remove run_with_limited_processes().

-Peff