From a8b53b13ff1424e27a150346087ac35d73e5096e Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 11 Jun 2020 21:04:47 +0200 Subject: [PATCH 1/8] ci-sanitizer-runs: deflake a couple of tests Signed-off-by: Otto van der Schaaf --- test/integration/nighthawk_grpc_service.py | 2 +- test/integration/nighthawk_test_server.py | 4 ++-- test/integration/test_connection_management.py | 17 ++++++++--------- test/integration/test_integration_basics.py | 10 +++++----- test/integration/test_remote_execution.py | 2 +- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/test/integration/nighthawk_grpc_service.py b/test/integration/nighthawk_grpc_service.py index eeaed679f..88ab77ce2 100644 --- a/test/integration/nighthawk_grpc_service.py +++ b/test/integration/nighthawk_grpc_service.py @@ -60,7 +60,7 @@ def _serverThreadRunner(self): self._address_file = None def _waitUntilServerListening(self): - tries = 30 + tries = 90 while tries > 0: contents = "" if not self._address_file is None: diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index c07183534..194063371 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -53,7 +53,7 @@ def __init__(self, server_binary_path, config_template_path, server_ip, ip_versi def serverThreadRunner(self): args = [ self.server_binary_path, self.server_binary_config_path_arg, self.parameterized_config_path, - "-l", "error", "--base-id", self.instance_id, "--admin-address-path", + "-l", "error", "--concurrency", "1", "--base-id", self.instance_id, "--admin-address-path", self.admin_address_path ] logging.info("Test server popen() args: [%s]" % args) @@ -101,7 +101,7 @@ def enableCpuProfiler(self): def waitUntilServerListening(self): # we allow 30 seconds for the server to have its listeners up. # (It seems that in sanitizer-enabled runs this can take a little while) - timeout = time.time() + 30 + timeout = time.time() + 60 while time.time() < timeout: if self.tryUpdateFromAdminInterface(): return True diff --git a/test/integration/test_connection_management.py b/test/integration/test_connection_management.py index c696c72cd..857abfb8c 100644 --- a/test/integration/test_connection_management.py +++ b/test/integration/test_connection_management.py @@ -107,26 +107,25 @@ def countLogLinesWithSubstring(logs, substring): return len([line for line in logs.split(os.linesep) if substring in line]) _, logs = http_test_server_fixture.runNighthawkClient([ - "--rps 20", "-v", "trace", "--connections", "2", "--prefetch-connections", + "--rps 5", "-v", "trace", "--connections", "2", "--prefetch-connections", "--experimental-h1-connection-reuse-strategy", "mru", "--termination-predicate", - "benchmark.http_2xx:10", "--simple-warmup", - http_test_server_fixture.getTestServerRootUri() + "benchmark.http_2xx:4", http_test_server_fixture.getTestServerRootUri() ]) - requests = 60 - connections = 3 assertNotIn("[C1] message complete", logs) - assertEqual(countLogLinesWithSubstring(logs, "[C0] message complete"), 22) + assertEqual(countLogLinesWithSubstring(logs, "[C0] message complete"), 10) + requests = 12 + connections = 3 _, logs = http_test_server_fixture.runNighthawkClient([ - "--rps", "20", "-v trace", "--connections", + "--rps", "5", "-v trace", "--connections", str(connections), "--prefetch-connections", "--experimental-h1-connection-reuse-strategy", "lru", "--termination-predicate", - "benchmark.http_2xx:%d" % requests, + "benchmark.http_2xx:%d" % (requests-1), http_test_server_fixture.getTestServerRootUri() ]) for i in range(1, connections): line_count = countLogLinesWithSubstring(logs, "[C%d] message complete" % i) strict_count = (requests / connections) * 2 # We need to mind a single warmup call - assertBetweenInclusive(line_count, strict_count, strict_count + 2) + assertBetweenInclusive(line_count, strict_count, strict_count) diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py index a34ef52fb..8a12b83ce 100644 --- a/test/integration/test_integration_basics.py +++ b/test/integration/test_integration_basics.py @@ -87,8 +87,8 @@ def test_http_h1_mini_stress_test_with_client_side_queueing(http_test_server_fix "10", "--connections", "1", "--duration", "100", "--termination-predicate", "benchmark.http_2xx:99", "--simple-warmup" ]) - assertCounterEqual(counters, "upstream_rq_pending_total", 11) - assertCounterEqual(counters, "upstream_cx_overflow", 10) + assertCounterGreaterEqual(counters, "upstream_rq_pending_total", 11) + assertCounterGreaterEqual(counters, "upstream_cx_overflow", 10) def test_http_h1_mini_stress_test_without_client_side_queueing(http_test_server_fixture): @@ -115,7 +115,7 @@ def test_http_h2_mini_stress_test_with_client_side_queueing(http_test_server_fix "--termination-predicate", "benchmark.http_2xx:99", "--simple-warmup" ]) assertCounterEqual(counters, "upstream_rq_pending_total", 1) - assertCounterEqual(counters, "upstream_rq_pending_overflow", 10) + assertCounterGreaterEqual(counters, "upstream_rq_pending_overflow", 10) def test_http_h2_mini_stress_test_without_client_side_queueing(http_test_server_fixture): @@ -124,7 +124,7 @@ def test_http_h2_mini_stress_test_without_client_side_queueing(http_test_server_ queueing. """ counters = mini_stress_test(http_test_server_fixture, [ - http_test_server_fixture.getTestServerRootUri(), "--rps", "999999", "--h2", + http_test_server_fixture.getTestServerRootUri(), "--rps", "1000", "--h2", "--max-active-requests", "1", "--connections", "1", "--duration", "100", "--termination-predicate", "benchmark.http_2xx:99" ]) @@ -276,7 +276,7 @@ def test_https_h2_multiple_connections(https_test_server_fixture): "10" ]) counters = https_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json) - assertCounterEqual(counters, "benchmark.http_2xx", 100) + assertCounterGreaterEqual(counters, "benchmark.http_2xx", 100) # Empirical observation shows we may end up creating more then 10 connections. # This is stock Envoy h/2 pool behavior. assertCounterGreaterEqual(counters, "upstream_cx_http2_total", 10) diff --git a/test/integration/test_remote_execution.py b/test/integration/test_remote_execution.py index 2c2b9cf3e..ea6920c57 100644 --- a/test/integration/test_remote_execution.py +++ b/test/integration/test_remote_execution.py @@ -20,7 +20,7 @@ def test_remote_execution_basics(http_test_server_fixture): ] parsed_json, _ = http_test_server_fixture.runNighthawkClient(args) counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json) - assertCounterEqual(counters, "benchmark.http_2xx", 25) + assertCounterGreaterEqual(counters, "benchmark.http_2xx", 25) # As a control step, prove we are actually performing remote execution: re-run the command without an # operational gRPC service. That ought to fail. From 39934228942e6f6fe759267b9e3f008eaa9d5da5 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 11 Jun 2020 21:12:17 +0200 Subject: [PATCH 2/8] Remove stale comment Signed-off-by: Otto van der Schaaf --- test/integration/test_connection_management.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/test_connection_management.py b/test/integration/test_connection_management.py index 857abfb8c..edf727499 100644 --- a/test/integration/test_connection_management.py +++ b/test/integration/test_connection_management.py @@ -127,5 +127,4 @@ def countLogLinesWithSubstring(logs, substring): for i in range(1, connections): line_count = countLogLinesWithSubstring(logs, "[C%d] message complete" % i) strict_count = (requests / connections) * 2 - # We need to mind a single warmup call assertBetweenInclusive(line_count, strict_count, strict_count) From 2da7cbb5a2f3ec2331992da0ca797819b42e67a2 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 11 Jun 2020 21:59:15 +0200 Subject: [PATCH 3/8] slow down test_grpc_service_happy_flow Signed-off-by: Otto van der Schaaf --- test/integration/test_grpc_service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/test_grpc_service.py b/test/integration/test_grpc_service.py index 6e02bf746..418b424b2 100644 --- a/test/integration/test_grpc_service.py +++ b/test/integration/test_grpc_service.py @@ -8,13 +8,13 @@ def test_grpc_service_happy_flow(http_test_server_fixture): http_test_server_fixture.startNighthawkGrpcService("dummy-request-source") parsed_json, _ = http_test_server_fixture.runNighthawkClient([ - "--termination-predicate", "benchmark.http_2xx:10", "--rps 100", + "--termination-predicate", "benchmark.http_2xx:5", "--rps 10", "--request-source %s:%s" % (http_test_server_fixture.grpc_service.server_ip, http_test_server_fixture.grpc_service.server_port), http_test_server_fixture.getTestServerRootUri() ]) counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json) - assertGreaterEqual(counters["benchmark.http_2xx"], 10) + assertGreaterEqual(counters["benchmark.http_2xx"], 5) assertEqual(counters["requestsource.internal.upstream_rq_200"], 1) From ef3ffdad9fb110ced04d55df7ed3d6259d1ff696 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 11 Jun 2020 22:02:15 +0200 Subject: [PATCH 4/8] fix expectation in _do_tls_configuration_test Signed-off-by: Otto van der Schaaf --- test/integration/test_integration_basics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py index 8a12b83ce..f57371aa6 100644 --- a/test/integration/test_integration_basics.py +++ b/test/integration/test_integration_basics.py @@ -310,7 +310,7 @@ def _do_tls_configuration_test(https_test_server_fixture, cli_parameter, use_h2) https_test_server_fixture.getTestServerRootUri() ]) counters = https_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json) - assertCounterEqual(counters, "ssl.ciphers.%s" % cipher, 1) + assertCounterGreaterEqual(counters, "ssl.ciphers.%s" % cipher, 1) def test_https_h1_tls_context_configuration(https_test_server_fixture): From 0d24955bf3179ec617f6f1a8108491c709c0de28 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 11 Jun 2020 22:32:11 +0200 Subject: [PATCH 5/8] minimize diff Signed-off-by: Otto van der Schaaf --- test/integration/test_integration_basics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py index f57371aa6..7ef1894d5 100644 --- a/test/integration/test_integration_basics.py +++ b/test/integration/test_integration_basics.py @@ -124,7 +124,7 @@ def test_http_h2_mini_stress_test_without_client_side_queueing(http_test_server_ queueing. """ counters = mini_stress_test(http_test_server_fixture, [ - http_test_server_fixture.getTestServerRootUri(), "--rps", "1000", "--h2", + http_test_server_fixture.getTestServerRootUri(), "--rps", "999999", "--h2", "--max-active-requests", "1", "--connections", "1", "--duration", "100", "--termination-predicate", "benchmark.http_2xx:99" ]) @@ -138,7 +138,7 @@ def test_http_h1_mini_stress_test_open_loop(http_test_server_fixture): H1 open loop stress test. We expect higher pending and overflow counts """ counters = mini_stress_test(http_test_server_fixture, [ - http_test_server_fixture.getTestServerRootUri(), "--rps", "10000", "--max-pending-requests", + http_test_server_fixture.getTestServerRootUri(), "--rps", "999999", "--max-pending-requests", "1", "--open-loop", "--max-active-requests", "1", "--connections", "1", "--duration", "100", "--termination-predicate", "benchmark.http_2xx:99", "--simple-warmup" ]) From 53b2b59c1aa277c59df8f1acc4f71e7cc8d9345f Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 11 Jun 2020 23:40:15 +0200 Subject: [PATCH 6/8] Minimize diff pt II Signed-off-by: Otto van der Schaaf --- test/integration/test_integration_basics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py index 7ef1894d5..bb0315588 100644 --- a/test/integration/test_integration_basics.py +++ b/test/integration/test_integration_basics.py @@ -138,7 +138,7 @@ def test_http_h1_mini_stress_test_open_loop(http_test_server_fixture): H1 open loop stress test. We expect higher pending and overflow counts """ counters = mini_stress_test(http_test_server_fixture, [ - http_test_server_fixture.getTestServerRootUri(), "--rps", "999999", "--max-pending-requests", + http_test_server_fixture.getTestServerRootUri(), "--rps", "10000", "--max-pending-requests", "1", "--open-loop", "--max-active-requests", "1", "--connections", "1", "--duration", "100", "--termination-predicate", "benchmark.http_2xx:99", "--simple-warmup" ]) From 071b9411f8a1e46ecf18ce5b905eee94d94c8d4c Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 11 Jun 2020 23:46:50 +0200 Subject: [PATCH 7/8] Back out accidentally left in change Signed-off-by: Otto van der Schaaf --- test/integration/nighthawk_test_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index 194063371..28ecb359c 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -53,7 +53,7 @@ def __init__(self, server_binary_path, config_template_path, server_ip, ip_versi def serverThreadRunner(self): args = [ self.server_binary_path, self.server_binary_config_path_arg, self.parameterized_config_path, - "-l", "error", "--concurrency", "1", "--base-id", self.instance_id, "--admin-address-path", + "-l", "error", "--base-id", self.instance_id, "--admin-address-path", self.admin_address_path ] logging.info("Test server popen() args: [%s]" % args) From ede3a9d7a08bdceb66df64cca5aaf7c653052d9a Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Fri, 12 Jun 2020 00:01:30 +0200 Subject: [PATCH 8/8] Fix format issue Signed-off-by: Otto van der Schaaf --- test/integration/test_connection_management.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/integration/test_connection_management.py b/test/integration/test_connection_management.py index edf727499..362519d45 100644 --- a/test/integration/test_connection_management.py +++ b/test/integration/test_connection_management.py @@ -109,7 +109,8 @@ def countLogLinesWithSubstring(logs, substring): _, logs = http_test_server_fixture.runNighthawkClient([ "--rps 5", "-v", "trace", "--connections", "2", "--prefetch-connections", "--experimental-h1-connection-reuse-strategy", "mru", "--termination-predicate", - "benchmark.http_2xx:4", http_test_server_fixture.getTestServerRootUri() + "benchmark.http_2xx:4", + http_test_server_fixture.getTestServerRootUri() ]) assertNotIn("[C1] message complete", logs) @@ -121,7 +122,7 @@ def countLogLinesWithSubstring(logs, substring): "--rps", "5", "-v trace", "--connections", str(connections), "--prefetch-connections", "--experimental-h1-connection-reuse-strategy", "lru", "--termination-predicate", - "benchmark.http_2xx:%d" % (requests-1), + "benchmark.http_2xx:%d" % (requests - 1), http_test_server_fixture.getTestServerRootUri() ]) for i in range(1, connections):