From cb16886be847eff42cd8acbd6a5f13694b0fde73 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Thu, 23 Jul 2026 01:14:58 +0000 Subject: [PATCH 1/4] Fix flaky test_s3_table_functions_timeouts The test injects a 1200ms network delay and expects an S3 INSERT to time out and raise. It set no explicit S3 timeout, so raising relied on a default timeout being below 1200ms on the connection path actually taken. On a fresh connection the adaptive connect timeout (500ms first attempt, 1000ms on retries) trips reliably. But when the S3 request reuses a pooled keep-alive connection there is no fresh TCP connect, so the connect timeout does not apply; the only remaining gate is the send/receive idleness timeout, whose default (adaptive 3000ms / configured 30000ms) is far above 1200ms, so the write completes and pytest.raises sees no exception. That is the rare DID NOT RAISE flake (one occurrence in 40 days of CI, zero on master). Make the request timeout the single failure mechanism: disable adaptive timeouts, keep the connect timeout above the delay, and set s3_request_timeout_ms=500. The send/receive idleness timeout applies to every attempt on both fresh and reused connections, so the 1200ms delay always exceeds it and the write times out deterministically. Also assert the raised error is a timeout, so the test cannot pass on an unrelated error. (cherry picked from commit bf0f90973919896ae76f52fb6e7cb51a067a6147) --- .../test_s3_table_functions/test.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_s3_table_functions/test.py b/tests/integration/test_s3_table_functions/test.py index b2dfddb23ebd..ec565de617ca 100644 --- a/tests/integration/test_s3_table_functions/test.py +++ b/tests/integration/test_s3_table_functions/test.py @@ -78,14 +78,25 @@ def test_s3_table_functions(started_cluster): def test_s3_table_functions_timeouts(started_cluster): """ - Test with timeout limit of 1200ms. - This should raise an Exception and pass. + A 1200ms network delay must make the S3 write time out and raise. """ + # Make the S3 request timeout (not the connect timeout) the single failure mechanism: + # disable adaptive timeouts and keep the connect timeout above the delay, so the write + # can only fail via s3_request_timeout_ms. This exercises the send/receive idleness + # timeout that applies to every attempt on both fresh and reused (pooled keep-alive) + # connections, which is the path that was silently not timing out before. + timeout_settings = { + **settings, + "s3_use_adaptive_timeouts": "0", + "s3_connect_timeout_ms": "10000", + "s3_request_timeout_ms": "500", + } + with PartitionManager() as pm: pm.add_network_delay(node, 1200) - with pytest.raises(QueryRuntimeException): + with pytest.raises(QueryRuntimeException, match="Timeout"): node.query( """ INSERT INTO FUNCTION s3 @@ -98,5 +109,5 @@ def test_s3_table_functions_timeouts(started_cluster): ) SELECT * FROM numbers(1000000) """, - settings=settings, + settings=timeout_settings, ) From 4326225e38f242074a2f3fc40e7962d5d7dc84e4 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Thu, 23 Jul 2026 02:39:41 +0000 Subject: [PATCH 2/4] Fix flaky test 01661_extract_all_groups_throw_fast under memory pressure The first query materialized a ~1.3 GB haystack column (numbers(1023), each row up to 2.66 MB) only to trip the per-row regexp_max_matches_per_row (default 1000) fast-throw TOO_LARGE_ARRAY_SIZE. The throw fires on the first row (2600 matches); the rest of the column is wasted allocation. Under the shared parallel-runner memory budget, building it could hit MEMORY_LIMIT_EXCEEDED before the expected throw, so the test intermittently failed (0 master failures, 27 unrelated PRs in 30 days). Use a single materialized 1001-character row, which crosses the same per-row limit at ~1 KB instead of 1.3 GB. materialize() keeps the runtime code path the throw lives in. A low max_memory_usage on the query also guards against regressing to a large fixture. The second query is unchanged. Co-Authored-By: Claude Opus 4.8 (cherry picked from commit b54dc15ccf10034125247dc4b38cad72e56101c3) --- .../0_stateless/01661_extract_all_groups_throw_fast.sql | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/01661_extract_all_groups_throw_fast.sql b/tests/queries/0_stateless/01661_extract_all_groups_throw_fast.sql index afb6866a0df5..f16e83a6d0f5 100644 --- a/tests/queries/0_stateless/01661_extract_all_groups_throw_fast.sql +++ b/tests/queries/0_stateless/01661_extract_all_groups_throw_fast.sql @@ -1,2 +1,4 @@ -SELECT repeat('abcdefghijklmnopqrstuvwxyz', number * 100) AS haystack, extractAllGroupsHorizontal(haystack, '(\\w)') AS matches FROM numbers(1023); -- { serverError TOO_LARGE_ARRAY_SIZE } +-- 1001 matches in one row exceeds `regexp_max_matches_per_row` (default 1000) and trips the fast-throw. +-- The low `max_memory_usage` also guards against regressing to a huge fixture that could hit MEMORY_LIMIT_EXCEEDED first. +SELECT extractAllGroupsHorizontal(materialize(repeat('a', 1001)), '(\\w)') FORMAT Null SETTINGS max_memory_usage = 100000000; -- { serverError TOO_LARGE_ARRAY_SIZE } SELECT count(extractAllGroupsHorizontal(materialize('a'), '(a)')) FROM numbers(1000000) FORMAT Null; -- shouldn't fail From 0360bbb30ec5e3ef726e9f6cdd3e19c9170f8b69 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Thu, 23 Jul 2026 22:46:53 +0000 Subject: [PATCH 3/4] Fix flaky test_kafka_formats_with_broken_message The per-format setup produced all messages (including the tail broken message) to the topic before the destination materialized views existed, then created the Kafka table, the data view and the errors view in sequence. Creating the first materialized view starts the streaming loop, so on slow builds the loop could consume and commit the broken message before the errors view was attached. The broken message is then past the committed offset and never reaches the errors view: the data view has its rows but the errors view stays empty, failing the test at "Error row for format X did not appear in kafka_errors_X_mv". Create both materialized views, detach/re-attach the Kafka table, and only then produce the messages, so nothing is consumable until both views exist. This mirrors the fix accepted for the sibling test test_kafka_engine_put_errors_to_stream in #107025. Co-Authored-By: Claude Opus 4.8 (cherry picked from commit 231c36ce98d6fd02c98891b8c7a5ab3eafdd93ef) --- .../integration/test_storage_kafka/test_batch_slow_0.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_storage_kafka/test_batch_slow_0.py b/tests/integration/test_storage_kafka/test_batch_slow_0.py index 03e7297aa3dd..6f6393f0bd00 100644 --- a/tests/integration/test_storage_kafka/test_batch_slow_0.py +++ b/tests/integration/test_storage_kafka/test_batch_slow_0.py @@ -301,7 +301,6 @@ def test_kafka_formats_with_broken_message(kafka_cluster, create_query_generator data_prefix = data_prefix + [""] if format_opts.get("printable", False) == False: raw_message = "hex(_raw_message)" - k.kafka_produce(kafka_cluster, topic_name, data_prefix + data_sample) create_query = create_query_generator( f"kafka_{format_name}", "id Int64, blockNo UInt16, val1 String, val2 Float32, val3 UInt8", @@ -313,6 +312,10 @@ def test_kafka_formats_with_broken_message(kafka_cluster, create_query_generator "kafka_flush_interval_ms": 1000, }, ) + # Create both materialized views, then detach/re-attach the Kafka table, + # before producing any message. Creating the first view starts the + # streaming loop, so producing earlier lets the loop consume and commit + # the broken message before the errors view is attached, leaving it empty. instance.query( f""" DROP TABLE IF EXISTS test.kafka_{format_name}; @@ -328,8 +331,12 @@ def test_kafka_formats_with_broken_message(kafka_cluster, create_query_generator CREATE MATERIALIZED VIEW test.kafka_errors_{format_name}_mv ENGINE=MergeTree ORDER BY tuple() AS SELECT {raw_message} as raw_message, _error as error, _topic as topic, _partition as partition, _offset as offset FROM test.kafka_{format_name} WHERE length(_error) > 0; + + DETACH TABLE test.kafka_{format_name}; + ATTACH TABLE test.kafka_{format_name}; """ ) + k.kafka_produce(kafka_cluster, topic_name, data_prefix + data_sample) raw_expected = """\ 0 0 AM 0.5 1 {topic_name} 0 {offset_0} From def2e35c51bae6d6b0263ac44cbee3675a0d5b54 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Fri, 24 Jul 2026 17:38:27 +0000 Subject: [PATCH 4/4] Fix flaky test_hedged_requests::test_async_connect test_async_connect verifies async-connect preemption: the first replica of each shard in test_cluster_connect is an unreachable address (129.0.0.1 / 129.0.0.2), so the in-progress connect must be preempted by the hedged_connection_timeout_ms timer and switch to the good replica. That timer is the only path counted by the HedgedRequestsChangeReplica event, which the test asserts fired (>= 2, one per shard). The connect to an unreachable 129.0.0.x address does not always stall. Depending on the runner's routing it can fail fast (host unreachable) before the 100 ms timer. A fast failure switches the replica through the connection-failure path (ConnectionEstablisher), which increments DistributedConnectionFailTry, not HedgedRequestsChangeReplica. When both shards fail fast the event stays 0; system.events omits zero-valued events, so the query returns an empty string and int('') raises "invalid literal for int() with base 10: ''". Slow builds (ASan, MSan, coverage) hit the fast-fail path often enough to flake while master stays green (15 failures across 7 unrelated PRs over 90 days, first 2026-05-05, 0 on master). Drop the initiator's outbound packets to 129.0.0.1/129.0.0.2 with PartitionManager so the connect always stalls and is preempted by the hedged timer, taking the counted path. This keeps the original assertion and the async-connect-preemption intent, and makes it deterministic regardless of routing. The engine is not changed: hedged failover already reaches the good replica on both paths. Co-Authored-By: Claude Opus 4.8 (cherry picked from commit c1e3273140e60e7c8958e291cb8bb37d77225ca9) --- .../integration/test_hedged_requests/test.py | 51 +++++++++++++------ 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/tests/integration/test_hedged_requests/test.py b/tests/integration/test_hedged_requests/test.py index 1aafc1aadafa..6c53ef01a5d4 100644 --- a/tests/integration/test_hedged_requests/test.py +++ b/tests/integration/test_hedged_requests/test.py @@ -7,6 +7,7 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from helpers.cluster import ClickHouseCluster +from helpers.network import PartitionManager from helpers.test_tools import TSV cluster = ClickHouseCluster(__file__) @@ -429,28 +430,46 @@ def test_async_connect(started_cluster): Distributed('test_cluster_connect', 'default', 'test_hedged')""" ) - NODES["node"].query( - "SELECT hostName(), id FROM distributed_connect ORDER BY id LIMIT 1 SETTINGS prefer_localhost_replica = 0, connect_timeout_with_failover_ms=5000, async_query_sending_for_remote=0, max_threads=1, max_distributed_connections=1" - ) - check_changing_replica_events(2) - check_if_query_sending_was_not_suspended() - - # Restart server to reset connection pool state - NODES["node"].restart_clickhouse() + # The first replica of each shard in test_cluster_connect is an unreachable + # address (129.0.0.1 / 129.0.0.2). Silently drop the initiator's packets to + # them so the connect always stalls and is preempted by the + # hedged_connection_timeout_ms timer (the path HedgedRequestsChangeReplica + # counts). Otherwise, on slow builds the connect can fail fast (host + # unreachable), switching the replica through the connection-failure path, + # which does not increment that event. + with PartitionManager() as pm: + for unreachable_ip in ("129.0.0.1", "129.0.0.2"): + pm.add_rule( + { + "instance": NODES["node"], + "chain": "OUTPUT", + "destination": unreachable_ip, + "action": "DROP", + } + ) - attempt = 0 - while attempt < 100: NODES["node"].query( - "SELECT hostName(), id FROM distributed_connect ORDER BY id LIMIT 1 SETTINGS prefer_localhost_replica = 0, connect_timeout_with_failover_ms=5000, async_query_sending_for_remote=1, max_threads=1, max_distributed_connections=1" + "SELECT hostName(), id FROM distributed_connect ORDER BY id LIMIT 1 SETTINGS prefer_localhost_replica = 0, connect_timeout_with_failover_ms=5000, async_query_sending_for_remote=0, max_threads=1, max_distributed_connections=1" ) - check_changing_replica_events(2) - if check_if_query_sending_was_suspended(): - break + check_if_query_sending_was_not_suspended() - attempt += 1 + # Restart server to reset connection pool state + NODES["node"].restart_clickhouse() - assert attempt < 100 + attempt = 0 + while attempt < 100: + NODES["node"].query( + "SELECT hostName(), id FROM distributed_connect ORDER BY id LIMIT 1 SETTINGS prefer_localhost_replica = 0, connect_timeout_with_failover_ms=5000, async_query_sending_for_remote=1, max_threads=1, max_distributed_connections=1" + ) + + check_changing_replica_events(2) + if check_if_query_sending_was_suspended(): + break + + attempt += 1 + + assert attempt < 100 NODES["node"].query("DROP TABLE distributed_connect")