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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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