Stable-26.3 - Automated backport of flaky-fix commits from upstream (2026-08-17) - #2195
Open
github-actions[bot] wants to merge 15 commits into
Open
Stable-26.3 - Automated backport of flaky-fix commits from upstream (2026-08-17)#2195github-actions[bot] wants to merge 15 commits into
github-actions[bot] wants to merge 15 commits into
Conversation
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 967a951)
…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 (ClickHouse#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 a2a3a41)
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 cfc1fd2. 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 0e2550f)
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 (ClickHouse#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 <noreply@anthropic.com> (cherry picked from commit 7199612)
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) <noreply@anthropic.com>
(cherry picked from commit 2938a9d)
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 <noreply@anthropic.com> (cherry picked from commit 5af2fa9)
…nt 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 9476b38, 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 e1857b6)
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 767e8b4)
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: ClickHouse#110188 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit b87486d)
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 89d47b3 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 <noreply@anthropic.com> (cherry picked from commit ebec058)
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 `<Trace>` 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: ClickHouse#91062 Poll for up to a minute until trace logging becomes active instead. (cherry picked from commit 0b51ec0)
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 3e1ba66)
Author
The test lists the five patch parts produced by its five lightweight UPDATEs
from system.parts, and only then runs an explicit
OPTIMIZE TABLE ... PARTITION ID 'patch-...' FINAL and asserts they collapsed
into one part. That first listing was not deterministic.
Patch parts are ordinary background merge candidates: MergeTreePartsCollector
declares affordable_kinds{Regular, Patch} (same in the replicated collector),
and nothing in selection exempts them. Five tiny parts in one partition are an
ideal merge range, so a background merge races the test's next statement and
sometimes collapses them into all_3_11_<lvl>_10 with 13 rows before the first
listing runs.
PR ClickHouse#108594 masked the merge level in the patch part names, which fixed the
variant where only the level differed. This is the same race caught earlier,
where the block range and the row count change too, so a level mask cannot
express it.
Pinning max_bytes_to_merge_at_max_space_in_pool = 1 stops background merges
from selecting the patch parts, while the explicit OPTIMIZE FINAL still merges
them: it goes through selectAllPartsToMergeWithinPartition, which never reads
that limit. The setting's own documentation states this contract, and
04546_text_index_merge_fsync and 04654_text_index_merge_with_empty_part
already use the idiom for the same purpose. The assertion is unchanged and
.reference is untouched.
Validated by forcing the background merge to win deterministically: without
the pin the test fails with exactly the reported diff, with the pin it passes
and the explicit OPTIMIZE still merges the five parts into one. 50/50 passes
under full randomization.
(cherry picked from commit a899045)
Q4 asserts RuntimeFilterBlocksProcessed = 0, which only the bloom filter can produce: with join_runtime_filter_exact_values_limit=1 and join_runtime_bloom_filter_bytes=100, ApproximateRuntimeFilter takes 2000 keys into 800 bits, so checkBloomFilterWorthiness sees an expected fill rate of 0.9994 against max_ratio_of_set_bits=0.7 and calls setFullyDisabled(). That short-circuits shouldSkip for every block, so findImpl and updateStats never run and nothing is counted as processed. join_runtime_filter_from_fixed_hash_table, whose default flipped to true in 26.6, lets HashJoin::publishSharedRuntimeFilters replace that fully-disabled bloom filter with a SharedFixedHashTableRuntimeFilter. That implementation has no full-disable path at all: setFullyDisabled has exactly two call sites in the tree and neither is reachable from it, and its finishInsertImpl is empty. The only remaining throttle is the pass-ratio heuristic in updateStats, and Q4's probe keys 5, 6, 7 and 100 all lie inside the build range 0..1999, so every probed row passes, the ratio stays at 1.0, and the filter probes one block then skips thirty, indefinitely. With max_block_size=10 that splits the 302 blocks into 12 processed and 290 skipped, so Processed = 0 is unreachable. Which implementation serves the join is not pinned by the test. The file sets join_algorithm = 'hash,parallel_hash', and PlannerJoins picks ConcurrentHashJoin when rhs_size_estimation is absent and plain HashJoin when it is present and below parallel_hash_join_threshold. ConcurrentHashJoin overrides neither hasPostBuildPhase nor runPostBuildPhase, so it never publishes the shared filter and Q4 passes there. rhs_size_estimation is overwritten from the process-global HashTablesStatistics cache, which is written when a join with the same cache key is destroyed and which collect_hash_table_stats_during_joins populates by default. Whether Q4 sees an estimate therefore depends on what ran earlier in the same server process, which is why the failure is invisible on master, appears in parallel jobs where one server runs the suite concurrently, and is fully deterministic once it starts: the in-job diagnostic rerun without randomized settings failed 104 out of 104. Pin join_runtime_filter_from_fixed_hash_table = 0 in Q4's SETTINGS clause, selecting the implementation whose dynamic-disable behaviour Q4 exists to test. The same pin for the same reason already exists at 04357_join_runtime_filter_size_from_hash_table_stats.sql:13. The pin is scoped to Q4 alone so the other six queries keep the default and the file still covers the shared fixed hash table filter end to end; 04241 remains its dedicated test. Verified locally: the forced arms give 12/290 with the setting on and 0/302 with it off, RuntimeFiltersCreated = 1 in both; the test fails before this change and passes after on one server and binary; 50 runs with randomized settings and 50 without all pass; and reverting the pin reddens the test 5 out of 5. (cherry picked from commit 0266e3d)
The test asserted that the scalar subquery in a materialized view definition is not executed at CREATE time by racing a 2 second max_execution_time against a 3 second sleepEachRow. That makes the assertion a function of machine load rather than of the property under test, and it failed on master in Stateless tests (amd_tsan, parallel): Code: 159. DB::Exception: Timeout exceeded: maximum: 2000 ms. (TIMEOUT_EXCEEDED) The message has no "elapsed ... ms" clause. QueryStatus::throwProperExceptionIfNeeded emits that prefix only for a non-zero elapsed, so the throw came from throwIfKilled via executeQuery.cpp, where the deadline fires while the query is still pending (slow to analyze or plan). Every query including DDL is registered with the CancellationChecker watchdog unconditionally, and the in-sleep check in sleep.cpp always reports a non-zero elapsed. The scalar was therefore never executed: the engine behaved correctly and only the clock failed. max_execution_time is not randomized by the test runner, so there is no setting to pin, and widening the bound was already tried on the structural twin (upstream ClickHouse#91744) without holding. Remove the timing oracle and assert the property directly with throwIf(1), following the fix for the twin 03356_analyzer_unused_scalar_subquery in ClickHouse#111983. throwIf::isSuitableForConstantFolding() returns false, the same guard sleepEachRow relies on, so the rewrite keeps the mechanism the old oracle depended on while detecting execution at all rather than only execution slower than 2 seconds. Cover CREATE TABLE ... EMPTY AS SELECT as well, and add an executed-position arm that must fail so the check has teeth. Validated on both analyzer paths at 50/50, 8 concurrent copies, and with two mutations that each redden the new oracle. The reference file stays empty. (cherry picked from commit 316855e)
github-actions
Bot
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated backport of upstream flaky-fix commits.
Applied
967a951b012aFix flaky 01184_long_insert_values_huge_strings by pinningmax_threadsa2a3a410c255Fix flaky 02703_max_local_read_bandwidth by restoring throttler rate …0e2550f7fa8cFix flaky test_replication_credentials replication race71996125cb8eFix flaky 02040_clickhouse_benchmark_query_id_pass_through2938a9d9adf4Fix flaky test_tcp_handler_connection_limits5af2fa94c216Fix flaky 02559_add_parts: assert active_parts after DETACH PARTITIONe1857b623a04Fix flaky test_named_collections_encrypted2 by syncing the kazoo clie…767e8b42afcaFix flaky 04357_table_readonly_background_movesb87486d3f05aFix flaky test_dirty_pages_force_purge: lower purge thresholdebec05871cccFix flaky 03717_async_deduplication_with_mv losing a row0b51ec04cb06Fix flakytest_keeper_dynamic_log_level: poll for the log level change3e1ba66d8175Fix flaky test_concurrent_watches losing one watcha89904564057Fix flaky 03100_lwu_07_merge_patches (committed 2026-08-10T20:42:43Z)0266e3d171bbFix flaky Q4 of 03753_join_runtime_filter_dynamically_disable (committed 2026-08-11T02:51:34Z)316855e01c00Fix flaky 02999_scalar_subqueries_bug_2 (committed 2026-08-13T07:40:19Z)Skipped (cherry-pick conflict — manual backport needed)
9dc9d7858c08Fix flakytest_url_reconnect(committed 2026-08-12T17:47:10Z)