From 8800f5d2d8d3c1f650e4c2229566ca9708624ef8 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 01/15] 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 823a5e4d46b6b4f8956dca4afa08fdb2917ad33d 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 02/15] 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 becb223a7b0d73b389ef3e0bd466e3e3d8cc76e3 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 03/15] 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 6ac8566e039c4ad0fc1d4509aa0bbcc664a157f2 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 04/15] 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 fa8434eb929b92fe58c2e60829de228c4ac74de6 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Wed, 5 Aug 2026 16:51:55 +0000 Subject: [PATCH 05/15] 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 cdbb609719c5b49a83b8cce25eba3eb90c347b44 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 06/15] 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 f086bdafb2355c52eca911b5bb8a8cd25259db92 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 07/15] 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 233c128add25dae36c73555688e14c4d6fff402b 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 08/15] 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 + From 1b717a06b6738f2ef4622d91519e40724778570a Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 7 Aug 2026 12:37:52 +0000 Subject: [PATCH 09/15] Fix flaky test_dirty_pages_force_purge: lower purge threshold The test query peaks at ~417 MiB, but the purge was triggered only when jemalloc dirty pages exceeded 4 GiB * 0.2 = 819 MiB. Since jemalloc reuses dirty pages within the same arenas across iterations, `pdirty` can plateau below the threshold when few arenas are touched (e.g. when the number of query threads is lowered by `MemoryTrackerUtils` on a constrained CI machine), so the `MemoryAllocatorPurge` event never fired and the test timed out. Lower the ratio to 0.05 (~205 MiB) so a single iteration of the test query reliably exceeds the threshold. Seen in: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=110188&sha=5b0dd0156a543c7e72ceb73c0ebb890d53cb1f83&name_0=PR&name_1=Integration%20tests%20%28arm_binary%2C%20distributed%20plan%2C%204%2F4%29 PR: https://github.com/ClickHouse/ClickHouse/pull/110188 Co-Authored-By: Claude Fable 5 (cherry picked from commit b87486d3f05a7105cd80b7ffbe6963513da7c326) --- .../test_dirty_pages_force_purge/configs/overrides.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_dirty_pages_force_purge/configs/overrides.yaml b/tests/integration/test_dirty_pages_force_purge/configs/overrides.yaml index 195236e51dde..229d9175b265 100644 --- a/tests/integration/test_dirty_pages_force_purge/configs/overrides.yaml +++ b/tests/integration/test_dirty_pages_force_purge/configs/overrides.yaml @@ -1,3 +1,7 @@ --- max_server_memory_usage: 4Gi -memory_worker_purge_dirty_pages_threshold_ratio: 0.2 +# The threshold must be reliably exceeded by a single iteration of the test query +# (peak usage ~417 MiB). With a higher ratio, `pdirty` may plateau below the threshold, +# because jemalloc reuses dirty pages within the same arenas across iterations, +# and the number of touched arenas depends on the machine and the number of threads. +memory_worker_purge_dirty_pages_threshold_ratio: 0.05 From 155a4b114dc38d7e314a557f3e337ebc3acfda2d Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Sat, 8 Aug 2026 00:02:24 +0000 Subject: [PATCH 10/15] Fix flaky 03717_async_deduplication_with_mv losing a row The test loses a row: `03717_table` returns 4 rows instead of 5 and the two `count()` materialized views undercount by 1. Observed on the flaky check (`Stateless tests (amd_asan_ubsan, flaky check)`), 1 FAIL against 3 OK in that job, the failing run being the slowest of the four. The cause is a synchronization gap in the test rather than deduplication. It inserts with `wait_for_async_insert = 0` and drains with the table-scoped `SYSTEM FLUSH ASYNC INSERT QUEUE`, which by design collects `futures_to_wait` only from the containers that call itself moved out of `shard.queue` and waits on exactly those; its own comment states the limitation, and `flushAll` additionally calls `pool.wait()` and says its wait also covers jobs scheduled earlier. With `async_insert_use_adaptive_busy_timeout = 0`, `getBusyWaitTimeoutMs` returns `async_insert_busy_timeout_max_ms` unconditionally, so the container deadline was 5000 ms and `async_insert_busy_timeout_min_ms` was unused. When `processBatchDeadlines` pops the second batch before the flush statement arrives, the flush finds nothing matching, waits on nothing, and the following `SELECT` runs while that batch is still in flight. Rows `1` and `3` of the second batch are content duplicates and would be filtered anyway, so the only user-visible difference is the missing `5` plus the two aggregates short by one, which is exactly the observed diff. Raise `async_insert_busy_timeout_max_ms` to 600000 so the scoped flush is the only thing that can drain the batch. The other two auto-schedule triggers cannot fire here, since `async_insert_max_data_size` is 10485760 against a few bytes and `async_insert_max_query_number` is 450 against three entries, and `max_busy_timeout_exceeded` is gated on adaptive being on, which the test disables. This is the same remedy as 89d47b3c9a7f34d for `03148_async_queries_in_query_log_errors`, whose message documents this mechanism; of the 27 stateless tests combining a scoped flush with `wait_for_async_insert = 0`, 19 now pin a large maximum including this one, the deduplication siblings 03652, 03662, 04603 and 04614 at 600000. The reference stays byte-identical and no source file is touched. Reproduced deterministically with the `async_insert_flush_pause_in_executor` failpoint. Reading `value1` of the flush's own `Will wait for finishing of N flushing jobs` row from `system.text_log`, scoped by `query_id`, gives 0 at the previous 5000 ms pin with a 10 s insert-to-flush gap and 1 at 600000, and `Found duplicate block IDs` occurs 0 times for the repro's own table in every arm. End to end, forcing the pin to 1 ms fails 2 of 8 runs with a diff identical to the reported one while 600000 passes 8 of 8, and 50 of 50 randomized runs pass. The 37 async-insert and flush tests run identically on both arms with no regressions. Trade-off, the same one the siblings and the precedent already accept: a genuine server-side failure to flush now surfaces as the test's own timeout rather than a wrong answer. Five other class members still pin 5000 and three more inherit that value from `tests/config/users.d/timeouts.xml`, but none has an observed failure of this signature and the discriminator is the insert-to-flush gap rather than the pin, so they are left alone. Co-Authored-By: Claude Opus 5 (cherry picked from commit ebec05871ccc88aa1cf2ea5b368accaf0fac98b1) --- .../queries/0_stateless/03717_async_deduplication_with_mv.sql | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/03717_async_deduplication_with_mv.sql b/tests/queries/0_stateless/03717_async_deduplication_with_mv.sql index da7179fd7d27..a3bf674eda97 100644 --- a/tests/queries/0_stateless/03717_async_deduplication_with_mv.sql +++ b/tests/queries/0_stateless/03717_async_deduplication_with_mv.sql @@ -64,7 +64,9 @@ SELECT count() as value FROM 03717_table; SET async_insert = 1, insert_deduplicate = 1, async_insert_deduplicate = 1, wait_for_async_insert = 0, deduplicate_blocks_in_dependent_materialized_views=1; -set async_insert_use_adaptive_busy_timeout=0, async_insert_busy_timeout_min_ms=1000, async_insert_busy_timeout_max_ms=5000; +-- The busy timeout must outlast this test: the table-scoped flush below waits only for the jobs it +-- schedules itself, so a batch the deadline timer already drained is not waited for at all. +set async_insert_use_adaptive_busy_timeout=0, async_insert_busy_timeout_min_ms=1000, async_insert_busy_timeout_max_ms=600000; SET max_block_size=1; SET max_insert_block_size=1; From 1c70349a56e69c691e25c65b7a4b1ef4ee2f4b4b Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 Aug 2026 10:17:15 +0000 Subject: [PATCH 11/15] Fix flaky `test_keeper_dynamic_log_level`: poll for the log level change The test wrote a new logger config, slept a fixed 3 seconds and expected the asynchronous config reloader to have already applied the `trace` level, so on slow runs (e.g. MSan) the final assertion saw zero `` lines: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=91062&sha=5bd33a1f41d843f69d93cb75e2192bb668e61368&name_0=PR&name_1=Integration%20tests%20%28amd_msan%2C%204%2F8%29 Related: https://github.com/ClickHouse/ClickHouse/pull/91062 Poll for up to a minute until trace logging becomes active instead. (cherry picked from commit 0b51ec04cb06c39ab8f3eee8af1a8ea0f6d81ca3) --- .../test_keeper_dynamic_log_level/test.py | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/tests/integration/test_keeper_dynamic_log_level/test.py b/tests/integration/test_keeper_dynamic_log_level/test.py index db212fc4ca0f..bb91d425b53a 100644 --- a/tests/integration/test_keeper_dynamic_log_level/test.py +++ b/tests/integration/test_keeper_dynamic_log_level/test.py @@ -61,21 +61,24 @@ def test_adjust_log_level(start_cluster): """, ] ) - time.sleep(3) - node.query( - "SELECT * FROM system.zookeeper SETTINGS allow_unrestricted_reads_from_keeper = 'true'" - ) - node.exec_in_container( - [ - "bash", - "-c", - "sync", - ], - privileged=True, - user="root", - ) - assert ( - int( + # The config reloader applies the new logger settings asynchronously (it polls the config + # every couple of seconds), so poll until trace logging becomes active instead of relying + # on a fixed sleep, which is not enough on slow (e.g. sanitizer) runs. + trace_lines = 0 + for _ in range(60): + node.query( + "SELECT * FROM system.zookeeper SETTINGS allow_unrestricted_reads_from_keeper = 'true'" + ) + node.exec_in_container( + [ + "bash", + "-c", + "sync", + ], + privileged=True, + user="root", + ) + trace_lines = int( node.exec_in_container( [ "bash", @@ -86,5 +89,7 @@ def test_adjust_log_level(start_cluster): user="root", ) ) - >= 1 - ) + if trace_lines >= 1: + break + time.sleep(1) + assert trace_lines >= 1 From b1e7acd985de59aa172a779465a51b3187447384 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Sun, 9 Aug 2026 13:03:00 +0000 Subject: [PATCH 12/15] Fix flaky test_concurrent_watches losing one watch test_keeper_back_to_back::test_concurrent_watches fails as `assert 999 == 1000`. It is a test bug, not a Keeper defect. trigger_watch mutated a path before it owned the corresponding existing_path token, and removed the token afterwards inside a bare `except: pass`. Since random.choice + set() + remove() is not atomic across the 10 pool threads, two threads can select the same token when only one is present. A watch is one-shot per (path, session), so the second mutation notifies nobody, and the deferred remove then consumes a token belonging to a different, still outstanding registration. That registration is never mutated again, so its callback never fires and the aggregate count lands one short. In the failing run this happened on /487: register, two mutations 1 ms apart, one event, a second register, then a stale remove ate its token, while the run's last mutation was ~1.8 s later on another path. The expected count is 1000 rather than the number of distinct paths because kazoo fans a single event out to every callback registered for that path, which is why the test creates a new closure per registration. Claim the token atomically before mutating and skip when nothing was claimed, so that every registration is followed by a mutation. All four assertions are kept verbatim, including the exact count: the test now satisfies its own precondition instead of relying on the race not firing. tests/integration/test_keeper_watches/test.py is the in-tree precedent for that discipline. The lock covers only the test's list bookkeeping and never a Keeper call, so the threads still issue overlapping requests and the multi-watcher collisions remain (372 duplicate registrations, up to 6 on one path). Validated with a deterministic replay of the observed schedule against a real Keeper (pre-fix body fails 3/3, post-fix passes 3/3), by four mutation arms that each redden the retained assertion, and by 50/50 repetitions under the job that reported the failure. (cherry picked from commit 3e1ba66d81752226b09de598112a4e812fa3053e) --- .../test_keeper_back_to_back/test.py | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/tests/integration/test_keeper_back_to_back/test.py b/tests/integration/test_keeper_back_to_back/test.py index a16909709c53..49f9779eb6cd 100644 --- a/tests/integration/test_keeper_back_to_back/test.py +++ b/tests/integration/test_keeper_back_to_back/test.py @@ -1,6 +1,7 @@ import os import random import string +import threading import time from multiprocessing.dummy import Pool @@ -755,9 +756,19 @@ def test_concurrent_watches(started_cluster, request): all_paths_triggered = [] existing_path = [] + # A watch is one-shot per (path, session), so a mutation only notifies a registration + # that is live at that moment. Mutating a path this thread does not hold a token for can + # therefore consume another thread's registration, leaving it forever unnotified. + existing_path_lock = threading.Lock() all_paths_created = [] watches_created = 0 + def claim_path(): + with existing_path_lock: + if not existing_path: + return None + return existing_path.pop(random.randrange(len(existing_path))) + def create_path_and_watch(i): nonlocal watches_created nonlocal all_paths_created @@ -775,7 +786,8 @@ def dumb_watch(event): fake_zk.get(global_path + "/" + str(i), watch=dumb_watch) all_paths_created.append(global_path + "/" + str(i)) watches_created += 1 - existing_path.append(i) + with existing_path_lock: + existing_path.append(i) trigger_called = 0 @@ -783,26 +795,19 @@ def trigger_watch(i): nonlocal trigger_called trigger_called += 1 fake_zk.set(global_path + "/" + str(i), b"somevalue") - try: - existing_path.remove(i) - except: - pass def call(total): for i in range(total): create_path_and_watch(random.randint(0, 1000)) time.sleep(random.random() % 0.5) - try: - rand_num = random.choice(existing_path) - trigger_watch(rand_num) - except: - pass - while existing_path: - try: - rand_num = random.choice(existing_path) + rand_num = claim_path() + if rand_num is not None: trigger_watch(rand_num) - except: - pass + while True: + rand_num = claim_path() + if rand_num is None: + break + trigger_watch(rand_num) p = Pool(10) arguments = [100] * 10 From 8c54c658f75b3284c124179864b1b1e7c87057ae Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Mon, 10 Aug 2026 20:42:43 +0000 Subject: [PATCH 13/15] Fix flaky 03100_lwu_07_merge_patches The test lists the five patch parts produced by its five lightweight UPDATEs from system.parts, and only then runs an explicit OPTIMIZE TABLE ... PARTITION ID 'patch-...' FINAL and asserts they collapsed into one part. That first listing was not deterministic. Patch parts are ordinary background merge candidates: MergeTreePartsCollector declares affordable_kinds{Regular, Patch} (same in the replicated collector), and nothing in selection exempts them. Five tiny parts in one partition are an ideal merge range, so a background merge races the test's next statement and sometimes collapses them into all_3_11__10 with 13 rows before the first listing runs. PR #108594 masked the merge level in the patch part names, which fixed the variant where only the level differed. This is the same race caught earlier, where the block range and the row count change too, so a level mask cannot express it. Pinning max_bytes_to_merge_at_max_space_in_pool = 1 stops background merges from selecting the patch parts, while the explicit OPTIMIZE FINAL still merges them: it goes through selectAllPartsToMergeWithinPartition, which never reads that limit. The setting's own documentation states this contract, and 04546_text_index_merge_fsync and 04654_text_index_merge_with_empty_part already use the idiom for the same purpose. The assertion is unchanged and .reference is untouched. Validated by forcing the background merge to win deterministically: without the pin the test fails with exactly the reported diff, with the pin it passes and the explicit OPTIMIZE still merges the five parts into one. 50/50 passes under full randomization. (cherry picked from commit a899045640573e91215d7da0c3addbb1a81da599) --- tests/queries/0_stateless/03100_lwu_07_merge_patches.sql | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/03100_lwu_07_merge_patches.sql b/tests/queries/0_stateless/03100_lwu_07_merge_patches.sql index c2060f0711c3..2dc9d49ad00e 100644 --- a/tests/queries/0_stateless/03100_lwu_07_merge_patches.sql +++ b/tests/queries/0_stateless/03100_lwu_07_merge_patches.sql @@ -3,7 +3,10 @@ SET enable_lightweight_update = 1; CREATE TABLE t_lightweight (id UInt64, c1 UInt64) ENGINE = MergeTree ORDER BY id -SETTINGS enable_block_number_column = 1, enable_block_offset_column = 1; +SETTINGS enable_block_number_column = 1, enable_block_offset_column = 1, + -- the patch parts are listed one by one below, so only the explicit OPTIMIZE FINAL + -- (which ignores this limit) may merge them + max_bytes_to_merge_at_max_space_in_pool = 1; INSERT INTO t_lightweight SELECT number, number FROM numbers(20); From f116b0f86e41f587a8268a9d2b5cb5d162ed1bd2 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Tue, 11 Aug 2026 02:51:34 +0000 Subject: [PATCH 14/15] Fix flaky Q4 of 03753_join_runtime_filter_dynamically_disable Q4 asserts RuntimeFilterBlocksProcessed = 0, which only the bloom filter can produce: with join_runtime_filter_exact_values_limit=1 and join_runtime_bloom_filter_bytes=100, ApproximateRuntimeFilter takes 2000 keys into 800 bits, so checkBloomFilterWorthiness sees an expected fill rate of 0.9994 against max_ratio_of_set_bits=0.7 and calls setFullyDisabled(). That short-circuits shouldSkip for every block, so findImpl and updateStats never run and nothing is counted as processed. join_runtime_filter_from_fixed_hash_table, whose default flipped to true in 26.6, lets HashJoin::publishSharedRuntimeFilters replace that fully-disabled bloom filter with a SharedFixedHashTableRuntimeFilter. That implementation has no full-disable path at all: setFullyDisabled has exactly two call sites in the tree and neither is reachable from it, and its finishInsertImpl is empty. The only remaining throttle is the pass-ratio heuristic in updateStats, and Q4's probe keys 5, 6, 7 and 100 all lie inside the build range 0..1999, so every probed row passes, the ratio stays at 1.0, and the filter probes one block then skips thirty, indefinitely. With max_block_size=10 that splits the 302 blocks into 12 processed and 290 skipped, so Processed = 0 is unreachable. Which implementation serves the join is not pinned by the test. The file sets join_algorithm = 'hash,parallel_hash', and PlannerJoins picks ConcurrentHashJoin when rhs_size_estimation is absent and plain HashJoin when it is present and below parallel_hash_join_threshold. ConcurrentHashJoin overrides neither hasPostBuildPhase nor runPostBuildPhase, so it never publishes the shared filter and Q4 passes there. rhs_size_estimation is overwritten from the process-global HashTablesStatistics cache, which is written when a join with the same cache key is destroyed and which collect_hash_table_stats_during_joins populates by default. Whether Q4 sees an estimate therefore depends on what ran earlier in the same server process, which is why the failure is invisible on master, appears in parallel jobs where one server runs the suite concurrently, and is fully deterministic once it starts: the in-job diagnostic rerun without randomized settings failed 104 out of 104. Pin join_runtime_filter_from_fixed_hash_table = 0 in Q4's SETTINGS clause, selecting the implementation whose dynamic-disable behaviour Q4 exists to test. The same pin for the same reason already exists at 04357_join_runtime_filter_size_from_hash_table_stats.sql:13. The pin is scoped to Q4 alone so the other six queries keep the default and the file still covers the shared fixed hash table filter end to end; 04241 remains its dedicated test. Verified locally: the forced arms give 12/290 with the setting on and 0/302 with it off, RuntimeFiltersCreated = 1 in both; the test fails before this change and passes after on one server and binary; 50 runs with randomized settings and 50 without all pass; and reverting the pin reddens the test 5 out of 5. (cherry picked from commit 0266e3d171bb30e06e5a61bc39f18c0bd99b29cb) --- .../03753_join_runtime_filter_dynamically_disable.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/03753_join_runtime_filter_dynamically_disable.sql b/tests/queries/0_stateless/03753_join_runtime_filter_dynamically_disable.sql index fbe2d940d885..0c0a70bbcfc8 100644 --- a/tests/queries/0_stateless/03753_join_runtime_filter_dynamically_disable.sql +++ b/tests/queries/0_stateless/03753_join_runtime_filter_dynamically_disable.sql @@ -91,7 +91,7 @@ SELECT count() FROM customer, numbers(2000) AS n WHERE c_nationkey = n.number::Int32 -SETTINGS join_runtime_filter_exact_values_limit=1, join_runtime_bloom_filter_bytes=100, max_block_size=10, max_threads=1, log_comment='Q4'; +SETTINGS join_runtime_filter_exact_values_limit=1, join_runtime_bloom_filter_bytes=100, join_runtime_filter_from_fixed_hash_table=0, max_block_size=10, max_threads=1, log_comment='Q4'; -- Check all blocks were skipped From 4843d4912d22bf8613fb913f1fc412c689355748 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Thu, 13 Aug 2026 07:40:19 +0000 Subject: [PATCH 15/15] Fix flaky 02999_scalar_subqueries_bug_2 The test asserted that the scalar subquery in a materialized view definition is not executed at CREATE time by racing a 2 second max_execution_time against a 3 second sleepEachRow. That makes the assertion a function of machine load rather than of the property under test, and it failed on master in Stateless tests (amd_tsan, parallel): Code: 159. DB::Exception: Timeout exceeded: maximum: 2000 ms. (TIMEOUT_EXCEEDED) The message has no "elapsed ... ms" clause. QueryStatus::throwProperExceptionIfNeeded emits that prefix only for a non-zero elapsed, so the throw came from throwIfKilled via executeQuery.cpp, where the deadline fires while the query is still pending (slow to analyze or plan). Every query including DDL is registered with the CancellationChecker watchdog unconditionally, and the in-sleep check in sleep.cpp always reports a non-zero elapsed. The scalar was therefore never executed: the engine behaved correctly and only the clock failed. max_execution_time is not randomized by the test runner, so there is no setting to pin, and widening the bound was already tried on the structural twin (upstream #91744) without holding. Remove the timing oracle and assert the property directly with throwIf(1), following the fix for the twin 03356_analyzer_unused_scalar_subquery in #111983. throwIf::isSuitableForConstantFolding() returns false, the same guard sleepEachRow relies on, so the rewrite keeps the mechanism the old oracle depended on while detecting execution at all rather than only execution slower than 2 seconds. Cover CREATE TABLE ... EMPTY AS SELECT as well, and add an executed-position arm that must fail so the check has teeth. Validated on both analyzer paths at 50/50, 8 concurrent copies, and with two mutations that each redden the new oracle. The reference file stays empty. (cherry picked from commit 316855e01c00c01ff3bcd22a498a64ea64443f81) --- .../0_stateless/02999_scalar_subqueries_bug_2.sql | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/queries/0_stateless/02999_scalar_subqueries_bug_2.sql b/tests/queries/0_stateless/02999_scalar_subqueries_bug_2.sql index 03ac91e401ae..cf9ea1b004c5 100644 --- a/tests/queries/0_stateless/02999_scalar_subqueries_bug_2.sql +++ b/tests/queries/0_stateless/02999_scalar_subqueries_bug_2.sql @@ -2,6 +2,7 @@ drop table if exists source; drop table if exists target1; drop table if exists target2; drop table if exists v_heavy; +drop table if exists t_as_select; create table source(type String) engine=MergeTree order by type; @@ -13,6 +14,12 @@ select count(*) n from (select number from numbers(1e5) n1 cross join nums); create table target1(type String) engine=MergeTree order by type; create table target2(type String) engine=MergeTree order by type; -set max_execution_time=2; --- we should not execute scalar subquery here -create materialized view vm_target2 to target2 as select * from source where type='two' and (select sum(sleepEachRow(0.1)) from numbers(30)); +-- A scalar subquery in a statement that is only analyzed, never executed, must not be +-- evaluated, so `throwIf` never fires. This is checked with `throwIf` rather than with a +-- timeout, because a timeout makes the test depend on machine load. +create materialized view vm_target2 to target2 as select * from source where type='two' and (select throwIf(1)); + +create table t_as_select engine=MergeTree order by tuple() empty as select * from source where type='two' and (select throwIf(1)); + +-- But it is evaluated when the statement is actually executed. +select * from source where type='two' and (select throwIf(1)) format Null; -- { serverError FUNCTION_THROW_IF_VALUE_IS_NON_ZERO }