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 01/19] 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 02/19] 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 03/19] 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 04/19] 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") From d5b53ffd2346a5025da4e557701a134809ad4266 Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Sun, 26 Jul 2026 20:29:34 +0000 Subject: [PATCH 05/19] Fix flaky 03356_analyzer_unused_scalar_subquery The test checked that an unused scalar subquery and an unused `WITH` expression are not evaluated by measuring wall clock time: it ran `sleepEachRow(3)` inside them with `max_execution_time = 2`, so the query was expected to finish quickly instead of timing out. That makes the test depend on machine load rather than on the property under test. On a loaded sanitizer runner even the trivial `SELECT * FROM system.one` exceeds the limit: the reported timeouts were `elapsed 6086.177 ms` and `elapsed 13035.071488 ms` against `maximum: 2000 ms`, both far above the 3 second sleep, and one run failed with `MEMORY_LIMIT_EXCEEDED` instead. Raising the limit from 1 to 2 seconds in #91744 did not help. Check the same property with `throwIf` instead: if the unused expression is evaluated, the query fails with `FUNCTION_THROW_IF_VALUE_IS_NON_ZERO`, and otherwise it succeeds immediately, with no dependency on timing. Two queries that do use the value are added to make sure the check has teeth. The test now takes about 0.4 seconds instead of 7.7 seconds. https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=f9189f48f9bfdf7d079878e800b6eb46beb8924a&name_0=MasterCI&name_1=Stateless%20tests%20%28amd_tsan%2C%20parallel%29 Related: https://github.com/ClickHouse/ClickHouse/issues/89202 Related: https://github.com/ClickHouse/ClickHouse/pull/91744 Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 4ce52e577aa1d4a981c6a4a5b474112891009711) --- .../03356_analyzer_unused_scalar_subquery.sql | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/queries/0_stateless/03356_analyzer_unused_scalar_subquery.sql b/tests/queries/0_stateless/03356_analyzer_unused_scalar_subquery.sql index cb8594046ad0..90242ab22a2d 100644 --- a/tests/queries/0_stateless/03356_analyzer_unused_scalar_subquery.sql +++ b/tests/queries/0_stateless/03356_analyzer_unused_scalar_subquery.sql @@ -1,16 +1,31 @@ +-- An unused scalar subquery or an unused `WITH` expression must not be evaluated, +-- so `throwIf` never fires. This is checked with `throwIf` rather than with a +-- timeout, because a timeout makes the test flaky on a loaded machine. + set enable_analyzer = 1; WITH ( - SELECT sleepEachRow(3) + SELECT throwIf(1) ) AS res SELECT * FROM system.one -FORMAT Null -SETTINGS max_execution_time = 2; +FORMAT Null; -WITH sleepEachRow(3) AS res +WITH throwIf(1) AS res SELECT * FROM system.one -FORMAT Null -SETTINGS max_execution_time = 2; +FORMAT Null; + +-- But it is evaluated when it is actually used. +WITH ( + SELECT throwIf(1) + ) AS res +SELECT res +FROM system.one +FORMAT Null; -- { serverError FUNCTION_THROW_IF_VALUE_IS_NON_ZERO } + +WITH throwIf(1) AS res +SELECT res +FROM system.one +FORMAT Null; -- { serverError FUNCTION_THROW_IF_VALUE_IS_NON_ZERO } From 75af409c2fe88e6bc0a48120e4ba5a5daf02779b Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:34:48 +0000 Subject: [PATCH 06/19] Fix flaky `test_kafka_flush_by_time` by reusing one Kafka producer `test_kafka_flush_by_time` asserts `uniqExact` over the insert timestamp plus a row count of at least 15 after an 18 second window. The flush count, which the test is named for, is correct in every failure; the row count is what falls short. The produce loop sent one message per `k.kafka_produce` call, and that helper constructs a new `KafkaProducer` on every call, so every message paid a full client bootstrap including the `kafka-python` broker-version probe. In the failing CI run that probe took 2.1 s on one iteration (3.023 s for the whole iteration, against 0.107-0.113 s for the other 17), which cost about 3 messages out of a budget that had roughly half a message of slack. The result was 5 + 9 = 14 rows over 2 distinct flush timestamps, so the flush-by-time behaviour was correct and only the total was short. Both engines flushed on schedule (7841/7599 ms, versus 7537/7535 ms in the parametrization that passed on the same server 21 seconds earlier), so this is not a `StorageKafka` or `StorageKafka2` flush race. Hoist one producer out of the loop, reusing the existing `k.get_kafka_producer` and `k.producer_serializer` helpers that five other producers in this file already use, and close it after the producer thread is joined. Per-iteration cost drops from 0.107-3.02 s to a single send plus flush round trip. The assertion, the threshold of 15, the 18 second window and the 0.8 s message rate are unchanged. Measured after the change: producer constructions per run drop from 17 to 1; there are still exactly 2 in-window flushes in 25 of 25 runs and never 3, so the distinct-timestamp assertion stays live and the cadence stays on the `stream_flush_interval_ms` default at 7514-9020 ms; in-window row totals are 19 to 21 against the threshold of 15, instead of 14. 50 executions across both parametrizations pass. Verified in both directions by injecting the measured 2.9 s stall into the produce path: on master the test fails with the CI signature, with this change it passes even when the stall fires inside the loop, and reverting only the hoist under the same stall brings the failure back. (cherry picked from commit dce0ed243024dd6e12484f37967ca8e110ad4674) --- .../test_storage_kafka/test_batch_fast.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_storage_kafka/test_batch_fast.py b/tests/integration/test_storage_kafka/test_batch_fast.py index c9705f2eafdb..5608582281f5 100644 --- a/tests/integration/test_storage_kafka/test_batch_fast.py +++ b/tests/integration/test_storage_kafka/test_batch_fast.py @@ -2081,10 +2081,19 @@ def test_kafka_flush_by_time(kafka_cluster, create_query_generator): cancel = threading.Event() + # Reuse one producer: `k.kafka_produce` opens a new connection per call, + # and a single broker-version probe there can cost seconds, which is + # enough to miss the row count asserted below. + producer = k.get_kafka_producer( + kafka_cluster.kafka_port, k.producer_serializer, retries=15 + ) + def produce(): while not cancel.is_set(): - messages = [json.dumps({"key": 0, "value": 0})] - k.kafka_produce(kafka_cluster, topic_name, messages) + producer.send( + topic=topic_name, value=json.dumps({"key": 0, "value": 0}) + ) + producer.flush() time.sleep(0.8) kafka_thread = threading.Thread(target=produce) @@ -2102,6 +2111,7 @@ def produce(): cancel.set() kafka_thread.join() + producer.close() instance.query(f""" DROP TABLE test.{kafka_table}_consumer; From 1057b1c1db0a21280b576831b72f2112049dc04a Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Mon, 27 Jul 2026 18:06:05 +0000 Subject: [PATCH 07/19] Fix flaky `03100_lwu_15_future_reads` The test ends with SELECT sum(rows) FROM system.parts WHERE ... AND startsWith(name, 'patch') and expects `200`, but that query was racing against the background cleanup thread. Once `OPTIMIZE TABLE ... FINAL` has applied the patches, the data version of the merged part reaches the patches' max data version, so `clearUnusedPatchParts` considers both patch parts unused and drops them. For `ReplicatedMergeTree` the drop goes through a `DROP_PART` log entry, which removes the parts outright rather than leaving them `Outdated`, so they disappear from `system.parts` and the last query returns `0` - or `100`, when only one of them had been dropped yet. On a fast runner the test finishes long before the first iteration of the cleanup thread, so the race is invisible. On a loaded runner it gets a turn before the last query; accordingly, all observed failures are on `amd_tsan`, `amd_msan` and flaky checks. Pin `remove_unused_patch_parts = 0` on the table, the same way `03100_lwu_01_basics` and other lightweight-update tests already do, so the patch parts stay in place for the final check. The future-reads check itself is unaffected. Reproduced locally by making the cleanup thread run every second: without this change the test fails with exactly the diff seen in CI (`-200` / `+0`), with it the test passes. CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=107275200b9ab432ec9375a091aad62690484bfd&name_0=MasterCI&name_1=Stateless%20tests%20%28amd_tsan%2C%20parallel%29 Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 9101372db9fb6ff53c42d898fb5e56154170a1ea) --- tests/queries/0_stateless/03100_lwu_15_future_reads.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/03100_lwu_15_future_reads.sh b/tests/queries/0_stateless/03100_lwu_15_future_reads.sh index 3c74ff76ba0d..518dce65dde0 100755 --- a/tests/queries/0_stateless/03100_lwu_15_future_reads.sh +++ b/tests/queries/0_stateless/03100_lwu_15_future_reads.sh @@ -26,7 +26,11 @@ $CLICKHOUSE_CLIENT --query " ORDER BY id SETTINGS enable_block_number_column = 1, - enable_block_offset_column = 1; + enable_block_offset_column = 1, + -- The test checks the number of rows in patch parts at the end. + -- Once the patches are applied, the cleanup thread is free to drop them, + -- so keep them around to make the last query deterministic. + remove_unused_patch_parts = 0; INSERT INTO t_lwu_future_reads SELECT number, number FROM numbers(1000); SYSTEM ENABLE FAILPOINT $failpoint_name; From a94aa9a1c6babb46dd65aa7954b50ee0edf443bf Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Mon, 27 Jul 2026 19:20:13 +0000 Subject: [PATCH 08/19] Fix flaky test 02465_limit_trivial_max_rows_to_read The statement `SELECT a FROM t_max_rows_to_read LIMIT 1` has no ORDER BY, so its result is unspecified: any row of the table is a legal answer. On a single reader the first granule is read first, so the answer happened to be 0 and the reference file has asserted that one particular unspecified answer since the test was written. Under parallel replicas the read is split into per-replica mark ranges and whichever replica answers first supplies the row that LIMIT 1 keeps, so the assertion is not stable and the test fails intermittently with a one-line reference diff of 32 instead of 0. 32 is not a random row. On the failing lane all parts are on object storage, so PartRangesReadInfo takes the remote branch where both merge_tree_min_rows_for_concurrent_read_for_remote_filesystem and its bytes counterpart default to 0, and minMarksForConcurrentRead collapses to merge_tree_min_read_task_size = 8 marks per task. With index_granularity = 4 the second task begins at mark 8, whose first row is 8 * 4 = 32. On a local disk the same formula yields a single task covering the whole part, which is why the failure only appears on the object-storage lanes and why earlier local-disk reproduction attempts could only ever observe 0. Project a constant instead, so every legal row selection produces the reference value. The reference file does not change, because it already asserts 0. What the line measures is unchanged, verified rather than assumed: EXPLAIN still shows ReadFromMergeTree and ReadFromRemoteParallelReplicas, read_rows and ProfileEvents['SelectedMarks'] are identical before and after (4 and 25), and lowering max_rows_to_read to 3, 2 or 1 still raises TOO_MANY_ROWS at exactly the old thresholds, on both analyzers. The read is not eliminated because chooseSmallestColumnToReadFromStorage injects the table's single column. max_rows_to_read = 20 and index_granularity = 4, which are what the test actually measures, are untouched, and no setting is pinned, so nothing leaks into the following statements. A no-parallel-replicas tag or a parallel_replicas_blacklist entry was deliberately avoided: either would remove the whole test from that lane, where it otherwise runs green thousands of times, while only one of its seven statements is affected. (cherry picked from commit cc956b9e614b2f4aa5a169d1bf89cfb6010c1807) --- .../0_stateless/02465_limit_trivial_max_rows_to_read.sql | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/02465_limit_trivial_max_rows_to_read.sql b/tests/queries/0_stateless/02465_limit_trivial_max_rows_to_read.sql index 700a54044271..2b6302292b2a 100644 --- a/tests/queries/0_stateless/02465_limit_trivial_max_rows_to_read.sql +++ b/tests/queries/0_stateless/02465_limit_trivial_max_rows_to_read.sql @@ -15,7 +15,10 @@ SELECT number FROM numbers(30) LIMIT 21; -- { serverError TOO_MANY_ROWS } SELECT number FROM numbers(30) LIMIT 1; SELECT number FROM numbers(5); -SELECT a FROM t_max_rows_to_read LIMIT 1; +-- Under parallel replicas this read is split into per-replica mark ranges, so an +-- ORDER-less LIMIT 1 may legally return any row (CI saw 32, the first row of the +-- second task). Project a constant: only `max_rows_to_read` is measured here. +SELECT 0 FROM t_max_rows_to_read LIMIT 1; SELECT a FROM t_max_rows_to_read LIMIT 11 offset 11; -- { serverError TOO_MANY_ROWS } SELECT a FROM t_max_rows_to_read WHERE a > 50 LIMIT 1; -- { serverError TOO_MANY_ROWS } From 909e78a0766281caac58d3dec3e14190fd392690 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 27 Jul 2026 20:48:09 +0000 Subject: [PATCH 09/19] Fix flaky 03394 shuffle-join tests under join-order randomization `03394_distributed_shuffle_join_with_in` and `03394_distributed_shuffle_join_with_aggregation` assert that `BuildRuntimeFilter` appears in the `EXPLAIN` plan, but neither pins the two settings that decide whether the runtime join filter is built at all. Reproduced locally with the exact randomized setting set from the CI report (https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=110321&sha=fbebd96fb7f9dcee3d3bdb40df67558ffeaa1b40&name_0=PR&name_1=Stateless%20tests%20%28amd_msan%2C%20WasmEdge%2C%20parallel%2C%201%2F2%29, PR https://github.com/ClickHouse/ClickHouse/pull/110321) and minimized to a two-setting combination: `query_plan_optimize_join_order_randomize` nonzero together with `join_runtime_filter_min_probe_rows` above the estimated probe size. Join-order randomization changes which side is the probe side, and `joinRuntimeFilter` then skips the filter because the probe side is estimated to produce at most `join_runtime_filter_min_probe_rows` rows, so the `BuildRuntimeFilter` step disappears from the plan and the plan diff fails. Neither setting reproduces the failure alone. Pin both, mirroring `03394_distributed_shuffle_join_with_prewhere`, which already pins them for the same reason. Verified: with the randomized setting set the plan output of both tests now matches the reference exactly (before the fix `BuildRuntimeFilter` was absent in both). Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit dbd998cb1e9457b331e66de7bb50b82e2e2db646) --- .../03394_distributed_shuffle_join_with_aggregation.sql | 4 ++++ .../0_stateless/03394_distributed_shuffle_join_with_in.sql | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/tests/queries/0_stateless/03394_distributed_shuffle_join_with_aggregation.sql b/tests/queries/0_stateless/03394_distributed_shuffle_join_with_aggregation.sql index cdf868a409fc..9f196df7108b 100644 --- a/tests/queries/0_stateless/03394_distributed_shuffle_join_with_aggregation.sql +++ b/tests/queries/0_stateless/03394_distributed_shuffle_join_with_aggregation.sql @@ -14,6 +14,10 @@ INSERT INTO test SELECT 'path_' || number::String, 'en', number FROM numbers(5); INSERT INTO test SELECT 'path_' || (number%3)::String, 'de', number%4 FROM numbers(10); SET query_plan_join_swap_table = 0; +SET query_plan_optimize_join_order_randomize = 0; -- Pinned because the test asserts on join plan/order +-- Pinned because the test asserts that the runtime join filter is built; the default threshold +-- skips it when the probe side is estimated to be tiny. +SET join_runtime_filter_min_probe_rows = 0; SET optimize_move_to_prewhere = 1, diff --git a/tests/queries/0_stateless/03394_distributed_shuffle_join_with_in.sql b/tests/queries/0_stateless/03394_distributed_shuffle_join_with_in.sql index 6e7680024dc9..655401b1ec40 100644 --- a/tests/queries/0_stateless/03394_distributed_shuffle_join_with_in.sql +++ b/tests/queries/0_stateless/03394_distributed_shuffle_join_with_in.sql @@ -14,6 +14,10 @@ INSERT INTO test SELECT 'path_' || number::String, 'en', number FROM numbers(5); INSERT INTO test SELECT 'path_' || (number%3)::String, 'de', number%4 FROM numbers(10); SET query_plan_join_swap_table = 0; +SET query_plan_optimize_join_order_randomize = 0; -- Pinned because the test asserts on join plan/order +-- Pinned because the test asserts that the runtime join filter is built; the default threshold +-- skips it when the probe side is estimated to be tiny. +SET join_runtime_filter_min_probe_rows = 0; SET From 18f7d17fa4a79f9637410143e0e9465fdf181825 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:55:06 +0000 Subject: [PATCH 10/19] Fix flaky 02423_ddl_for_opentelemetry by retrying the span-log read A span reaches system.opentelemetry_span_log only when its holder is destroyed: TracingContextHolder::~TracingContextHolder enqueues it via shared_span_log->add(...) (src/Common/OpenTelemetryTraceContext.cpp:469-472), and SpanHolder::finish() does the same at :207-210. SYSTEM FLUSH LOGS opentelemetry_span_log flushes only what is already queued, so it cannot flush a span whose holder is still alive. The TCPHandler SERVER span is exactly such a span. Its holder is declared one scope out of the query block with the comment "Initialized later. It has to be destroyed after query_state is destroyed." (src/Server/TCPHandler.cpp :572-573), is constructed at :604-609, and is destroyed only at the end of the connection-loop iteration, that is after query_state->finalizeOut(out) at :1153 has already sent the client its answer. check_span read the count once, so a flush issued by the very next statement could observe a short count and report [operation_name like 'TCPHandler' ]=0, expected: 1 while the test's own dump showed every other span for that trace id. Retry the read while the numeric count is below the expectation and let the existing exact comparison decide. The assertion, the failure line and the span dump are unchanged. This mirrors the merged fixes for the two sibling tests with the identical race, 02417_opentelemetry_insert_on_distributed_table and 01455_opentelemetry_distributed. The sleep budget is test-wide rather than per call: the default path has 11 check_span calls with a positive expectation, so a per-call bound would cost up to about 165 seconds when many assertions are short, and Fast test runs this file with --timeout 60 (ci/jobs/fast_test.py:378), which would kill the run and lose the diagnostic. A count already above the expectation is reported on the first iteration, and a site expecting 0 reads exactly once. Also break out immediately on a non-numeric result and quote the comparison operands, which removes a real shell error seen in CI when the client failed and the count came back empty: 02423_ddl_for_opentelemetry.sh: line 52: [: =: unary operator expected (cherry picked from commit c29108df684b8fa278d5dec1fb6b5b9717a277b2) --- .../02423_ddl_for_opentelemetry.sh | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/tests/queries/0_stateless/02423_ddl_for_opentelemetry.sh b/tests/queries/0_stateless/02423_ddl_for_opentelemetry.sh index 0dc7b7500797..ef1bff5d47d6 100755 --- a/tests/queries/0_stateless/02423_ddl_for_opentelemetry.sh +++ b/tests/queries/0_stateless/02423_ddl_for_opentelemetry.sh @@ -39,17 +39,31 @@ function check_span() extra_condition="" fi - ret=$(${CLICKHOUSE_CLIENT} -q " - SYSTEM FLUSH LOGS opentelemetry_span_log; - - SELECT count() - FROM system.opentelemetry_span_log - WHERE finish_date >= yesterday() - AND lower(hex(trace_id)) = '${2}' - AND operation_name like '${3}' - ${extra_condition};") - - if [ $ret = $1 ]; then + # A span is enqueued into the async opentelemetry_span_log only when its holder is + # destroyed, and the TCPHandler holder is destroyed after the client already has its + # answer, so a single flush+count can legitimately read a short count. Retry while the + # count is below the expectation; a non-numeric result (client or flush error) breaks + # out immediately, and the sleep budget is shared by the whole test so a genuinely + # broken assertion cannot multiply the added wall time by the number of call sites. + for _ in {1..30}; do + ret=$(${CLICKHOUSE_CLIENT} -q " + SYSTEM FLUSH LOGS opentelemetry_span_log; + + SELECT count() + FROM system.opentelemetry_span_log + WHERE finish_date >= yesterday() + AND lower(hex(trace_id)) = '${2}' + AND operation_name like '${3}' + ${extra_condition};") + + [[ "$ret" =~ ^[0-9]+$ ]] || break + [[ "$ret" -ge "$1" ]] && break + [[ "${span_poll_sleeps_left:-0}" -gt 0 ]] || break + span_poll_sleeps_left=$((span_poll_sleeps_left - 1)) + sleep 1 + done + + if [ "$ret" = "$1" ]; then echo 1 else echo "[operation_name like '${3}' ${extra_condition}]=$ret, expected: ${1}" @@ -79,6 +93,9 @@ cluster_name=$($CLICKHOUSE_CLIENT -q "select if(engine = 'Replicated', name, 'te # # Only format_version 4 enables the tracing # +# Total number of 1-second sleeps check_span may spend waiting for spans to be enqueued. +span_poll_sleeps_left=15 + for ddl_version in 3 4; do # Echo a separator so that the reference file is more clear for reading echo "===ddl_format_version ${ddl_version}====" From 2c152cf46c492c2734acf16ddfe58eaf20895d7e Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:00:40 +0000 Subject: [PATCH 11/19] Fix flaky test_partial_auth cleanup after widening the parent ACL test_keeper_auth/test.py::test_partial_auth[get_genuine_zk] could fail in its finally cleanup with NoAuthError while deleting /test_partial_acl_delete/subnode, right after the test had granted ALL on the parent. Only the Apache ZooKeeper parametrization was affected; the ClickHouse Keeper one passed the identical sequence. ZooKeeper authorizes a delete against the PARENT's ACL, and its setACL publishes an outstanding ChangeRecord that still carries the old ACL: the new ACL goes only into the SetACLTxn, and duplicate() copies acl verbatim while updating only stat.aversion. getRecordForPath prefers that outstanding record, so a delete prepped after the client's setACL reply but before that txn has been applied is authorized against the pre-setACL ACL, which this test deliberately creates with delete=False. Keeper instead writes the new acl_id into uncommitted state and reads it back in the delete preprocess, i.e. it is read-your-own-writes on ACLs where ZooKeeper is not - hence the asymmetry. This is why the earlier remedy did not hold: it added the two setACL calls that already succeed immediately before the delete that still fails. The permission is not the problem, its visibility is. Retry that one cleanup delete, only on NoAuthError, bounded by a wall-clock deadline. The subsequent parent delete is authorized against "/", whose ACL this test never touches, so it is left un-retried and still fails loudly. Validated by opening the window on demand (issuing the child delete before the parent setACL is acknowledged) against a 3-node ZooKeeper 3.6.2 ensemble: 10 failures in 40 iterations. With the helper at timeout=0 the failure still occurs (2 of 32 opened windows); at the shipped timeout=30 it does not (0 of 85). The in-body pytest.raises(NoAuthError) oracle was confirmed non-vacuous. 50 runs of both parametrizations and the whole 26-test file pass. (cherry picked from commit bd24ffde1e5b000bc7a650261cbe5678346a108d) --- tests/integration/test_keeper_auth/test.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_keeper_auth/test.py b/tests/integration/test_keeper_auth/test.py index 68a1c95e7d5c..ebf78ab4d6b3 100644 --- a/tests/integration/test_keeper_auth/test.py +++ b/tests/integration/test_keeper_auth/test.py @@ -61,6 +61,22 @@ def zk_stop_and_close(zk): zk.close() +# ZooKeeper checks delete against the PARENT's ACL, and its setACL leaves the +# outstanding change record carrying the old ACL (duplicate() copies acl verbatim +# and updates only stat.aversion), so a delete issued right after a widening +# setACL can still be authorized against the pre-setACL ACL and get NoAuthError. +def zk_delete_after_acl_change(zk, path, timeout=30.0): + deadline = time.monotonic() + timeout + while True: + try: + zk.delete(path) + return + except NoAuthError: + if time.monotonic() >= deadline: + raise + time.sleep(0.1) + + @pytest.mark.parametrize(("get_zk"), [get_genuine_zk, get_fake_zk]) def test_remove_acl(started_cluster, get_zk): auth_connection = None @@ -412,7 +428,9 @@ def test_partial_auth(started_cluster, get_zk): ) auth_connection.set_acls("/test_partial_acl_delete", acls=[acl]) auth_connection.set_acls("/test_partial_acl_delete/subnode", acls=[acl]) - auth_connection.delete("/test_partial_acl_delete/subnode") + zk_delete_after_acl_change(auth_connection, "/test_partial_acl_delete/subnode") + # Authorized against "/", whose ACL this test never touches, so a + # NoAuthError here would be a real problem: do not retry it. auth_connection.delete("/test_partial_acl_delete") zk_stop_and_close(auth_connection) From ca71e384cfa26c494ca8432b6312cd54884fef1a Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Tue, 4 Aug 2026 05:14:45 +0000 Subject: [PATCH 12/19] Fix flaky 01184_long_insert_values_huge_strings by pinning `max_threads` The test intermittently failed with a per-query memory limit error while reading the wide `String` column `s`: Code: 241. Query memory limit exceeded: would use 5.00 GiB (attempt to allocate chunk of 512.00 MiB), maximum: 4.66 GiB: (while reading column s) ... While executing MergeTreeSelect. (in query: select sum(h = cityHash64(s)) from huge_strings) This is the per-query `max_memory_usage` limit, not the server-wide `(total)` one; the two share `Code: 241` but are different failure modes. The cause is aggregate read-stream concurrency rather than one oversized allocation. `tests/clickhouse-test` draws `max_threads = 32` with probability 0.03, and `max_streams_to_max_threads_ratio` is 1 and not randomized, so about 32 read streams are created. Each stream deserializes `s` into its own `ColumnString::chars` buffer, and with rows up to 9 MB it grows that buffer through the doubling realloc in `SerializationString`. `Allocator::realloc` charges the new size before freeing the old one, so the throwing allocation is tested against everything already live across all streams. Meanwhile `max_memory_usage` is a fixed suite default of 4.66 GiB and is not randomized. The margin is thin: the observed rows report `would use` 4.70 to 5.04 GiB, so 1 to 8 percent above the limit. `SerializationString` is the last straw, not the defect. Pin `max_threads = 3` on the two verification queries that materialize `s`. Three is the top of the range 97 percent of runs already draw, so it is not an invented constant, and it cuts the concurrent buffer footprint by roughly 6x against an overshoot of a few percent. `select count()` is left unpinned: it is answered from metadata, with a measured peak of 0 B, and where the trivial count is disabled the smallest compressed column is chosen, which can never be `s`. The insert loops, which are the behavior under test, are untouched, and the assertions and the reference file are unchanged. Validated on an ASAN+UBSAN build against a forced worst-case fixture of 60 Compact parts holding 5 GiB of `s`. Both arms ran on one dataset and one binary with only the pin differing, and both carried the same `--max_threads 32` client option: unpinned 12/12 failures reproducing the signature above at a 4.6 GiB peak, pinned 0/12 at 645 MiB. The `sum(l = length(s))` line behaves the same under its `optimize_functions_to_subcolumns = 0` draw, failing 6/6 unpinned. Running the fixed test through `clickhouse-test` with `--client-option max_threads=32` confirms the query-level clause wins over the client option, with `system.query_log` recording an effective `max_threads` of 3 for the pinned queries and 32 for the unpinned count. 50 randomized runs pass, 3 of which drew the 32-thread value, with no change in runtime. Note this signature has a single public CI hit in the last 365 days, so a green CI run on this pull request is not evidence about the mechanism either way. (cherry picked from commit 967a951b012aa4239de2c883149b41a3083bb43b) --- .../0_stateless/01184_long_insert_values_huge_strings.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/queries/0_stateless/01184_long_insert_values_huge_strings.sh b/tests/queries/0_stateless/01184_long_insert_values_huge_strings.sh index 8d41c32467d2..0973bf5d17d2 100755 --- a/tests/queries/0_stateless/01184_long_insert_values_huge_strings.sh +++ b/tests/queries/0_stateless/01184_long_insert_values_huge_strings.sh @@ -17,7 +17,8 @@ done; wait $CLICKHOUSE_CLIENT -q "select count() from huge_strings" -$CLICKHOUSE_CLIENT -q "select sum(l = length(s)) from huge_strings" -$CLICKHOUSE_CLIENT -q "select sum(h = cityHash64(s)) from huge_strings" +# Pin `max_threads`: each read stream holds its own buffer for a ~9 MB row of `s`, so the randomized 32-thread draw exceeds `max_memory_usage`. +$CLICKHOUSE_CLIENT -q "select sum(l = length(s)) from huge_strings SETTINGS max_threads = 3" +$CLICKHOUSE_CLIENT -q "select sum(h = cityHash64(s)) from huge_strings SETTINGS max_threads = 3" $CLICKHOUSE_CLIENT -q "drop table huge_strings" From ee7319b3e72c3a74f8a491ee60bd7c1f2eb8166a Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Tue, 4 Aug 2026 13:50:27 +0000 Subject: [PATCH 13/19] Fix flaky 02703_max_local_read_bandwidth by restoring throttler rate headroom The test intermittently failed on pread_threadpool with only its 4th column wrong (1 1 1 0): QueryLocalReadThrottlerSleepMicroseconds came out 0 while the query was demonstrably slow and all bytes had passed the throttler. The 4th assertion is a coupled oracle. Throttler::throttle increments the bytes counter unconditionally, but the sleep counter only inside `if (block)`, and block requires tokens_value < 0. The token bucket only goes negative when bytes arrive faster than the cap, so once a loaded runner's read rate falls to about the cap the throttler correctly does not sleep and the assertion fails. With a 1 MiB/s cap over an 8 MB payload the fixture had only 1.115x of margin between the cap and the arrival rate at which the assertion breaks (measured), which a contended sanitizer runner erases. The nominal required sleep was also 8e6/1048576-1 = 6.63 s, already below the 7 s the first column demands, so that column had been passing on unrelated overhead; the in-file comment claiming "(8-1)/1=7 seconds" was wrong because '1M' is 1048576, not 1e6. The sleep time is not lost or misattributed to a pool thread: ThrottlerSleepMicroseconds is 0 as well in the reproduced failure, so no thread slept at all. Both counters are charged through the same CurrentThread::getProfileEvents() a few lines apart, and the pread_threadpool throttle call runs on the consuming pipeline thread in AsynchronousReadBufferFromFileDescriptor::nextImpl, not on a pool thread - ThreadPoolReader never throttles. Co-reduce the payload (1e6 -> 2e5 rows) and the cap ('1M' -> 160000 B/s) together, which raises the required sleep to 9 s at comparable wall clock, and rescale the two byte thresholds by the same factor so they keep asserting that the whole payload passed through the throttler. All four assertions, both time thresholds and the reference file are unchanged. This is the same fix that was merged for the sibling 04103_user_network_bandwidth_throttler (#103422). Reproduced deterministically by capping the arrival rate with max_execution_speed_bytes and max_threads=1: the unmodified test prints 1 1 1 0 on all three arms, and 1 1 1 1 with the injection absent. The measured failure boundary moves from 1.115x of the cap to at most 1.006x, and the test now also passes at arrival rates below its own cap. 50/50 clean local runs; runtime 23.9 s -> 28.3 s. No source change: the throttler behaved correctly at every arrival rate measured, so there is no product defect here. (cherry picked from commit a2a3a410c255100acd34d0710516508bcde28841) --- .../0_stateless/02703_max_local_read_bandwidth.sh | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/queries/0_stateless/02703_max_local_read_bandwidth.sh b/tests/queries/0_stateless/02703_max_local_read_bandwidth.sh index fb7f47613c1d..9a049fd34d7f 100755 --- a/tests/queries/0_stateless/02703_max_local_read_bandwidth.sh +++ b/tests/queries/0_stateless/02703_max_local_read_bandwidth.sh @@ -11,8 +11,10 @@ $CLICKHOUSE_CLIENT -m -q " create table data (key UInt64 CODEC(NONE)) engine=MergeTree() order by tuple() settings min_bytes_for_wide_part=1e9; " -# reading 1e6*8 bytes with 1M bandwith it should take (8-1)/1=7 seconds -$CLICKHOUSE_CLIENT -q "insert into data select * from numbers(1e6)" +# Reading 2e5*8 bytes at 160000 B/s takes 1.6e6/160000-1 = 9 seconds (-1 is the 1s token burst). +# The throttler only sleeps while the arrival rate exceeds the cap, so the cap must stay far +# below the natural read rate or the sleep assertion flaps on loaded runners. +$CLICKHOUSE_CLIENT -q "insert into data select * from numbers(2e5)" read_methods=( read @@ -25,14 +27,14 @@ read_methods=( ) for read_method in "${read_methods[@]}"; do query_id=$(random_str 10) - $CLICKHOUSE_CLIENT --query_id "$query_id" -q "select * from data format Null settings max_local_read_bandwidth='1M', local_filesystem_read_method='$read_method'" + $CLICKHOUSE_CLIENT --query_id "$query_id" -q "select * from data format Null settings max_local_read_bandwidth=160000, local_filesystem_read_method='$read_method'" $CLICKHOUSE_CLIENT -m -q " SYSTEM FLUSH LOGS query_log; SELECT '$read_method', query_duration_ms >= 7e3, - ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] > 8e6, - ProfileEvents['QueryLocalReadThrottlerBytes'] > 8e6, + ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] > 1.5e6, + ProfileEvents['QueryLocalReadThrottlerBytes'] > 1.5e6, ProfileEvents['QueryLocalReadThrottlerSleepMicroseconds'] > 7e6*0.5 FROM system.query_log WHERE event_date >= yesterday() AND event_time >= now() - 600 AND current_database = '$CLICKHOUSE_DATABASE' AND query_id = '$query_id' AND type != 'QueryStart' From 22b7646df6730f1942cc7a8b39777b939e435143 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:24:09 +0000 Subject: [PATCH 14/19] Fix flaky test_replication_credentials replication race test_same_credentials and test_no_credentials insert into one replica and then assert the table contents on the other, with a fixed time.sleep(1) as the only barrier. ReplicatedMergeTreeSink commits the part's /log/log-N znode in the same multi-op transaction that commits the part, so when the INSERT returns the log entry is durably in ZooKeeper. The other replica, however, learns of it only asynchronously: its queue_updating_task pulls the log, a background pool task executes the resulting GET_PART, and the part is fetched over the interserver HTTP endpoint and committed. None of that chain is bounded by anything the test controls, so on a loaded sanitizer runner it routinely exceeds one second and the assertion reads a stale replica: AssertionError: assert '111\n' == '111\n222\n' That reached master at cfc1fd2512eb276fe020907434207a08fbaa5e0b. Over the last 90 days CIDB has 7 occurrences of the lag signature - an AssertionError at one of the four reads that query the replica which did not receive the insert - across 4 distinct refs, spanning both tests. Over the same 90 days CIDB records 220751 OK and 13 FAIL results for these two tests, so the lag accounts for 7 of the 13 failures: a genuine low-rate race rather than a broken check. Replace the barrier instead of the timing constant: before each cross-replica assertion, the replica about to be read runs SYSTEM SYNC REPLICA test_table with an explicit timeout. waitForProcessingQueue first calls pullLogsToQueue(..., SYNC), so a pending GET_PART is guaranteed visible before the wait set is computed, then triggers the background assignee, then addSubscriber snapshots the queue's entry ids under state_mutex while registering the callback, so the wait cannot miss the entry it must wait for and returns as soon as the fetch lands. That turns an unbounded asynchronous wait into a deterministic barrier at no fixed cost. The file already uses this idiom at four other places. Raising the sleep was rejected: it treats the symptom and re-races on a slower runner. Scope is deliberately two tests and four lines. In test_different_credentials and test_credentials_and_no_credentials the sleep guards a negative assertion across intentionally mismatched interserver credentials, where replication must not happen; a sync there can never complete and fails with QueryTimeoutExceedException, which was measured rather than assumed. All eight assertions are left byte-identical, so the change only strengthens the barrier. Validated with the fetch stalled deliberately on the reading replica: the assertion fails before this change with the exact CI signature and passes after it on the same binary, and reverting only the new barrier reddens it again. 200/200 green over 50 repeats of the whole file. (cherry picked from commit 0e2550f7fa8cd91ad475cef63026908145bfab60) --- tests/integration/test_replication_credentials/test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_replication_credentials/test.py b/tests/integration/test_replication_credentials/test.py index e1ce61067d94..44df4ccd4f15 100644 --- a/tests/integration/test_replication_credentials/test.py +++ b/tests/integration/test_replication_credentials/test.py @@ -46,13 +46,13 @@ def same_credentials_cluster(): def test_same_credentials(same_credentials_cluster): node1.query("insert into test_table values ('2017-06-16', 111, 0)") - time.sleep(1) + node2.query("SYSTEM SYNC REPLICA test_table", timeout=60) assert node1.query("SELECT id FROM test_table order by id") == "111\n" assert node2.query("SELECT id FROM test_table order by id") == "111\n" node2.query("insert into test_table values ('2017-06-17', 222, 1)") - time.sleep(1) + node1.query("SYSTEM SYNC REPLICA test_table", timeout=60) assert node1.query("SELECT id FROM test_table order by id") == "111\n222\n" assert node2.query("SELECT id FROM test_table order by id") == "111\n222\n" @@ -85,13 +85,13 @@ def no_credentials_cluster(): def test_no_credentials(no_credentials_cluster): node3.query("insert into test_table values ('2017-06-18', 111, 0)") - time.sleep(1) + node4.query("SYSTEM SYNC REPLICA test_table", timeout=60) assert node3.query("SELECT id FROM test_table order by id") == "111\n" assert node4.query("SELECT id FROM test_table order by id") == "111\n" node4.query("insert into test_table values ('2017-06-19', 222, 1)") - time.sleep(1) + node3.query("SYSTEM SYNC REPLICA test_table", timeout=60) assert node3.query("SELECT id FROM test_table order by id") == "111\n222\n" assert node4.query("SELECT id FROM test_table order by id") == "111\n222\n" From d2bed358924bf2b52a5a9410074708827705896c Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Wed, 5 Aug 2026 12:13:17 +0000 Subject: [PATCH 15/19] Fix flaky 02040_clickhouse_benchmark_query_id_pass_through The test asserts that a clickhouse-benchmark run logs 3 queries under one initial_query_id. On a loaded runner it read 0 instead of 3. The benchmark process never ran its query. Connection::connect uses handshake_timeout_ms (default 10000, Settings.cpp:410) as the socket receive timeout for the server Hello read, and clickhouse-test gives the benchmark no timeout overrides (shell_config.sh builds CLICKHOUSE_BENCHMARK_OPT0 from only --port, --database and --log_comment, while the client gets connect_timeout and receive_timeout from the runner). When the accept/handshake path stalls for longer than 10 s the benchmark exits with SOCKET_TIMEOUT before sending any query, so query_log has no rows for that query_id and the assertion reads 0. In the reported run the server accepted no TCP connection for 22.50 s (21:45:14.247 to 21:45:36.747 in the job's clickhouse-server.log, zero TCPHandlerFactory accepts in between, and executeQuery starts fell to 0 for the 11 s from 21:45:17 to 21:45:27). The benchmark's connection was accepted at the end of that window and the server logged "Client has gone away", the benchmark having already given up. The job's query_log.tsv confirms it: the failing instance has 0 rows with client_name = 'ClickHouse benchmark', while each of the 6 in-place reruns has exactly 3. Give connect and handshake a generous budget, matching the same fix already merged for 01600_benchmark_query (#108570) and present in 03630_benchmark_accept_invalid_certificate and 03636_benchmark_error_messages. The assertion is unchanged, so what the test verifies is unchanged. Co-Authored-By: Claude Fable 5 (cherry picked from commit 71996125cb8e791d472b78f597e050f7bab06c58) --- .../02040_clickhouse_benchmark_query_id_pass_through.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/queries/0_stateless/02040_clickhouse_benchmark_query_id_pass_through.sh b/tests/queries/0_stateless/02040_clickhouse_benchmark_query_id_pass_through.sh index 59538534fa71..6d084cfd9858 100755 --- a/tests/queries/0_stateless/02040_clickhouse_benchmark_query_id_pass_through.sh +++ b/tests/queries/0_stateless/02040_clickhouse_benchmark_query_id_pass_through.sh @@ -6,6 +6,11 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) query_id="${CLICKHOUSE_DATABASE}_$$" benchmark_args=( + # A loaded runner can take longer than the default 10 s handshake_timeout_ms + # to send Hello; the benchmark then exits without running a query and + # query_log has 0 rows instead of 3. + --connect_timeout 60 + --handshake_timeout_ms 60000 --iterations 1 --log_queries 1 --query_id "$query_id" From 86cd68b27bd7c8c47b9f9c306eb656d5942f530e Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Wed, 5 Aug 2026 16:51:55 +0000 Subject: [PATCH 16/19] Fix flaky test_tcp_handler_connection_limits The test counted every `Done processing connection.` message in the server log and asserted that its own connection was the only one that closed. Two unrelated connections break that count: * The readiness probe of `cluster.start` connects to the TCP port and closes it without sending any data. The port is bound and listening from `createServers` onwards, so the kernel completes the handshake long before the server starts accepting; the probe is served only once startup finishes, which can be after the test has already sampled the initial count. This is what fails on MSan, where startup takes seconds: Application: Ready for connections. TCPHandlerFactory: TCP Request. Address: 172.16.2.1:59612 TCPHandler: Client has not sent any data. TCPHandler: Done processing connection. <-- counted, but not sampled * `clickhouse-client` reconnects after the server drops it, so a single run of the test can close two connections by itself. That is the shape of the earlier failures, where both tests reported a delta of two. Count the connections closed *because a limit was reached* instead - the server logs `Closing connection due to limits` exactly once per such connection, and the reason distinguishes the query-count limit from the time limit. Neither the readiness probe nor a reconnecting client reaches a limit: `query_count` and `connection_timer` are per-connection, so a fresh connection starts from zero. With the count no longer polluted by startup connections, the `sleep` that tried to wait them out is not needed. https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=7d22d13ab50ff1474fad83ba8e19db7b60c24412&name_0=MasterCI&name_1=Integration%20tests%20%28amd_msan%2C%203%2F8%29 Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 2938a9d9adf481f960bd543a0f0db28257707652) --- .../test.py | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/tests/integration/test_tcp_handler_connection_limits/test.py b/tests/integration/test_tcp_handler_connection_limits/test.py index ef9a35f40f2a..ac9ec0a4eddd 100644 --- a/tests/integration/test_tcp_handler_connection_limits/test.py +++ b/tests/integration/test_tcp_handler_connection_limits/test.py @@ -1,6 +1,5 @@ import pytest import subprocess -import time from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) @@ -14,11 +13,6 @@ def started_cluster(): finally: cluster.shutdown() -@pytest.fixture(scope="module", autouse=True) -def stabilize_container(started_cluster): - """Wait for container startup processes to complete before running tests""" - time.sleep(1) - def execute_queries_persistent_connection(queries): """Execute multiple queries through a single persistent clickhouse-client connection""" proc = subprocess.Popen( @@ -34,17 +28,19 @@ def execute_queries_persistent_connection(queries): return stdout, stderr -def get_connection_done_count(): - try: - log_result = node.exec_in_container( - ["grep", "-c", "Done processing connection", "/var/log/clickhouse-server/clickhouse-server.log"] - ) - return int(log_result.strip()) - except Exception: - return 0 +def get_limit_closed_count(reason): + """Count the connections that the server closed because a limit was reached. + + Counting every closed connection instead would be racy: the readiness probe of + `cluster.start` connects to the port and closes it without sending any data, and the + server accepts that connection only once it starts serving, which can happen after the + test has already sampled the initial count. Connections closed for other reasons never + report a limit, so counting only those keeps the assertion exact. + """ + return int(node.count_in_log(f"Closing connection due to limits: {reason}").strip()) def test_query_count_limit(started_cluster): - initial_count = get_connection_done_count() + initial_count = get_limit_closed_count("queries=") queries = ["SELECT 1;", "SELECT 2;", "SELECT 3;", "SELECT 4;", "SELECT 5;"] stdout, stderr = execute_queries_persistent_connection(queries) @@ -53,11 +49,11 @@ def test_query_count_limit(started_cluster): assert "4" not in stdout and "5" not in stdout assert "TCP_CONNECTION_LIMIT_REACHED" in stderr - final_count = get_connection_done_count() + final_count = get_limit_closed_count("queries=") assert final_count == initial_count + 1, f"Expected exactly 1 connection closure, got {final_count - initial_count}" def test_time_limit(started_cluster): - initial_count = get_connection_done_count() + initial_count = get_limit_closed_count("elapsed=") queries = ["SELECT sleep(3);", "SELECT 1;", "SELECT 2;"] stdout, stderr = execute_queries_persistent_connection(queries) @@ -65,5 +61,5 @@ def test_time_limit(started_cluster): assert "1" not in stdout and "2" not in stdout assert "TCP_CONNECTION_LIMIT_REACHED" in stderr - final_count = get_connection_done_count() + final_count = get_limit_closed_count("elapsed=") assert final_count == initial_count + 1, f"Expected exactly 1 connection closure, got {final_count - initial_count}" From 68d2299300e3d248278766f3f5aa4ab3c4b73882 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:47:20 +0000 Subject: [PATCH 17/19] Fix flaky 02559_add_parts: assert active_parts after DETACH PARTITION The fourth assertion in this test snapshotted `parts`, `active_parts` and `total_marks` from `system.tables` immediately after `ALTER TABLE ... DETACH PARTITION 1` and required exactly `2 1 2`. Two of those three columns are not well defined at that point. `parts` is `getAllPartsCount()` = `data_parts_by_info.size()` and `total_marks` sums `getMarksCount()` over the same container, so both count parts in *every* state, `Outdated` included. `DETACH PARTITION` does not erase the detached part: it covers it with an empty level+1 part, flipping the original to `Outdated`, and then makes a single best-effort synchronous reclamation pass. That pass may legitimately remove nothing. `grabOldParts` declines when it cannot immediately take `grab_old_parts_mutex` (the per-table cleanup thread calls the same function every second by default), and it skips any part whose `DataPartPtr` is still held elsewhere, for example by a concurrent read. In those cases the detached part remains in `data_parts_by_info` and both counters include it, so the test reads `3 1 4`. `active_parts` is the `total_active_size_parts` atomic, maintained under the parts lock alongside every state transition, and it is 1 in every state reachable here. The final query now asserts only that column. The first three assertions are unchanged and still check `parts` and `total_marks` exactly: no part is ever `Outdated` before the DETACH, and each of the two partitions holds a single part, so nothing is mergeable and the all-states and active-only counters coincide. Note that `SETTINGS old_parts_lifetime = 0` does not fix this. The ownership and lock-contention checks in `grabOldParts` are evaluated before the removal-time gate, so the race survives; it would only change which value the test expects. Co-Authored-By: Claude Fable 5 (cherry picked from commit 5af2fa94c216403b0a9886bac65475f4dc16403b) --- tests/queries/0_stateless/02559_add_parts.reference | 2 +- tests/queries/0_stateless/02559_add_parts.sql | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/queries/0_stateless/02559_add_parts.reference b/tests/queries/0_stateless/02559_add_parts.reference index 50bd3725056e..845bf13dd8d9 100644 --- a/tests/queries/0_stateless/02559_add_parts.reference +++ b/tests/queries/0_stateless/02559_add_parts.reference @@ -1,4 +1,4 @@ 0 0 0 1 1 2 2 2 4 -2 1 2 +1 diff --git a/tests/queries/0_stateless/02559_add_parts.sql b/tests/queries/0_stateless/02559_add_parts.sql index 9f4e85a32589..b8f427a90537 100644 --- a/tests/queries/0_stateless/02559_add_parts.sql +++ b/tests/queries/0_stateless/02559_add_parts.sql @@ -16,5 +16,7 @@ SELECT parts, active_parts,total_marks FROM system.tables WHERE name = 'check_sy INSERT INTO check_system_tables VALUES (1, 2, 1); SELECT parts, active_parts,total_marks FROM system.tables WHERE name = 'check_system_tables' AND database = currentDatabase(); ALTER TABLE check_system_tables DETACH PARTITION 1; -SELECT parts, active_parts,total_marks FROM system.tables WHERE name = 'check_system_tables' AND database = currentDatabase(); +-- `parts` and `total_marks` count Outdated parts too, and reclamation after DETACH is best-effort, +-- so only `active_parts` is well defined here. +SELECT active_parts FROM system.tables WHERE name = 'check_system_tables' AND database = currentDatabase(); DROP TABLE IF EXISTS check_system_tables; From a332460f62fdacab66174e0b989f5a7088001e03 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:05:48 +0000 Subject: [PATCH 18/19] Fix flaky test_named_collections_encrypted2 by syncing the kazoo client before reads The test's kazoo client is pinned to a single ensemble member, zoo1 (`cluster.get_kazoo_client("zoo1")` -> `helpers/cluster.py:4672` connects to that one container), while the server's session spans zoo1/zoo2/zoo3 (`helpers/zookeeper_config.xml`). `CREATE NAMED COLLECTION` writes synchronously and commits on leader+quorum before the query returns (`NamedCollectionsMetadataStorage.cpp:313` -> `ZooKeeper.cpp:906`), so there is no server-side durability gap. But `quorum_reads` defaults to false (`src/Coordination/CoordinationSettings.cpp:62`, not overridden by any integration keeper config), so a follower read is not linearizable: if zoo1 has not yet applied the committed transaction to its in-memory state, its `get` answers from the pre-write snapshot and raises `NoNodeError`. The read is also effectively single-shot. `KazooClientWithImplicitRetries.get` routes through `KazooRetry`, whose retry set is `(ConnectionLoss, OperationTimeoutError, ForceRetryError)` plus `SessionExpiredError`; `NoNodeError` is not a member, so `KazooRetry.__call__` re-raises it on the first attempt. `zk.sync(path)` forces the connected follower to catch up with the leader before responding, eliminating the race rather than waiting it out. It is a real raft barrier, not a hint: `ZooKeeperSyncRequest::isReadRequest()` returns false (`src/Common/ZooKeeper/ZooKeeperCommon.h:124`) and `OpNum::Sync` is classified as a write (`ZooKeeperConstants.cpp:132`), and kazoo's `sync` blocks until the response is acknowledged. This mirrors 9476b38b6109097482ad7a0f2531516b94a350f8, which fixed the same class in the sibling module `tests/integration/test_named_collections/test.py` with eight `zk.sync(ZK_PATH)` insertions, and follows the older pattern in `tests/integration/test_drop_replica/test.py`. The two sites changed here are the only unpolled external kazoo reads left in the file; the reads at `wait_zk_child_exists`/`wait_zk_child_absent` are bounded polling loops and are left alone. Fixing `check_encrypted` covers five tests (test_zookeeper_encrypted_storage, test_encryption_persists_after_restart, test_special_characters_and_unicode, test_many_keys, test_survives_restart); the inline read in test_new_replica_encrypted_data_integrity needs its own line, and is where a second occurrence of this failure was recorded. Every existing assertion is unchanged, so a genuinely missing or unencrypted znode still fails the test. (cherry picked from commit e1857b623a04c62bb4046fc8fb733d4614bca33a) --- tests/integration/test_named_collections_encrypted2/test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/test_named_collections_encrypted2/test.py b/tests/integration/test_named_collections_encrypted2/test.py index aaa0d7989982..b7e29e127b4c 100644 --- a/tests/integration/test_named_collections_encrypted2/test.py +++ b/tests/integration/test_named_collections_encrypted2/test.py @@ -62,6 +62,7 @@ def wait_not_exists(node, collection, timeout=10): def check_encrypted(zk, collection): + zk.sync(ZK_PATH) content = zk.get(f"{ZK_PATH}/{collection}.sql")[0] assert content[:3] == b"ENC" return content @@ -717,6 +718,7 @@ def test_new_replica_encrypted_data_integrity(stopped_node3): password='P@ssw0rd!Complex#123' """) + zk.sync(ZK_PATH) content = zk.get(f"{ZK_PATH}/encrypted_coll.sql")[0] assert content[:3] == b"ENC" assert b"super_secret_api_key_12345" not in content From 687328938572eb8eb6360885ea6fd70c3ab57b77 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Fri, 7 Aug 2026 01:27:55 +0000 Subject: [PATCH 19/19] Fix flaky 04357_table_readonly_background_moves The test asserts that a read-only table's part stays on the local volume while a writable control table's part is moved to the remote volume in the background. It failed once on amd_tsan with both disk_name reads flipped to s3_disk, the writable control line in between still passing: the read-only table's part was already remote at the first observation, before the test had marked the table read-only at all. This is not a product defect. table_readonly is set after the INSERT, so no move guard was bypassed; the guard was simply never handed a local part to hold in place. The fixture encoded a wall-clock assumption instead. SYSTEM STOP MOVES cancels parts_mover.moves_blocker, which only the background mover reads, so it cannot gate the INSERT's own space reservation: when a part's move TTL is already expired at write time, tryReserveSpacePreferringTTLRules reserves directly on the TTL destination volume, because perform_ttl_move_on_insert defaults to true and the local_remote policy did not override it. The interval that must stay under the 5 second TTL is therefore inside the INSERT statement, from the now() constant fixed at query analysis time to the time(nullptr) read at reservation, and a loaded runner can exceed it. Disable perform_ttl_move_on_insert on that volume so a part always starts on the local volume however long the INSERT takes. MergeTreePartsMover never reads the flag, so the part stays move-eligible and the writable control table still moves, which is what keeps the test meaningful: reverting only the table_readonly guard in MergeTreeData::scheduleDataMovingJob makes the test fail again with "readonly disk after control moved: s3_disk". The assertion, the reference file and the test tags are unchanged. The signature has one occurrence in 180 days and none on master. The other two tests using this shared policy move parts with explicit ALTER ... MOVE and declare no TTL, so an insert-time TTL-move flag cannot reach them. (cherry picked from commit 767e8b42afca252cbf183337a3475de614d1ce6e) --- tests/config/config.d/storage_conf.xml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/config/config.d/storage_conf.xml b/tests/config/config.d/storage_conf.xml index f046b9c31dce..1f80b8ae05a4 100644 --- a/tests/config/config.d/storage_conf.xml +++ b/tests/config/config.d/storage_conf.xml @@ -112,7 +112,13 @@ default - s3_disk + + + s3_disk + 0 +