`03322_view_over_parameterized_view` leaves the parameterized view `test_1.v`
and an interrupted `02703_row_policy_for_database` leaks `ON test_1.* USING 1`.
A parameterized view has no columns until its parameters are given, so
`SELECT 1 FROM v` fails the access check for any user (Code 497), and a filter
naming a column fails with an unknown identifier, which dropped a valid policy.
Red on #126880 and #127094.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The stress phase can leave a database-level policy (`ON test_1.* USING 1`,
from an interrupted 02703) over a database where another test left a data
lake table without files (04894 deletes them in an `EXIT` trap). The
row-policy check stopped on that table with `Code: 107 FILE_DOESNT_EXIST`,
which is not a filter error, and wrote a false `Cannot check whether the row
policies can be applied` FAIL row (seen on #126223).
The batched check of all covered tables stays the fast path. When it fails
with an analysis error or 107, the tables are checked one by one, with and
without the filter: a table that fails the same way without the filter is
logged to `unreadable_row_policy_tables.txt` and skipped, a filter that fails
on a readable table is classified as before, anything else is a FAIL row.
A `DeltaLakeLocal` table without files fails with `Code: 742
DELTA_KERNEL_ERROR` instead of 107 and is handled the same way.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`EXPLAIN QUERY TREE` executes the scalar subqueries of the filter, and their
reads apply the row policies of the tables they read as the `default` user.
So a valid policy was dropped when a broken policy covered its subquery's
table, and a valid user-scoped policy failed the check with ACCESS_DENIED
when no policy on that table covered `default`.
`DESCRIBE TABLE view(...)` analyzes the query with `only_analyze`: scalar
subqueries get type-only placeholders and the planner applies no row
policies, so each filter is checked on its own. Subqueries in table function
arguments still run; no row policy in the suite has one. Measured on 26.3.33.24 and
26.9.19.9: every filter it rejects also fails a normal read, and it also
rejects plan-level type errors (`s = 1` on a String column). A scalar
subquery that returns several rows (125) is no longer detected, so the
ordering of filters with subqueries and code 125 are removed.
The stress phase fuzzes `CREATE ROW POLICY` filters (`ast_fuzzer_any_query`),
so add the other analyzer errors a fuzzed filter gives, each measured on
26.9 to fail both the check and a normal read: 20, 38, 41, 44, 81, 153, 190,
407, 675. Reader-dependent errors (ACCESS_DENIED, an unknown custom setting)
still keep the policy and write the FAIL row.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A scalar subquery of a filter is evaluated by `EXPLAIN QUERY TREE` and reads
its table with that table's row policies, so a working policy such as
`k > (SELECT max(k) FROM allowed)` failed its check, and was dropped, when
`allowed` carried a broken policy (`x = 1` over a table without `x`). The
filters without a subquery are now checked first, and a broken policy is
dropped as soon as its check fails, so the later checks no longer see it.
The check also recognizes more errors that the analysis of a broken filter
gives on 26.9 (each also fails every read of the table): 53 (`k IN (SELECT
tuple(1, 2))`), 42 (`equals(k)`), 184 (`count() > 0`), 36 (`dictGet` over a
missing dictionary, an unknown time zone), 69 (`toDecimal32(k, 100)`), 6
(`toUInt8('abc')`) and 125 (a scalar subquery returning two rows). These
policies were kept with a FAIL row before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`Received exception from server` also covers resource limits, server-side
timeouts and cancellation, which say nothing about the filter. With a memory
exception forced in the validation client (26.9.19.9), every checked policy
was dropped, the valid ones included, and no FAIL row was written.
Drop a policy only when the server rejects its probe with one of the codes an
unresolvable filter produces on `EXPLAIN QUERY TREE`, measured on 26.9.19.9:
ILLEGAL_TYPE_OF_ARGUMENT (43), UNKNOWN_FUNCTION (46), UNKNOWN_IDENTIFIER (47),
ILLEGAL_TYPE_OF_COLUMN_FOR_FILTER (59), UNKNOWN_TABLE (60) and NO_COMMON_TYPE
(386, e.g. `has([1, 2], x)` over a `String` column). A failed `USE` gives
UNKNOWN_DATABASE or an operational code, so it never matches. Any other error
keeps the policy and is reported in the existing FAIL row, with the client
output. The `Code: N. DB::Exception: Received from` line is the same in the
26.3, 26.6, 26.8, 26.9 and master clients.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The filter of a row policy may name a table without a database, and such a name
resolves in the current database of the reader. The probe ran in `default`, so a
working policy like `k IN (SELECT k FROM allowed)` of a test in `test_1` failed
with `UNKNOWN_TABLE` and was dropped. Select the policy's database before
`EXPLAIN QUERY TREE`.
Any failure of the probe also fell through to `DROP`, including a failure of the
query that only generates it, a timeout and a lost connection. Drop a policy only
when the server rejects its probe (`Received exception from server`); otherwise
keep it and write a FAIL row with the client errors.
Both reported by the AI review of #126169.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop only the row policies left by the stress phase whose filter does not
resolve against a table they cover, and keep the rest across the upgrade, so
the upgraded server still has to load and apply them (review on #126169).
A filter is checked with `EXPLAIN QUERY TREE SELECT 1 FROM <table> WHERE
<filter>` for every table it covers (every table of the database for
`ON db.*`). Row policies are applied by the planner, not by the analyzer, so
this checks the one filter without the other policies of the table. The
deliberate fixture policy is removed: it only compensated for dropping all
policies.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The stress phase runs many tests in one shared database per worker, and a
test that is stopped before its last statement leaves its row policies
behind. A database-level one (`02703_row_policy_for_database.sh`:
`ON * USING x=1 TO ALL` on `test_1`) filters every table of that database,
so after the upgrade the eager dictionary of `04365_dictionary_lazy_load`
over `test_1.src`, which has no column `x`, fails to load in the background
with `UNKNOWN_IDENTIFIER` and the log scan fails the job (seen on #126078).
The old server fails the same way, so it is leftover test state, not an
upgrade incompatibility. 15 row policies were alive when the old server
stopped in that run, two of them created by the AST fuzzer.
Drop every row policy not defined in `users.xml` before the upgrade and
fail the job when one survives, as is done for leftover mutations
(#120978). Neutralizing them instead does not work: the test config sets
`throw_on_unmatched_row_policies`, so a table whose policies apply to
nobody throws for every reader. The statements are generated server-side
from `system.row_policies.name`, so no name passes through the shell.
The drop takes away the policies that happened to cross the upgrade, so
one is created deliberately (`k < 3` on a 10-row table) and the new
server has to apply it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each server restart resets jemalloc's dump sequence, so sorting by it could
render a stale snapshot.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The fuzzer continues after a server-side `MEMORY_LIMIT_EXCEEDED`, so such a
run can exit 0, where artifacts are not attached. Attach the archive when the
server log has the total-memory flush line. The stress lib now takes the logs
to scan as arguments, so the upgrade check consults only the upgraded server's
log and does not render on the previous release's memory events.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`install.sh` already enables the peak/limit flushes there, but without
`prof_prefix` every flush was skipped ("empty prefix") and nothing was kept.
Set `prof_prefix` for non-sanitizer builds (sanitizer builds have no jemalloc),
archive the profiles as `jemalloc.tar.zst`, and symbolize the latest one when
the total limit was hit or the server was killed. The upgrade check moves the
previous release's profiles aside, since only its binary can symbolize them.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`throwAtAssertionFailed` prints `at end of stream` whenever the input ends
before the expected string, so the matched line is logged both for an
empty `columns` value and for one cut off inside its
`columns format version: 1` header line. The comment claimed every other
`columns` value still fails the job, which is not true for the truncated
headers. The log line carries no other field to separate the two, so state
the real scope instead.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`02980_dist_insert_readonly_replica` breaks its `shard_1.data` replica on
purpose: it empties the replica's `columns` znode and re-attaches the table,
so the attach thread keeps it read-only. The file drops `shard_0` and
`shard_1` only in its last two statements. When stress fault injection
(`memory_tracker_fault_probability`) hits a query before them, the file
stops, the replica survives into the upgrade restart, and the new server
logs the non-retryable attach-thread branch:
<Error> shard_1.data (ReplicatedMergeTreeAttachThread): Initialization
failed, table will remain readonly. Error: Code: 27. DB::Exception: Cannot
parse input: expected 'columns format version: 1\n' at end of stream.
The old server logged the identical line before the upgrade, so it is not a
compatibility signal. Only the retryable branch (`Initialization failed.
Error`) is allow-listed, which is right in general: the read-only branch
carries real upgrade findings (METADATA_MISMATCH, TOO_MANY_UNEXPECTED_PARTS,
BAD_DATA_PART_NAME on other tables).
The new matcher requires the anchored `shard_1.data` attach-thread logger,
the read-only branch, Code 27 and `at end of stream` (an empty `columns`
value) together. Any other table, code or `columns` content still fails the
job. Seen on PR #123566 (Upgrade check (amd_release), 27b84d71).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A stress query can outlive the stress phase and keep a shared lock on a
replicated table. In the failing job a `merge()` SELECT of
01169_old_alter_partition_isolation_stress kept retrying the S3 listing of a
table left on an unreachable endpoint by 04627. The drain's
`SYSTEM RESTART REPLICA` shut the replica down, timed out after
lock_acquire_timeout waiting for the exclusive lock (DEADLOCK_AVOIDED, owner =
that query), and left the replica read-only, so `SYSTEM SYNC REPLICA ... PULL`
failed with TABLE_IS_READ_ONLY and the step reported a failure with nothing
left to drain.
Cancel every running query once, before the mutation section. Nothing uses
their results: their clients are gone, and the stop of the old server does not
cancel them either (the test config sets shutdown_wait_unfinished_queries, so
the stop waits shutdown_wait_unfinished and exits with them still running). The
drain itself is unchanged and still fails closed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stress worker 1 (`--database=test_1`) runs with
`memory_tracker_fault_probability=0.05`. Work that the stress phase leaves
behind keeps the settings of the query that created it and is executed again
by the upgraded server after the restart:
- a distributed DDL entry stores the initiator's changed settings
(`DDLLogEntry`) and `DDLTaskBase::makeQueryContext` applies them, so a
queued `BACKUP ... ON CLUSTER` of `test_1` replays with fault injection and
`BackupsWorker` logs `Failed to make internal backup ... fault injected` at
`<Error>`;
- a pending `Distributed` batch is sent with its stored settings, and the
receiving async-insert flush logs `AsynchronousInsertQueue: Failed insertion
... fault injected` at `<Error>` with an empty query id, so the existing
`} <Error> executeQuery: Code:` entry does not cover it.
Seen on three unrelated PRs in 90 days (#123464, #117943, #120725); in each job
these lines were the only output of the scan. Nothing enables fault injection
on the upgraded server itself, so the message can only come from carried-over
stress work. Ignore it as a fixed string, the same string the stress smoke
check already tolerates (`ci/jobs/scripts/stress/stress.py`). It is
MemoryTracker's message for both injection sites and does not match a real
`memory limit exceeded` error.
The large DDL backlog in the #123464 job comes from the previous release
(26.9) wedging its DDLWorker on `KILL PART_MOVE_TO_SHARD ... ON CLUSTER`,
which #122132 fixed on master only.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pre-upgrade kill step (#120978) kills every mutation that some replica has not finished. When a test
stopped merges or fetches on one replica only (04907_mutation_parts_to_do_deprecated_syntax,
03100_lwu_40_cleanup_race), the other replica has already applied the mutation, and the stopped one still
holds MUTATE_PART entries for it. `KILL MUTATION` removes the mutation node but not those entries. After
the upgrade the restart lifts the stops, the entry finds no mutation ("There are no mutations for partition
ID ..."), clones its source part unchanged, and the commit check against the other replica's mutated part
fails with CHECKSUM_DOESNT_MATCH ("Data after mutation is not byte-identical to data on another replicas"),
which the `<Error>` scan reports. The engine documents this race in both messages and repairs it by
fetching; the scan sees it only because the harness made the upgraded server run the entry.
Before the kill, list every replica that shares (zookeeper_name, zookeeper_path) with a table holding an
unfinished mutation or a queued MUTATE_PART. After the kill and its count check, `SYSTEM RESTART REPLICA`
each of them and check that it came back writable: the restart waits for a running merge selection (so a
selection made before the kill has logged or dropped its entry), reloads the mutations from Keeper (where the
killed ones are gone), and drops the tests' `SYSTEM STOP`s, i.e. the state the upgrade restart would produce.
Then `SYSTEM SYNC REPLICA ... PULL` each, and wait up to 60 s until no replica, read-only ones included, has
a MUTATE_PART queued. The old server thus runs the leftovers: an entry nobody applied clones identically, a
lagging one mismatches once and fetches the mutated part. Operational failures write a FAIL row with the
errors; an entry still queued at the deadline is dumped to undrained_mutation_entries.txt for attribution.
The lagging replica's mismatch is now logged by the old server (clickhouse-server.stress.log, not scanned)
instead of the upgraded one.
Validated on a local rig (26.9.8.3 -> master): the 04907 state reproduces the CI hash pair
351fbe96.. vs b54e201a.. after the upgrade without this change (3/3) and none with it (3/3); the same for the
03100_lwu_40 state ("has different columns hash") and for an entry the lagging replica never pulled.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The recovery pass compared `count() FROM system.replicas` before and after its
restarts, which answers for every replicated table on the server rather than for
the ones it restarted. An unrelated table attaching, or being dropped, inside
that window made the step write `Cannot restart a read-only replica holding a
mutation left unfinished by the stress phase` on a run where every restart it
issued had succeeded and every mutation it targeted was killed. The stress phase
leaves other replicated tables behind, so the window is not quiet.
Measured on v26.8.10.6-lts, the release the job downloads, with two bricked
ReplicatedMergeTree tables and one independent replica changing inside the
window: `recovered_replica` returned 1 for both targets, no unfinished mutation
and no read-only replica was left, and the FAIL row was written anyway - in both
directions, 2 rows to 3 and 3 rows to 2.
The per-table check inside the loop is what the pass is accountable for, and it
is strictly stronger for those tables: a table left detached and a replica whose
initialization failed both return 0 from `... WHERE database = ... AND table =
... AND NOT is_readonly`. The global count was blind exactly where that check is
needed - with Keeper stopped the restart exits 0, the replica stays read-only,
`system.mutations` reads 0 and the replica count does not move - so scoping the
comparison to the restarted pairs would only restate the per-table check.
Removing it also retires the `restarted_replicas` gate, which existed to keep
the comparison from firing when the pass restarted nothing.
Re-measured on the same rig after the change: the `restart_replica_fail_after_detach`
failpoint still reds while the mutation count reads 0, Keeper stopped still reds
with both tables named in the errors file, and a clean server still issues no
restart at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The kill step compares `system.replicas` before and after the restart pass to
catch a restart that detaches a table and leaves it detached: the count that
decides the red reports only what an attached replica holds, so that state would
report success for a mutation the upgraded server then resumes.
The comparison also ran when the selection of read-only replicas came back empty
and the pass restarted nothing. Such a run has none of those failures to answer
for, yet an unreadable or changed count still wrote `Cannot restart a read-only
replica ...`. Measured on 26.8.10.6, with the post-loop count timing out: the
step reds a run that killed every mutation and reported `The stress phase left
no unfinished mutation to the upgrade` in the same file, pointing at an errors
artifact that was never created.
Gate the comparison on the number of replicas the pass actually restarted. The
per-table check inside the loop, and the same clause for a run that did restart
something, are unchanged: with `restart_replica_fail_after_detach` enabled the
step still FAILs while the mutation count reads zero.
The pre-upgrade kill loop added in #120978 reds with "N mutations could not be killed before
the upgrade" on pull requests unrelated to mutations: 4 failures against 331 OK runs of the
counterpart assertion since it landed on 2026-09-20, on 4 unrelated pull requests, in 3 of
them the job's only red.
In the three occurrences whose job logs I read, the kill threw Code: 242 TABLE_IS_READ_ONLY
and the deliberate `||:` discarded it. The chain: 04070_truncate_all_tables_skip_views.sql
runs TRUNCATE ALL TABLES FROM currentDatabase(), which in stress mode is the runner's shared
test_N database, so it covers every table earlier tests left behind (--upgrade-check implies
--fake-drop, so their DROPs were no-ops). On the previous release the unfiltered form still
calls flushAndPrepareForShutdown() on every table in the database;
StorageReplicatedMergeTree::flushAndPrepareForShutdown does
restarting_thread.shutdown(part_of_full_shutdown = true), which sets need_stop, deactivates
the task and calls setReadonly(), so the replica is read-only permanently with no thread left
to re-activate it. The per-table truncate then throws, because truncate() begins with
assertNotReadonly(), so the tables are left read-only and un-truncated with their mutations
intact. Thirteen minutes later the kill loop reaches killMutation(), which also begins with
assertNotReadonly().
#119425 fixed the product half on master, the `if (!truncate || !query.has_tables)` guard in
InterpreterDropQuery, but it is not backported and the stress phase runs the previous
release's binary and queries, so only this script can act. Reproduced on v26.8.10.6-lts,
which find_previous_release picks today.
Three changes, all in the kill step:
- Each KILL MUTATION keeps its stderr in unkilled_mutation_errors.txt, and the FAIL line
names it. Today the survivor is reported with an empty latest_fail_reason, so the artifact
says nothing about why the kill did not happen, and attributing one occurrence cost a whole
investigation.
- If a mutation survives, SYSTEM RESTART REPLICA the read-only replicas that still hold one,
then run the same kill pass again. Re-attaching the table builds it from its metadata and
starts a replica that can remove the mutation's ZooKeeper node. Waiting instead cannot work
for this cause: the restarting thread is deactivated, so no session-expiry check
re-activates the replica. The no-argument SYSTEM RESTART REPLICAS is unusable here because
restartReplicas() passes throw_on_error = false, so a table it fails to restart is left
detached and its mutation silently disappears from system.mutations.
- A restart that does not report success, that changes the number of rows in system.replicas,
or that leaves its own replica read-only, writes its own FAIL row whatever the count says.
Without this the change would be fail-open, which is worse than the red it removes, and it
is fail-open in two distinct ways. doRestartReplica() detaches before rebuilding, so a
failure after that point leaves the table detached; and because it re-creates the table with
LoadingStrictnessLevel::ATTACH the storage always goes through
ReplicatedMergeTreeAttachThread, whose startup() only waits for the first initialization
attempt and never rethrows, so a restart whose first attempt fails returns success and
leaves the replica read-only with a retry scheduled initialization_retry_period later.
system.mutations reports only what an attached, initialized replica holds, because the
mutation queue is filled by pullLogsToQueue(..., LOAD) on the same success path that later
calls setNotReadonly(). Either state would otherwise drive the count to zero and report
success for a mutation still in ZooKeeper. That load order is also why the per-replica check
is race-free: is_readonly = 0 implies the mutations are already loaded.
Validated against a real v26.8.10.6-lts server, since no harness executes upgrade_runner.sh.
On one fixture of two bricked replicated tables the old block reports FAIL and the new one
OK; on a clean server it does nothing, leaves no artifact, and never enters the recovery
block. Both fail-open shapes were injected and both produce the new FAIL row while
system.mutations has already reached zero: the restart_replica_fail_after_detach failpoint for
the detached table, and a stopped Keeper for the replica that stays read-only, where the
unguarded block reported "The stress phase left no unfinished mutation to the upgrade" for a
mutation whose znode was still there and which executed as soon as the replica came back.
shellcheck reports the same findings as before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The mutation-across-upgrade probe asks system.mutations whether the mutation
submitted to the old server was finished by the new one. That query cannot be
narrowed to the table it is about: StorageSystemMutations::fillData resolves
every table it enumerates (StorageSystemMutations.cpp:91) before the *MergeTree
narrowing at :95 and the predicate at :128, and getTablesIterator waits on each
table's startup job and rethrows its exception (DatabaseOrdinary.cpp:722-741).
One unrelated table that cannot load therefore makes the probe throw whatever
the WHERE says.
`var=$(clickhouse-client ...) ||:` then erased the throw: stderr went to the job
log, the exit status was discarded, and the variable was left as the empty
string. Only `= 1` was tested, so empty fell into the else branch and the job
reported that this mutation had not finished. The diagnostic dump reads
system.mutations too, so it threw for the same reason and the artifact the
message pointed at was 0 bytes.
Over 14 days of CIDB this produced 9 such rows on 9 unrelated pull requests
against 225 passes, with an empty failure context every time. On #121247 the
mutation was applied at 07:59:40.104 while all 60 probe executions ran between
08:00:35 and 08:01:39 and every one of them threw, so the report named the
wrong subsystem and carried no evidence for it. The underlying load failure in
those runs was the Keeper feature-flag error already fixed by #120932; this
change is the attribution half.
Give the probe three outcomes instead of two, which is the idiom this file
already uses for its two other system.mutations probes at :203 and :220: keep
the answer-1 and answer-0 rows exactly as they were, and report a thrown probe
as its own row carrying the real error text. That row stays a FAIL, because the
check could not be performed and passing silently would lose the same signal in
the other direction. The assertion is neither relaxed nor narrowed and a
genuinely unfinished mutation still reds byte-identically.
The `||` guard is load-bearing rather than decoration: `set -ex` is in effect,
and `A || B` with an assignment for B always succeeds. The existing
if/then/break/fi is kept for the same reason, since a false `&&` list as the
last command of the loop body would exit the job. The probe's stderr is
captured so the new row can carry its cause, and removed when empty per the
convention already at :239 and :439. The dump gains 2>&1 so it can no longer be
the empty artifact a message points at.
Requested at https://github.com/ClickHouse/ClickHouse/pull/120908#issuecomment-5756172516
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Upgrade check (amd_release) has gone red on 5 unrelated PRs in 90 days (0
master rows) with
Code: 40 ... Checksums of parts don't match: hash of uncompressed files
doesn't match (X vs Y), uncompressed hash of compressed files doesn't
match (A vs B). (CHECKSUM_DOESNT_MATCH)
followed by a refetch from the peer. The server self-heals, but the <Error>
line is logged and no allow-list entry covers it, so the job fails.
tests/config/install.sh draws the server-level default_compression_codec
from ("LZ4" "ZSTD(1)" "ZSTD(3)") with $RANDOM on every configure call, and
tests/docker_scripts/upgrade_runner.sh calls configure twice over one data
directory: once for the previous-release server and once for the new one.
The two draws are independent, so with probability 2/3 the two servers of a
single job write parts under different codecs. Because it is a server
default it is not persisted into a table's CREATE, so every write resolves
it from the running server. A part the previous-release server committed to
ZooKeeper is then no longer byte-identical when the new server re-merges or
re-mutates it, and checkPartChecksumsAndCommit rejects it. This is reason 2
of the error message's own list ("Using another compression method"), so the
product is behaving as designed; the harness is what is unfaithful, since a
real in-place upgrade does not re-roll the server's compression codec.
Both compared aggregates are codec-sensitive, which is why the message
carries both clauses. serializeStatisticsPacked compresses each per-column
statistics blob with the part's codec but registers the container with only
file_size/file_hash, leaving is_compressed false, so codec-dependent raw
bytes are compared as uncompressed content. Separately, a mark encodes
offset_in_compressed_file, so the marks' uncompressed content embeds the
compressed layout of the data. Measured by turning statistics off: the
uncompressed-files aggregate then stops diverging while the other still
does, which attributes one bucket to statistics.packed and the other to the
marks. For a Wide part both move once a column's data spans more than one
compressed block; below that every mark offset is 0.
install.sh gains a --default-compression-codec option, validated against
the same array that defines the legal values and failing closed on an
invalid, empty or missing value; upgrade_runner.sh reads back the codec its
previous-release configure drew and passes it to the second configure only
(not through configure_opts, which both calls share). If the read-back is
empty the runner writes a FAIL row and exits. The runner then re-reads the
generated config after the second configure and fails the job if the
upgraded server's codec differs, so a pin that stops being applied cannot
quietly restore the randomized condition.
An option rather than an environment variable because b41d0c5bbc moved
this runner off env vars onto install.sh options, and the option makes the
requirement legible at the site a future part-format randomization would be
added. Per-job randomization is preserved, all three codecs are still
covered across runs, and reads are unaffected because every part records
its own codec in default_compression_codec.txt.
Deliberately not an allow-list entry for this message: that would also hide
a genuine non-deterministic-merge regression and the separate
statistics-format divergence, which is the signal worth keeping. After this
change that sub-case presents with only the "hash of uncompressed files"
clause, so it becomes cleanly attributable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Upgrade check (amd_release)` intermittently fails its `Error message in
clickhouse-server.log` sub-test on one benign line:
<Error> StorageFileLog (test_1.filelog_bad_path_attach): The absolute data
path should be inside `user_files_path`(/var/lib/clickhouse/user_files/)
04202_filelog_attach_path_outside_user_files has an explicit ATTACH query for a
FileLog table whose path is outside `user_files_path`. ATTACH is
LoadingStrictnessLevel::ATTACH, which is >= SECONDARY_CREATE, so the constructor
takes the relaxed branch in src/Storages/FileLog/StorageFileLog.cpp: it logs at
<Error> and returns instead of throwing BAD_ARGUMENTS. That branch is what the
test covers, since refusing to load at reattach time would break server startup.
The table outlives the test because stress threads run with a fixed
--database=test_N (ci/jobs/scripts/stress/stress.py) and clickhouse-test skips
its per-test teardown whenever --database is set, so the shared database is never
dropped. The upgrade restart re-attaches the table, the relaxed branch fires
again, and the line lands in the scanned log, where the post-restart scrub had no
entry for it.
The matcher keys on the StorageFileLog logger plus the message and carries no
table or database name, so the same message is allowlisted for any FileLog table.
The `): ` boundary keeps the discriminating cases visible: a different message
from the same logger, this message from another storage, and a Code: 36 throw of
the same text (the strict-mode path, a real failure) all still fail the job.
Validated against the failing run's own 12.6 MB clickhouse-server.upgrade.log
(PR #117355, sha cff55108d1): with the entry the scan artifact is empty, and
with it removed the artifact is the 189-byte line CI reported as the failure.
Related: #113781
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every `KILL MUTATION` is under `||:`, on purpose, so that one read-only table
does not stop the rest of them, and the count that followed the loop only
printed its result. A kill that threw therefore carried its mutation into the
upgrade, where the `<Error>` scan - no longer tolerating mutation errors -
reported it as an unexplained error message with nothing pointing back at this
step.
Check the invariant the removed allow-list entries depend on where it is
established: a non-zero count of unfinished mutations after the loop, or a
count that cannot be taken at all, is a `FAIL` before the server is upgraded,
and the mutations still standing are written to `unkilled_mutations.txt`. The
per-mutation `||:` stays, so the loop still tries all of them.
The check runs before the mutation submitted for the upgrade to resume, which
is meant to be unfinished at that point; the order is noted in the comment.
Verified against a local server holding the three broken mutations of
`04338_on_fly_mutation_read_overwritten_lc_source`: the loop kills all three
and the check reports `OK`, an unkilled mutation gives
`1 mutations could not be killed before the upgrade`, and an unreachable
server gives `Cannot count the mutations left unfinished by the stress phase`.
Killing every unfinished mutation also takes away the one thing they were
incidentally covering: a mutation submitted to the old server and finished by
the new one, which is a real contract - a submitted mutation is persisted and
continues to execute after a restart.
That coverage was never dependable, because which mutations reach the upgrade,
and whether they are valid at all, is decided by which test file the stress
runner happened to interrupt. Replace it with a fixture: after the kill loop,
create a table, submit `UPDATE v = v + 1` to it and hold the mutation with
`SYSTEM STOP MERGES`. `SYSTEM STOP MERGES` is in-memory state, so the upgraded
server starts with merges enabled and has to resume the mutation on its own.
After the upgrade the job requires it to be done with no `latest_fail_reason`,
and `sum(v)` to have changed, so the mutation has to have been applied to the
data and not only marked done.
The report artifact of the `Upgrade check` of this pull request says why both
halves are needed. It lists 15 unfinished mutations, all of them from
`04338_on_fly_mutation_read_overwritten_lc_source`, and **all 15 with an empty
`latest_fail_reason`**, because that test holds its mutations with
`SYSTEM STOP MERGES` and they were therefore never attempted. Reproduced
locally from the artifact - same five tables, same 15 rows, same empty failure
state - and restarted:
- 12 of them resumed and finished. This is the coverage the fixture now makes
deliberate.
- 3 of them (`t_overwrite_before_consume`, whose `MODIFY COLUMN v UInt64`
meets the on-disk `'x'`) fail forever with
`Code: 6 ... Cannot parse string 'x' as UInt64`, 45 `<Error>` lines in the
first 45 seconds, in the three shapes
(`MutatePlainMergeTreeTask`, `executeStep()`, `MergeTreeBackgroundExecutor`)
that the removed `Cannot parse string 'x' as UInt64` entry was masking.
So the kill cannot be narrowed to the broken mutations by a predicate over
failure state: at kill time these three are indistinguishable from the twelve
healthy ones - every one of the 15 has no error code and no fail reason. It is
the restart that separates them, which is exactly too late.
Verified on a local server: the check expression is `0` while the mutation is
held, and `1` on the first poll after a restart, with `sum(v) = 500500`.
Report: https://s3.amazonaws.com/clickhouse-test-reports/PRs/120978/dcefe415cd055caaa25ad62e37a4935de124315e/pr/upgrade_check_amd_release/unfinished_mutations.txt
The keys of the mutations to kill were listed by a command substitution in
the list of a `for` loop. That is not a command `set -ex` watches, so a
listing that failed or timed out was indistinguishable from an empty result:
the loop ran zero times, the job carried on, every unfinished mutation
survived into the upgraded server, and the post-upgrade `<Error>` scan - which
this pull request stops tolerating mutation errors in - failed much later,
with nothing pointing back at the skipped step. Demonstrated on a standalone
script: `for k in $(bash -c 'exit 124')` under `set -e` neither aborts nor
reports, while the assignment form below returns the exit code.
List into a variable, so the exit code is the condition of an `if`, and write
a `FAIL` row naming the step when it is non-zero. The kill statements
themselves keep their `||:` - `KILL MUTATION` legitimately throws for a
read-only table, and one such table must not stop the rest.
The mutations that survive a kill are still reported: `unfinished_mutations.txt`
holds the ones found and `Unfinished mutations left:` the count after the loop.
Yakov Olkhovskiy