From 6b9c084a2bc033fb6cc3943fb64edc9ea7568bbb Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Mon, 3 Aug 2026 22:01:42 +0000 Subject: [PATCH 01/17] Fix flaky test_replicated_database_and_unavailable_s3: wait for the digest rewrite The test overwrites the `digest` znode of the node2 replica with the sentinel value 123456, restarts node2, and asserts that the digest is no longer the sentinel, i.e. that the replica recomputed it during startup. The assertion read the znode exactly once, immediately after `restart_clickhouse` returned. That return only guarantees that the server answers a query (`Instance.wait_start` polls `SELECT 20`), but the digest is rewritten from a background job: `DatabaseReplicated::startupDatabaseAsync` schedules the `startup Replicated database` load job on TablesLoaderBackgroundStartupPoolId, and only that job runs `initDDLWorkerUnlocked` -> `DatabaseReplicatedDDLWorker::initializeReplication`, which is what sets the znode. The query interface is up before the job has run, so the single read can still observe the sentinel and fail spuriously. The window is widest on slow builds, which matches where this is seen (amd_asan_ubsan). Poll the digest until it changes, bounded by 60 seconds of wall-clock time with a 1 second sleep between reads, instead of reading it once. The assertion is unchanged, so a replica that genuinely never recomputes the digest still fails the test: it just no longer depends on the background job winning a race against the next statement. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 0739c135df0f851a25b0a1f4fde8d8b1307119ea) --- tests/integration/test_storage_delta/test.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_storage_delta/test.py b/tests/integration/test_storage_delta/test.py index 5d9400a04584..39684966f801 100644 --- a/tests/integration/test_storage_delta/test.py +++ b/tests/integration/test_storage_delta/test.py @@ -1701,12 +1701,20 @@ def test_replicated_database_and_unavailable_s3(started_cluster, use_delta_kerne node2.restart_clickhouse() - assert ( - node2.query( + # `restart_clickhouse` only waits until the server answers a query, but the + # digest is rewritten by the background `startup Replicated database` job, so + # poll instead of reading it once. + digest = None + deadline = time.monotonic() + 60 + while time.monotonic() < deadline: + digest = node2.query( f"SELECT value FROM system.zookeeper WHERE path = '{replica_path}' AND name = 'digest'" ).strip() - != "123456" - ) + if digest != "123456": + break + time.sleep(1) + + assert digest != "123456" def test_session_token(started_cluster): From 5c06df5669ca329ad2ff1a29c33045de11f20d31 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Tue, 4 Aug 2026 03:56:19 +0000 Subject: [PATCH 02/17] Fix flaky 02932_refreshable_materialized_views_2 step <27: cancelled> The step asserted that SYSTEM CANCEL VIEW records a non-empty exception, but it only did `sleep 1` before cancelling. That guarantees the refresh has started, not that it is still running when the cancel actually lands ~19 s later, so on a slow host the refresh completes successfully first and the cancel is a no-op on a finished attempt. The server then correctly reports an empty exception via the StorageSystemViewRefreshes ladder (last_attempt_succeeded is true), and the test fails with `Scheduled 0` instead of `Scheduled 1`. Proven from the CI server log of the failing run: a 17.70 s client-spawn stall (the largest inter-query gap of that run) let the refresh finish at 17:50:52.629 while the cancel arrived at 17:50:54.423. That cancel emits neither `Cancelling refresh in ...` nor `Code: 394 QUERY_WAS_CANCELLED`, while the two other cancels in the same run emit both, i.e. the interrupt found execution.executor already null. Replace the timing guess with a hard barrier instead of widening it. The refresh is parked at the existing `infinite_sleep` failpoint, which `sleepEachRow` hits inside `executor.execute()`, so the executor is installed and the cancel reaches the live pipeline exactly as the old step intended. The assertion moves to 04105_system_pause_view.sh, which is already `no-parallel` (a failpoint is server-global) and already owns the refresh cancel-semantics assertions, so 02932 keeps its parallel-safe tags. The relocated assertion is stronger than the one it replaces: same live-executor cancellation path and same `exception != ''` question, but behind a barrier rather than a 1 s guess, plus two discriminators the original lacked - the exception must contain `cancelled`, and `Cancelling refresh in ...` must appear in the log, which only happens when the interrupt finds a non-null executor. Degrading the new block back to the timed shape and injecting the measured stall reproduces the CI failure (`0 0 0`); restoring the barrier with the same stall injected passes. 02932 also gets faster, since a step with a `sleep 1` plus a poll loop is gone. (cherry picked from commit ffd1a9d623d98f2e73ea4e521d32f90ce9107c7f) --- ...refreshable_materialized_views_2.reference | 1 - .../02932_refreshable_materialized_views_2.sh | 10 ---- .../04105_system_pause_view.reference | 1 + .../0_stateless/04105_system_pause_view.sh | 53 +++++++++++++++++++ 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/tests/queries/0_stateless/02932_refreshable_materialized_views_2.reference b/tests/queries/0_stateless/02932_refreshable_materialized_views_2.reference index 7861f52373f8..235d5ef0ef5c 100644 --- a/tests/queries/0_stateless/02932_refreshable_materialized_views_2.reference +++ b/tests/queries/0_stateless/02932_refreshable_materialized_views_2.reference @@ -5,7 +5,6 @@ <23: simple refresh> 1 <24: rename during refresh> 1 <25: rename during refresh> rmv_f Running -<27: cancelled> rmv_f Scheduled 1 <28: drop during refresh> 0 0 CREATE MATERIALIZED VIEW default.rmv_g\nREFRESH EVERY 1 WEEK OFFSET 3 DAY 4 HOUR RANDOMIZE FOR 4 DAY 1 HOUR\n(\n `x` Int64\n)\nENGINE = Memory\nDEFINER = default SQL SECURITY DEFINER\nAS SELECT 42 AS x diff --git a/tests/queries/0_stateless/02932_refreshable_materialized_views_2.sh b/tests/queries/0_stateless/02932_refreshable_materialized_views_2.sh index 6553895b6356..741318f89719 100755 --- a/tests/queries/0_stateless/02932_refreshable_materialized_views_2.sh +++ b/tests/queries/0_stateless/02932_refreshable_materialized_views_2.sh @@ -82,16 +82,6 @@ $CLICKHOUSE_CLIENT -q " select '<24: rename during refresh>', * from rmv_f;" query_no_scheduling "select '<25: rename during refresh>', view, status from refreshes where view = 'rmv_f'" $CLICKHOUSE_CLIENT -q "alter table rmv_f modify refresh after 10 year settings refresh_retries = 0;" -sleep 1 # make it likely that at least one row was processed -# Cancel. -$CLICKHOUSE_CLIENT -q " - system cancel view rmv_f;" -while [ "`$CLICKHOUSE_CLIENT -q "select status from refreshes -- $LINENO" | xargs`" != 'Scheduled' ] -do - sleep 0.5 -done -# Check that another refresh doesn't immediately start after the cancelled one. -query_no_scheduling "select '<27: cancelled>', view, status, exception != '' from refreshes where view = 'rmv_f'" $CLICKHOUSE_CLIENT -q "system refresh view rmv_f;" while [ "`$CLICKHOUSE_CLIENT -q "select status from refreshes where view = 'rmv_f' -- $LINENO" | xargs`" != 'Running' ] do diff --git a/tests/queries/0_stateless/04105_system_pause_view.reference b/tests/queries/0_stateless/04105_system_pause_view.reference index e97f918273c3..33cbc01c2312 100644 --- a/tests/queries/0_stateless/04105_system_pause_view.reference +++ b/tests/queries/0_stateless/04105_system_pause_view.reference @@ -6,3 +6,4 @@ <5: start views resumes all> 2 2 <6: granted pause works> <6: denied pause errors as expected> +<7: cancel during refresh records an exception> 1 1 1 diff --git a/tests/queries/0_stateless/04105_system_pause_view.sh b/tests/queries/0_stateless/04105_system_pause_view.sh index f5ece885ae33..3d3306fc695f 100755 --- a/tests/queries/0_stateless/04105_system_pause_view.sh +++ b/tests/queries/0_stateless/04105_system_pause_view.sh @@ -216,3 +216,56 @@ $CLICKHOUSE_CLIENT -q " drop table denied; drop table src; drop user $test_user;" + +# --------------------------------------------------------------------------- +# Test 5: SYSTEM CANCEL VIEW records the cancellation as an exception. +# +# The cancel must reach a live `PipelineExecutor`, so the refresh is parked at the +# `infinite_sleep` failpoint (hit from `sleepEachRow` inside `executor.execute()`) +# instead of being timed. `infinite_sleep` is server-global and fires on every +# `sleep`/`sleepEachRow` call, so this block must stay LAST in the file: the views +# above use `sleepEachRow(1)` and would park too. +# --------------------------------------------------------------------------- + +# `SYSTEM DISABLE FAILPOINT` is also the resume mechanism below; this trap only covers an +# early exit between the enable and that disable, which would otherwise leave the global +# failpoint active and park every later `sleep`/`sleepEachRow` call in the run. Disabling an +# already-disabled failpoint is a no-op. +trap ' + $CLICKHOUSE_CLIENT -q "SYSTEM DISABLE FAILPOINT infinite_sleep" 2>/dev/null || true +' EXIT + +$CLICKHOUSE_CLIENT -q " + create table src (x Int64) engine Memory; + insert into src values (1); + create materialized view c refresh every 1 year settings refresh_retries = 0 (x Int64) engine Memory empty as + select x + sleepEachRow(0) as x from src settings max_block_size = 1, max_threads = 1; + system enable failpoint infinite_sleep; + system refresh view c;" + +if ! timeout 60 $CLICKHOUSE_CLIENT -q "SYSTEM WAIT FAILPOINT infinite_sleep PAUSE" +then + echo "FAIL: refresh did not reach the infinite_sleep failpoint" +fi + +# The refresh is parked inside `executor.execute()`, so the cancel cannot be outrun. Disabling +# the failpoint resumes it. +$CLICKHOUSE_CLIENT -q " + system cancel view c; + system disable failpoint infinite_sleep;" + +wait_status c Scheduled + +# `Cancelling refresh in ...` is logged only when the interrupt finds a non-null +# `execution.executor`, so it proves the cancel hit the running pipeline rather than an +# already-finished attempt. Matching 'cancelled' distinguishes a cancellation from any +# other refresh failure. +$CLICKHOUSE_CLIENT -q " + system flush logs text_log; + select '<7: cancel during refresh records an exception>', + (select exception != '' from refreshes where view = 'c'), + (select position(exception, 'cancelled') > 0 from refreshes where view = 'c'), + (select count() > 0 from system.text_log + where message = 'Cancelling refresh in ' || currentDatabase() || '.c'); + drop table c; + drop table src;" From a564460c959f4fb6a912d2392e24000e543e6f65 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 03/17] 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 843e0c4b84bc1be8e3803f6a42086ea3b3292970 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 04/17] 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 f061ae41b6fd71bf673196fafb2dcecff6f61143 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 05/17] 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 7d46bef3b8f2..5e0fb1b088aa 100644 --- a/tests/integration/test_replication_credentials/test.py +++ b/tests/integration/test_replication_credentials/test.py @@ -58,13 +58,13 @@ def test_same_credentials(same_credentials_cluster): node1.query("TRUNCATE TABLE test_table") node2.query("SYSTEM SYNC REPLICA test_table", timeout=10) 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" @@ -99,13 +99,13 @@ def test_no_credentials(no_credentials_cluster): node3.query("TRUNCATE TABLE test_table") node4.query("SYSTEM SYNC REPLICA test_table", timeout=10) 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 b281f3f96e9faf5bdcc0c74123d376ad8b916983 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 06/17] 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 bef0f38eda746adc07260105057f0fd2a9088e31 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Wed, 5 Aug 2026 16:51:55 +0000 Subject: [PATCH 07/17] 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 26fcab4c1351cf25e79c896e90df9485486d236f 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 08/17] 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 ed061362a831b6fc10f0829e8fee18ace6c9c130 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Wed, 5 Aug 2026 21:11:11 +0000 Subject: [PATCH 09/17] Fix flaky 03469_json_read_subcolumns_combined_2_compact_merge_tree timeouts The test times out on slow builds when the settings randomizer draws a small `index_granularity`. All 19 failures of this test on `Stateless tests (amd_msan, WasmEdge, parallel, 2/2)` in the last 200 days are `Reason: Timeout!` at 600050-600360 ms against the runner's 600 s per-test timeout; none contains `result differs`, `Received exception` or `Logical error`, so there is no wrong-results component. `index_granularity` is what discriminates. Extracted per failure, it is in [100, 211] in all 19 rows, while `ratio_of_defaults_for_sparse_serialization` takes both extremes and eight interior values among the same failures, and `index_granularity_bytes` spans its whole range. The test writes 80000 rows into a Compact part and then runs 71 SELECTs over 32 distinct JSON subcolumn expressions, up to 23 in a single query; marks scale as rows / `index_granularity`, so a small granularity multiplies the per-query mark work. That range is 380-800 marks per part, against 10 at the engine default. Measured locally on a debug build: `index_granularity` 107 gives 747 marks and a mean wall time of 8.06 s, while 8192 gives 11 marks and 1.69 s - a 4.76x ratio over four interleaved runs per arm, with non-overlapping ranges and byte-identical output in both arms. The test already carries an `index_granularity=(100, None)` limit, but it has been inert since 82d324be4ecbf4f raised the slow-build generator to `randint(100, 65536)`: a lower bound of 100 can no longer raise anything. Simulating the runner's own clamp over 100000 draws, 12.24% of slow-build values land below 8192 under the old bound and 0% under the new one. Raise only that one bound to the engine default. Randomization of the other MergeTree settings, of all query settings, and of the 8192-65536 granularity range is unchanged; no tag is added and no check is disabled. The clamp mechanism is used sparingly, so this is a narrow change: only 40 stateless tests bound `index_granularity` at all (`git grep -l 'Random settings limits.*index_granularity=' -- tests/queries/0_stateless/`), and this PR moves one of those 40 bounds. Flooring at 8192 is already precedented for a JSON subcolumn test by 03246_alter_from_string_to_json, and the sibling `03469_..._2_wide_merge_tree`, which runs the same 71 queries, pins `index_granularity=8192` in its own DDL. Also record beside the limits line why the bound is load-bearing, so a later cleanup does not read it as redundant and lower it again. Co-Authored-By: Claude Opus 4.8 (cherry picked from commit 52721e737b3b86c9218a36f9ae6c5b93d4b72368) --- ...469_json_read_subcolumns_combined_2_compact_merge_tree.sql | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/03469_json_read_subcolumns_combined_2_compact_merge_tree.sql b/tests/queries/0_stateless/03469_json_read_subcolumns_combined_2_compact_merge_tree.sql index 773e2c24f142..a34894479dcf 100644 --- a/tests/queries/0_stateless/03469_json_read_subcolumns_combined_2_compact_merge_tree.sql +++ b/tests/queries/0_stateless/03469_json_read_subcolumns_combined_2_compact_merge_tree.sql @@ -1,5 +1,7 @@ -- Tags: no-fasttest, long --- Random settings limits: index_granularity=(100, None); index_granularity_bytes=(100000, None); max_threads=(4, 32) +-- Random settings limits: index_granularity=(8192, None); index_granularity_bytes=(100000, None); max_threads=(4, 32) +-- index_granularity is floored at the engine default on purpose: tiny granules multiply the +-- mark count over these 71 SELECTs and timed the test out on slow builds. SET enable_json_type = 1; set allow_experimental_variant_type = 1; From fb361afb20c595048a73dc6df6e5fa4c98339e48 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 10/17] 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 bf2df9767875..647447a9a6c7 100644 --- a/tests/integration/test_named_collections_encrypted2/test.py +++ b/tests/integration/test_named_collections_encrypted2/test.py @@ -94,6 +94,7 @@ def wait_zk_child_absent(zk, path, child, 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 @@ -749,6 +750,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 fcfd036154f8a11cdc5e5f02bccfa419f6614309 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Thu, 6 Aug 2026 14:49:14 +0000 Subject: [PATCH 11/17] Fix flaky test_trace_log_memory_context: force a new global memory peak The test asserts that `MemoryPeak` events with `memory_context = 'Global'` appear in `system.trace_log`, but such events are sent only when the server-wide memory usage grows past its previous peak by at least `total_memory_profiler_step` (4 MiB). When the peak reached during server startup is higher than anything the test's small queries allocate, no `MemoryPeak` event can ever be produced and the existing retry loop cannot help - the test fails with `assert 0 > 0`. Force a new global peak by allocating an increasing amount of memory (~100 MB more per attempt) inside the retry loop. Seen in https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=102402&sha=f8b38f834a2e63df6820ad1e750205d84276799a&name_0=PR&name_1=Integration%20tests%20%28arm_binary%2C%20distributed%20plan%2C%202%2F4%29 of https://github.com/ClickHouse/ClickHouse/pull/102402 Co-Authored-By: Claude Fable 5 (cherry picked from commit cd9d6abde6d52f5c780a104a65a6b79dc1617daa) --- tests/integration/test_trace_log_memory_context/test.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_trace_log_memory_context/test.py b/tests/integration/test_trace_log_memory_context/test.py index 47ad24fa12d3..a6f52c55b58d 100644 --- a/tests/integration/test_trace_log_memory_context/test.py +++ b/tests/integration/test_trace_log_memory_context/test.py @@ -58,13 +58,20 @@ def get_trace_events(memory_context, memory_blocked_context, trace_type, query_i # server-wide allocations including background activity, not just # this query's allocations. # Retrying bounds the test runtime while keeping it reliable. - for _ in range(0, 15): + for attempt in range(0, 15): # Generate some logs to generate entries with memory_blocked_context=Global and trace_type=JemallocSample for i in range(10): node.query("SELECT logTrace('foo')") query_id = uuid.uuid4().hex node.query("SELECT * FROM numbers(100000) ORDER BY number", query_id=query_id) + # `Memory`/`MemoryPeak` with `memory_context = 'Global'` are sent only when the + # server-wide memory usage grows past its previous peak by at least + # `total_memory_profiler_step` (4 MiB). The peak reached during server startup can + # be above anything the small queries here allocate, in which case retrying alone + # never helps. Force a new global peak by allocating more memory on every attempt. + node.query(f"SELECT groupArray(number) FROM numbers({(attempt + 1) * 12500000}) FORMAT Null") + node.query("SYSTEM FLUSH LOGS system.trace_log") if ( get_trace_events("Unknown", "Max", "MemorySample", query_id) > 0 and From 6d69fe9b26023bdcc9da25324d204d70d1d03533 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 12/17] 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 a5216fc05249..1ef64249f2eb 100644 --- a/tests/config/config.d/storage_conf.xml +++ b/tests/config/config.d/storage_conf.xml @@ -123,7 +123,13 @@ default - s3_disk + + + s3_disk + 0 + From 403cced1ab29263ddd05df90e1dcedd46164c218 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 7 Aug 2026 12:37:52 +0000 Subject: [PATCH 13/17] 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 ecc05c4ab30c2b4762f42cb2251b3291226e9d63 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 14/17] 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 79f095bd35bb..a46083c95379 100644 --- a/tests/queries/0_stateless/03717_async_deduplication_with_mv.sql +++ b/tests/queries/0_stateless/03717_async_deduplication_with_mv.sql @@ -62,7 +62,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 6f043648067d068713d7f3402c48734a2bb5b7bf Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 Aug 2026 10:17:15 +0000 Subject: [PATCH 15/17] 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 891e474f6e38..a4b418916618 100644 --- a/tests/integration/test_keeper_dynamic_log_level/test.py +++ b/tests/integration/test_keeper_dynamic_log_level/test.py @@ -60,21 +60,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", @@ -85,5 +88,7 @@ def test_adjust_log_level(start_cluster): user="root", ) ) - >= 1 - ) + if trace_lines >= 1: + break + time.sleep(1) + assert trace_lines >= 1 From ade1bd77399599c0b6ffa60cc3f6b90a73d73e03 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 16/17] 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 5b73a79b65e8..01e7effb5595 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 0f0d519b4e79eddbe4ded48179c5ff11be63a46a Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Sun, 9 Aug 2026 19:08:12 +0000 Subject: [PATCH 17/17] Fix flaky test_attach_table_from_s3_plain_readonly: scope the upload to the table upload_to_minio was pointed at the node's entire database/store directory. That is the live, bind-mounted data root of every Atomic database, and the system database is Atomic in server mode (loadMetadataSystem in src/Interpreters/loadMetadata.cpp), so system.query_log and its siblings live under the very tree the test walks. Those logs flush on a background timer, so between os.walk enumerating a path and open() reading it the server can rename a temp part into place (tmp_insert_ prefix in MergeTreeDataWriter) or remove a part that has just been merged away (clearPartsFromFilesystemImpl in MergeTreeData). The resulting FileNotFoundError is not caught by the except S3Error handler, so the test fails with a different missing file each time. The failing path proves the racing writer is a system log rather than the table under test: local_db.test_table has no PARTITION BY, so its partition id is the literal "all" and its only possible part is all_1_1_0, whereas the reported part is 202608_..., the toYYYYMM(event_date) partition every system log uses. Scope the walk to store//, the one table the test means to copy, by hoisting the UUID lookup that already existed a few lines further down. Object keys are built as minio_path + relative_to(local_path), so moving those two path segments from the walk root into the minio_path prefix leaves every uploaded key byte-identical and the ATTACH resolves exactly as before. The table is quiesced before the walk, so the new walk root has no concurrent writer and the race is removed by construction rather than filtered. Skipping tmp_* entries or catching FileNotFoundError were both considered and rejected: neither can distinguish a harmless vanished system-log part from a genuinely missing file of the table under test, which would turn a loud failure into a silently incomplete upload. Scoping also stops the test uploading the whole system database into the bucket, which is what its own comment always claimed it did, and makes it noticeably faster. (cherry picked from commit 52d3e4520df50c8378dd6c12a8285931d66340c0) --- .../test.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/integration/test_attach_table_from_s3_plain_readonly/test.py b/tests/integration/test_attach_table_from_s3_plain_readonly/test.py index 2aae3563d955..140551a9fc29 100644 --- a/tests/integration/test_attach_table_from_s3_plain_readonly/test.py +++ b/tests/integration/test_attach_table_from_s3_plain_readonly/test.py @@ -77,17 +77,25 @@ def test_attach_table_from_s3_plain_readonly(started_cluster): assert int(node1.query("select num from local_db.test_table limit 1")) == 5 - # Copy local MergeTree data into minio bucket - table_data_path = os.path.join(node1.path, "database/store") + table_uuid = node1.query( + "SELECT uuid FROM system.tables WHERE database='local_db' AND table='test_table'" + ).strip() + + # Copy local MergeTree data into minio bucket. Scoped to this table: store/ + # also holds the Atomic `system` database, whose parts a background flush can + # remove mid-walk. + table_data_path = os.path.join( + node1.path, "database/store", table_uuid[:3], table_uuid + ) minio = cluster.minio_client upload_to_minio( - minio, cluster.minio_bucket, table_data_path, "data/disks/disk_s3_plain/store/" + minio, + cluster.minio_bucket, + table_data_path, + f"data/disks/disk_s3_plain/store/{table_uuid[:3]}/{table_uuid}/", ) # Drop the non-replicated table, we don't need it anymore - table_uuid = node1.query( - "SELECT uuid FROM system.tables WHERE database='local_db' AND table='test_table'" - ).strip() node1.query("drop table local_db.test_table SYNC;") # Create a replicated database