Antalya-26.6 - Backport flaky-fix commits from upstream (2026-08-10) - #2196
Open
github-actions[bot] wants to merge 17 commits into
Open
Antalya-26.6 - Backport flaky-fix commits from upstream (2026-08-10)#2196github-actions[bot] wants to merge 17 commits into
github-actions[bot] wants to merge 17 commits into
Conversation
…igest 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) <noreply@anthropic.com> (cherry picked from commit 0739c13)
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 ffd1a9d)
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)
…meouts 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 82d324b 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 <noreply@anthropic.com> (cherry picked from commit 52721e7)
…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 `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 ClickHouse#102402 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit cd9d6ab)
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)
…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/<uuid[:3]>/<uuid>, 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 52d3e45)
github-actions
Bot
added
antalya
cicd
Author
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
3 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
0739c135df0fFix flaky test_replicated_database_and_unavailable_s3: wait for the digest rewrite (committed 2026-08-03T22:01:42Z)ffd1a9d623d9Fix flaky 02932_refreshable_materialized_views_2 step <27: cancelled> (committed 2026-08-04T03:56:19Z)967a951b012aFix flaky 01184_long_insert_values_huge_strings by pinningmax_threads(committed 2026-08-04T05:14:45Z)a2a3a410c255Fix flaky 02703_max_local_read_bandwidth by restoring throttler rate headroom (committed 2026-08-04T13:50:27Z)0e2550f7fa8cFix flaky test_replication_credentials replication race (committed 2026-08-05T11:24:09Z)71996125cb8eFix flaky 02040_clickhouse_benchmark_query_id_pass_through (committed 2026-08-05T12:13:17Z)2938a9d9adf4Fix flaky test_tcp_handler_connection_limits (committed 2026-08-05T16:51:55Z)5af2fa94c216Fix flaky 02559_add_parts: assert active_parts after DETACH PARTITION (committed 2026-08-05T20:47:20Z)52721e737b3bFix flaky 03469_json_read_subcolumns_combined_2_compact_merge_tree timeouts (committed 2026-08-05T21:11:11Z)e1857b623a04Fix flaky test_named_collections_encrypted2 by syncing the kazoo client before reads (committed 2026-08-06T13:05:48Z)cd9d6abde6d5Fix flaky test_trace_log_memory_context: force a new global memory peak (committed 2026-08-06T14:49:14Z)767e8b42afcaFix flaky 04357_table_readonly_background_moves (committed 2026-08-07T01:27:55Z)b87486d3f05aFix flaky test_dirty_pages_force_purge: lower purge threshold (committed 2026-08-07T12:37:52Z)ebec05871cccFix flaky 03717_async_deduplication_with_mv losing a row (committed 2026-08-08T00:02:24Z)0b51ec04cb06Fix flakytest_keeper_dynamic_log_level: poll for the log level change (committed 2026-08-08T10:17:15Z)3e1ba66d8175Fix flaky test_concurrent_watches losing one watch (committed 2026-08-09T13:03:00Z)52d3e4520df5Fix flaky test_attach_table_from_s3_plain_readonly: scope the upload to the table (committed 2026-08-09T19:08:12Z)Skipped (cherry-pick conflict — manual backport needed)
c0047bc1478aFix flaky 04655_pr_plan_based_join_runtime_filter_on_worker (committed 2026-08-06T17:26:16Z)3c0669cb1e00Fix flaky NATS test_direct_select_leftover_does_not_pollute_view (committed 2026-08-06T19:11:30Z)330c15079a4dFix flaky test04267_ttl_drop_merge_no_data_read(committed 2026-08-09T17:20:59Z)