From 60765d3e655d11ac3a9b4eabc8723c4ea8c96c7e Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 5 May 2023 16:49:29 +0100 Subject: [PATCH 1/8] tests: linkcheck: add client connection-pool contention coverage --- tests/test_build_linkcheck.py | 46 ++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index acfebd64999..3f1ce383266 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -16,7 +16,12 @@ import pytest -from sphinx.builders.linkcheck import HyperlinkAvailabilityCheckWorker, RateLimit +from sphinx.builders.linkcheck import ( + CheckRequest, + Hyperlink, + HyperlinkAvailabilityCheckWorker, + RateLimit, +) from sphinx.testing.util import strip_escseq from sphinx.util.console import strip_colors @@ -661,6 +666,45 @@ def test_limit_rate_bails_out_after_waiting_max_time(app): assert next_check is None +@mock.patch('sphinx.util.requests.requests.Session.get_adapter') +def test_connection_contention(get_adapter, app, capsys): + # Create a shared, but limited-size, connection pool + import requests + get_adapter.return_value = requests.adapters.HTTPAdapter(pool_maxsize=1) + + # Set an upper-bound on socket timeouts globally + import socket + socket.setdefaulttimeout(5) + + # Place a workload into the linkcheck queue + rqueue, wqueue, link_count = Queue(), Queue(), 10 + for _ in range(link_count): + wqueue.put(CheckRequest(0, Hyperlink("http://localhost:7777", "test", 1))) + + # Create single-hyperlink-consumer threads + with http_server(make_redirect_handler(support_head=True)): + begin, checked = time.time(), [] + while time.time() < begin + 5 and len(checked) < link_count: + worker = HyperlinkAvailabilityCheckWorker( + env=app.env, + config=app.config, + rqueue=rqueue, + wqueue=wqueue, + rate_limits={}, + ) + + worker.start() + result = rqueue.get(timeout=5) + worker.join(timeout=0) + + checked.append(result) + + # Ensure that all items were consumed within the time limit + _, stderr = capsys.readouterr() + assert len(checked) == link_count + assert "TimeoutError" not in stderr + + class ConnectionResetHandler(http.server.BaseHTTPRequestHandler): def do_HEAD(self): self.connection.close() From 589bca53c50d8dee080bf18b48ae42231e8676b3 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 May 2023 17:15:32 +0100 Subject: [PATCH 2/8] tests: linkcheck: increase workload size in connection-pool contention test --- tests/test_build_linkcheck.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index 3f1ce383266..7a1f1952118 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -677,8 +677,12 @@ def test_connection_contention(get_adapter, app, capsys): socket.setdefaulttimeout(5) # Place a workload into the linkcheck queue + # + # Note: excess linkcheck requests are issued here, intended to encourage + # Windows systems to flush pending network socket data when timeouts are + # enabled. rqueue, wqueue, link_count = Queue(), Queue(), 10 - for _ in range(link_count): + for _ in range(link_count * 5): wqueue.put(CheckRequest(0, Hyperlink("http://localhost:7777", "test", 1))) # Create single-hyperlink-consumer threads From 2ba1856ae75297bbc9b7b4f44308a8c8f55d89c7 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 May 2023 17:38:49 +0100 Subject: [PATCH 3/8] tests: linkcheck: refactor connection-pool contention test to use interleaved threads --- tests/test_build_linkcheck.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index 7a1f1952118..d27be73d47d 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -687,21 +687,23 @@ def test_connection_contention(get_adapter, app, capsys): # Create single-hyperlink-consumer threads with http_server(make_redirect_handler(support_head=True)): - begin, checked = time.time(), [] - while time.time() < begin + 5 and len(checked) < link_count: - worker = HyperlinkAvailabilityCheckWorker( + begin, threads, checked = time.time(), [], [] + threads = [ + HyperlinkAvailabilityCheckWorker( env=app.env, config=app.config, rqueue=rqueue, wqueue=wqueue, rate_limits={}, ) - - worker.start() - result = rqueue.get(timeout=5) - worker.join(timeout=0) - - checked.append(result) + for _ in range(10 * 5) + ] + for thread in threads: + thread.start() + while time.time() < begin + 5 and len(checked) < link_count: + checked.append(rqueue.get(timeout=5)) + for thread in threads: + thread.join(timeout=0) # Ensure that all items were consumed within the time limit _, stderr = capsys.readouterr() From 01842717c1d98133851e3382a9c5772324021039 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 May 2023 17:46:44 +0100 Subject: [PATCH 4/8] Revert "tests: linkcheck: increase workload size in connection-pool contention test" This reverts commit 589bca53c50d8dee080bf18b48ae42231e8676b3. --- tests/test_build_linkcheck.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index d27be73d47d..1fb602414b3 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -677,12 +677,8 @@ def test_connection_contention(get_adapter, app, capsys): socket.setdefaulttimeout(5) # Place a workload into the linkcheck queue - # - # Note: excess linkcheck requests are issued here, intended to encourage - # Windows systems to flush pending network socket data when timeouts are - # enabled. rqueue, wqueue, link_count = Queue(), Queue(), 10 - for _ in range(link_count * 5): + for _ in range(link_count): wqueue.put(CheckRequest(0, Hyperlink("http://localhost:7777", "test", 1))) # Create single-hyperlink-consumer threads From 3c0b04a0e740a55d796eedc8cf08e12671108d0b Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 May 2023 17:48:02 +0100 Subject: [PATCH 5/8] tests: linkcheck: further excess-workload revert cleanup Code introduced by commit 2ba1856ae75297bbc9b7b4f44308a8c8f55d89c7. Follows-on-from commit 01842717c1d98133851e3382a9c5772324021039. --- tests/test_build_linkcheck.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index 1fb602414b3..ec3decb9370 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -692,7 +692,7 @@ def test_connection_contention(get_adapter, app, capsys): wqueue=wqueue, rate_limits={}, ) - for _ in range(10 * 5) + for _ in range(10) ] for thread in threads: thread.start() From 89900615d5a38db8c73439a7529f9e4e98145a70 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 May 2023 17:55:01 +0100 Subject: [PATCH 6/8] tests: linkcheck: update explanatory comment --- tests/test_build_linkcheck.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index ec3decb9370..eea4d5ea62f 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -681,7 +681,7 @@ def test_connection_contention(get_adapter, app, capsys): for _ in range(link_count): wqueue.put(CheckRequest(0, Hyperlink("http://localhost:7777", "test", 1))) - # Create single-hyperlink-consumer threads + # Create parallel consumer threads with http_server(make_redirect_handler(support_head=True)): begin, threads, checked = time.time(), [], [] threads = [ From 64c50e19f5a60d0d3dc99c5f8f42e53700d93e4b Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sun, 23 Jul 2023 16:04:12 +0100 Subject: [PATCH 7/8] Fix --- tests/test_build_linkcheck.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index 30ddd722489..ee4d631fd9e 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -780,22 +780,22 @@ def test_connection_contention(get_adapter, app, capsys): socket.setdefaulttimeout(5) # Place a workload into the linkcheck queue - rqueue, wqueue, link_count = Queue(), Queue(), 10 + link_count = 10 + rqueue, wqueue = Queue(), Queue() for _ in range(link_count): - wqueue.put(CheckRequest(0, Hyperlink("http://localhost:7777", "test", 1))) + wqueue.put(CheckRequest(0, Hyperlink("http://localhost:7777", "test", "test.rst", 1))) # Create parallel consumer threads with http_server(make_redirect_handler(support_head=True)): - begin, threads, checked = time.time(), [], [] + begin, checked = time.time(), [] threads = [ HyperlinkAvailabilityCheckWorker( - env=app.env, config=app.config, rqueue=rqueue, wqueue=wqueue, rate_limits={}, ) - for _ in range(10) + ] for thread in threads: thread.start() From b04d993dd527f0f218055a807cf39a2f2ea982f5 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sun, 23 Jul 2023 16:13:23 +0100 Subject: [PATCH 8/8] Fix --- tests/test_build_linkcheck.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index ee4d631fd9e..9b08854b4ff 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -795,7 +795,7 @@ def test_connection_contention(get_adapter, app, capsys): wqueue=wqueue, rate_limits={}, ) - + for _ in range(10) ] for thread in threads: thread.start()