A prior ClickHouse to Datastore rename rewrote `github.com/ClickHouse/ClickHouse`
to `github.com/Datastore/Datastore`, a repository that does not exist. That
turned 162 references across 131 files into dead links, and turned captured
stack traces and historical changelogs into false statements.
A citation of upstream is a fact, not branding. Product surfaces keep the Hanzo
Datastore name; references to the upstream project point at the upstream project.
Also untracks `.claude/`, agent scratch that was never meant to be committed.
Co-authored-by: Hanzo Dev <[email protected]>
13,838 upstream commits, forward from the 26.6.1.1 fork point. 129 conflicts:
37 resolved mechanically (take upstream's file, re-apply the brand sweep) and
92 by hand where our fork and upstream had both changed the same code.
The rule throughout: upstream owns the engine, we own the naming and a small set
of deliberate deviations. Never resolve by discarding an upstream change — a
dropped rename or a missed new parameter becomes a compile error a thousand
files away.
Deviations verified intact after the merge:
ENABLE_XRAY still defaults OFF (CMakeLists.txt:105)
X-Datastore-User read by the server (authenticateUserByHTTP.cpp)
X-ClickHouse-* in src/ 0
hanzo/research present
system.*_log retention bound present
ArrowFlight rip held against upstream re-adding it
Two things worth naming, because neither shows up as a conflict:
src/CMakeLists.txt gained an UNGUARDED
`add_object_library(datastore_storages_arrowflight Storages/ArrowFlight)` from
upstream, for a directory our gRPC rip deleted. It sat outside every conflict
hunk, so a resolver reading only the markers would sail straight past it into
a CMake configure error. Removed; the Server/ArrowFlight sibling at line 474
stays because it is inside `if (TARGET ch_contrib::arrow_flight)`.
The brand sweep had rewritten github.com/ClickHouse/ClickHouse issue links to
github.com/Datastore/Datastore — a dead host. Those point at upstream's own
tracker and are restored, alongside the contributors list, sql.clickhouse.com
and the ClickHouse Inc copyright: attribution is theirs, not ours to rename.
base/glibc-compatibility/memcpy/memcpy.h is deleted, not kept: upstream dropped
it for LLVM-libc's, and our only change to it was a comment rebrand.
The removeOutdatedTables and PostgreSQLConnectionPool matchers inherited
from #108560 still matched any Connection to ... failed, so a real
cleaner-task connect failure for a persisted DatabasePostgreSQL on a
non-192.0.2.1 host would be masked. Scope both to the known 04210 fixture
host 192.0.2.1:5432, mirroring the getTablesIterator and AsyncLoader
matchers, so genuine connect-time regressions still fail the upgrade check.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Follow-up to #109256. That PR added a max_threads <max>128</max> constraint
in two places (query-fuzzer-tweaks-users.xml and the stress-path
stress_test_tweaks-users.xml heredoc in stress_tests.lib). Consolidate it into
limit-recursion-settings.xml, which is already cp'd into users.d by every
ast_fuzzer_runs entry point (run-fuzzer.sh, clickhouse_proc.py, stress_runner.sh),
so a single location covers the dedicated AST fuzzer and the stress test.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
PoolWithFailover::get builds every caught pqxx::broken_connection into the
same Code: 614 / "Connection to <host_port> failed" text, so the two new
matchers keying only on the code path plus "Connection to .* failed" would
suppress any connect-time PostgreSQL failure reaching getTablesIterator or
AsyncLoader, masking a real regression in loading or enumerating a persisted
DatabasePostgreSQL.
Scope both matchers to the known 04210_show_remote_databases_in_system_tables
fixture host 192.0.2.1:5432 (RFC 5737 TEST-NET, intentionally unreachable). The
exception text embeds the backQuoted host_port on the same log line, so the
benign leftover-engine noise still matches while a connect failure to a
different host, or a non-614 code, now surfaces and fails the job.
Validated against the authentic upgrade log signature: 30-day CIDB shows every
real getTablesIterator / AsyncLoader Code-614 connection-fail line uses the
192.0.2.1:5432 host (zero other-host benign variants), and a comm-diff over a
benign+regression corpus confirms 0 benign leak and that a different-host
regression the old broad matcher masked is no longer masked.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
The merged allowlist (PR #108560) covers the DatabasePostgreSQL cleaner task
(removeOutdatedTables) and the DatabaseMySQL engine, but the same leftover
DatabasePostgreSQL engine from 04210_show_remote_databases_in_system_tables
reaches the unreachable 192.0.2.1:5432 host through two more code paths that
also log the benign connection failure at <Error>:
- DatabasePostgreSQL::getTablesIterator: a system.tables scan probes the pool
and logs the swallowed error via tryLogCurrentException.
- AsyncLoader::worker: the post-upgrade startup asynchronously loads the
leftover engine and re-logs the POSTGRESQL_CONNECTION_FAILURE (Code: 614).
Both leak past the current matchers and keep the Upgrade check (amd_release)
red across many unrelated PRs (0 on master). Add two matchers mirroring the
existing removeOutdatedTables one: each requires the PostgreSQL code-path
context AND the connection-failure symptom together (the AsyncLoader matcher
additionally pins the PostgreSQL-specific Code: 614), so genuine PostgreSQL
regressions and unrelated async-load errors are not masked.
Related: #104416
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Per reviewer request: 128 still comfortably exceeds any realistic single
query thread count and stays far below the 10000-thread global pool, so
one fuzzed query cannot saturate it. Applied to both the dedicated
fuzzer profile and the mirrored stress-path constraint.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
The stress harness enables the same server-side AST fuzzer
(ast_fuzzer_runs=5) via configure_limits in tests/docker_scripts/stress_tests.lib,
which writes stress_test_tweaks-users.xml. That profile did not carry the
max_threads constraint, so a fuzzed max_threads in the tens of thousands could
still saturate the global thread pool in the real stress workload, causing the
same server-liveness hang this PR eliminates for the dedicated fuzzer job.
Mirror the max_threads <max>256</max> constraint into stress_test_tweaks-users.xml
so every ast_fuzzer_runs>0 profile is bounded. The smoke and hung checks in
stress.py already force ast_fuzzer_runs=0, so they need no cap.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
The upgrade check (`tests/docker_scripts/upgrade_runner.sh`) scans the
post-upgrade server log for `<Error>` lines and fails on anything not in its
allow-list. It intermittently fails with:
<Error> PostgreSQLConnectionPool: Connection error: connection to server at "192.0.2.1", port 5432 failed: timeout expired
<Error> void DB::DatabasePostgreSQL::removeOutdatedTables(): Code: 614. DB::Exception: Try 2. Connection to `192.0.2.1:5432` failed ...
These come from a leftover `DatabasePostgreSQL` engine created by
`04210_show_remote_databases_in_system_tables`, which points at the RFC 5737
documentation IP `192.0.2.1` (intentionally unreachable). The stress phase runs
the stateless tests under a global time limit and can interrupt that test
between its `CREATE DATABASE ... ENGINE = PostgreSQL('192.0.2.1:5432', ...)` and
the final `DROP DATABASE`, leaving the database in metadata. After the upgrade
restart, `DatabasePostgreSQL::startup` unconditionally activates the
`PostgreSQLCleanerTask`, whose `removeOutdatedTables` tries to connect and the
connection pool logs `<Error>` for each retry.
This is the same class of benign background-reconnection-after-restart noise
already allow-listed for Kafka (`rdk:FAIL`/`StorageKafka2`), Azure DNS, and
NuRaft. Add two regex filters in the secondary pipe that require the PostgreSQL
connection-pool / cleaner-task context together with the connection-failure
symptom, so real PostgreSQL regressions (auth, protocol, query errors) are not
masked.
CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=108939&sha=27734f50667a6b77ca59b768e605b4d843acde3c&name_0=PR&name_1=Upgrade%20check%20%28amd_release%29
Related: https://github.com/ClickHouse/ClickHouse/pull/108939
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Per @azat's review, collapse the two scoped `throwIf` exclusions
(`MutatePlainMergeTreeTask.*` and `MergeTreeBackgroundExecutor.*`) into a
single global `grep -av -e "Value passed to 'throwIf' function is non-zero"`
entry in the secondary pipe.
`throwIf` is a user-level SQL function that only raises when a query
explicitly calls it with a truthy argument, so the default
`FUNCTION_THROW_IF_VALUE_IS_NON_ZERO` message can only ever originate from
such a test query, never from a background or internal assertion - the
ClickHouse internals never use `throwIf` for their own checks. Matching the
message alone is therefore safe to suppress globally, exactly like the
already-global `Code: 236 ... Cancelled mutating parts` message that the same
cancelled test mutations emit. Matching the message rather than the task type
also keeps the wrapping `MergeTreeBackgroundExecutor` line of the replicated
case covered in a single entry.
Rewrote the comment block to explain the global match.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Address review feedback from `clickhouse-gh`: the previous fixed-string
exclusion `-e "Value passed to 'throwIf' function is non-zero"` dropped every
`<Error>` line containing that default message globally, which is broader than
the invariant the filter protects. The default `throwIf` message is generic, so
a real upgrade regression in any other background or persisted object that uses
`throwIf` as an assertion would silently disappear from
`upgrade_error_messages.txt`.
Move the exclusion from the primary fixed-string list into the secondary regex
pipe, pairing the message with the mutation-task logger that emits it, exactly
like the other scoped exclusions in this file:
- `MutatePlainMergeTreeTask` + the message (plain `MergeTree`, test
`04341_broken_mutation_part_log_flush`);
- `MergeTreeBackgroundExecutor` + the message (the wrapping background-executor
line, present for both the plain and `ReplicatedMergeTree` cases).
The replicated first line is already covered by the existing
`MutateFromLogEntryTask` exclusion, so all leaking lines remain suppressed while
`throwIf` errors from any other context now surface.
Verified against the real leaking log from the example report
(https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=108657&sha=70df14b1ec107f06eb401ecb1bda859af68e52a6&name_0=PR&name_1=Upgrade%20check%20%28amd_release%29):
all 273 `MutatePlainMergeTreeTask`/`MergeTreeBackgroundExecutor` retry lines are
dropped, while a synthetic unrelated `throwIf` error (e.g. from a materialized
view assertion) still reaches `upgrade_error_messages.txt` - whereas the old
global filter masked it.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The query fuzzer and stress test could run queries that build pathologically
deep/large expressions, taking minutes to parse and format and tripping the
hung check, e.g.:
SELECT formatQuery($$SELECT [[[[ ...20000... ]]]]$$)
with `max_parser_depth=100000000`. The stress test ignores `no-*` build tags
(`tests/clickhouse-test`) and runs the fuzzer over every query, so a single
test setting these high is enough to hang the server under sanitizers.
Move the recursion constraints out of `query-fuzzer-tweaks-users.xml` into a
shared `limit-recursion-settings.xml` and install it in both the AST fuzzer and
the stress runner. It caps `max_ast_depth`, `max_parser_depth`,
`max_parser_backtracks`, `max_ast_elements` and `max_expanded_ast_elements` at
their defaults via `<max>` (lowering is still allowed), so neither the fuzzer
nor a test can raise them to values that build huge queries.
Seen in: https://s3.amazonaws.com/clickhouse-test-reports/PRs/108593/633f156999f8febe513b07e4334382a758e0a1ec/stress_test_amd_tsan/hung_check.log
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The narrow allowlisted message matches the `throwIf` mutation error from
both `04341_broken_mutation_part_log_flush` (plain `MergeTree`,
`MutatePlainMergeTreeTask`) and the
`02597_column_{update,delete,update_tricky_expression}_and_replication`
tests (`ReplicatedMergeTree`, where only the first `MutateFromLogEntryTask`
line was already excluded and the wrapping `MergeTreeBackgroundExecutor`
line leaked). Matching the message rather than the task type covers all
leaking lines. No behavior change beyond the comment; the rg entry is
unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The stateless test `04341_broken_mutation_part_log_flush` runs an
intentionally-failing async mutation
`ALTER TABLE ... UPDATE x = x + throwIf(1) WHERE 1` (expecting
`serverError UNFINISHED`) to assert that the failed `MutatePart`
`part_log` entry is flushed before the failure is reported to the client.
After the upgrade restart the broken mutation is retried in the
background and logs `Value passed to 'throwIf' function is non-zero`
(`FUNCTION_THROW_IF_VALUE_IS_NON_ZERO`, Code: 395) to
`clickhouse-server.upgrade.log`, which fails the `Upgrade check` with
`Error message in clickhouse-server.log (see upgrade_error_messages.txt)`.
This is the expected test-induced error, not a backward incompatibility
or a regression - exactly the same class as the `CANNOT_PARSE_TEXT`
mutation errors already allowlisted (issue #39174). Note the table uses
a plain `MergeTree`, so the error is emitted by `MutatePlainMergeTreeTask`
and is not covered by the existing `MutateFromLogEntryTask` exclusion.
`throwIf` only raises when a query explicitly calls it with a truthy
argument, so the message can only come from such a test, never from an
upgrade incompatibility; the narrow message substring also covers the
wrapping `MergeTreeBackgroundExecutor` line.
The failure is not caused by any product change - it surfaces on any
pull request that runs the Upgrade check. Observed master-wide on ~19
distinct pull requests over the four days 2026-06-26..2026-06-29
(confirmed via play.clickhouse.com `checks` history), e.g. PR #108657.
This mirrors the earlier allowlist additions for `04338` ('x'),
`01414` ('Hello') and `00834` ('a'/'b').
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The "Error message in clickhouse-server.log" step of the Upgrade check
scans the server log for `<Error>` lines and filters out a list of
known-expected errors. The test `01155_old_mutation_parts_to_do` runs
ALTER TABLE rmt UPDATE m = m*toInt8(s) WHERE n=3
against a row where `s = 'fail'`, which is a deliberately-failing
mutation (it tests the `parts_to_do` counter). After an upgrade restart
the `MergeTreeBackgroundExecutor` retries the broken mutation and logs
<Error> MergeTreeBackgroundExecutor: Exception while executing background task ...:
Code: 6. DB::Exception: Cannot parse string 'fail' as Int8: syntax error at begin of string. (CANNOT_PARSE_TEXT)
This is the same class of expected `CANNOT_PARSE_TEXT` error already
allowlisted for the other deliberately-broken-mutation tests
(`00834_kill_mutation`, `01414_mutations_and_errors_zookeeper`,
`04338_on_fly_mutation_read_overwritten_lc_source`), but the `'fail'`/
`Int8` variant was missing, so it leaked through and failed the Upgrade
check on many unrelated pull requests.
Add the narrow fixed-string pattern `Cannot parse string 'fail' as Int8`
to the exclusion list (matching the wrapping `MergeTreeBackgroundExecutor`
log line) and document the test in the comment block. The pattern is
specific to this value and type, so genuine upgrade-compatibility errors
are not masked.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The stateless test `04338_on_fly_mutation_read_overwritten_lc_source`
(table `t_overwrite_before_consume`) chains the async mutations
`UPDATE v = toString(id + 100)`, `UPDATE b = isNotNull(materialize(v))`
and `MODIFY COLUMN v UInt64` over a column whose on-disk value is the
unconvertible String `'x'`. This is an intentionally-broken mutation of
exactly the class already documented in the `CANNOT_PARSE_TEXT` allowlist
(issue #39174): a `MODIFY COLUMN ... UInt64` on un-parseable String data,
just like the already-allowlisted `'Hello'`/`'a'`/`'b'` variants from
`00834_kill_mutation` and `01414_mutations_and_errors_zookeeper`.
After the upgrade restart the broken mutation is retried and logs
`Cannot parse string 'x' as UInt64: syntax error at begin of string`
(`CANNOT_PARSE_TEXT`) to `clickhouse-server.upgrade.log`, which fails the
`Upgrade check` with `Error message in clickhouse-server.log (see
upgrade_error_messages.txt)`. This is not a backward-incompatibility or a
regression; it is the expected test-induced error.
Add the narrow `'x'` message to the upgrade-check allowlist and document
`04338` in the comment block. The failure is not caused by any product
change - it surfaces on any pull request that runs the Upgrade check
(observed on PR #108011 and the unrelated PR #107433).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Address review (AI Review "Request changes"): the upgrade settings-history
check excluded every changed setting whose new value contained `auto(`
(`new_value NOT LIKE '%auto(%'`). That is broader than the intended
quoted-to-unquoted rendering change, so a genuine default change to an
`auto(N)` value (e.g. `4` -> `auto(8)`, or a brand-new auto-defaulted
setting) could escape the required `SettingsChangesHistory.cpp` entry.
Suppress only the pure rendering difference: a row where the old value is
exactly the new value wrapped in single quotes (the legacy `'auto(N)'` form).
A genuine change of an auto-valued setting's default is still reported.
Applied to both the `system.settings` and `system.merge_tree_settings`
queries.
Verified with `clickhouse-local` that the new condition drops the
quote-only difference and keeps genuine changes and brand-new auto settings.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The automatic value of settings like `max_threads` shows up in
`system.settings` as `'auto(64)'` - with literal single quotes baked
right into the string value. They are pure garbage and have annoyed me
for years.
Back in 2023 I removed them in a commit titled, with great restraint,
"Fix strange trash". Konstantin promptly put the trash back in #68979
(branch `put-strange-trash-back`, commit "Revert \"Fix strange trash\"").
And, in fairness, he was right at the time: the parser only understood
the quoted form, so a new un-quoted server could not parse the
`'auto(N)'` that older replicas send over the wire as a setting string,
which broke mixed-version clusters (#68748).
That obstacle is now gone. `stringToMaxThreads` accepts both `auto(N)`
and the legacy `'auto(N)'`, so emitting the clean form is safe in both
directions: an old replica's quoted value is still parsed by us, and
every released version can already parse the un-quoted value we now
send. So the strange trash goes back out - and this time the tolerant
parser stays, so it stays out for good.
Changes:
- `SettingFieldMaxThreads::toString` now returns `auto(N)` without quotes.
- Documented why `stringToMaxThreads` must keep accepting the legacy
quoted form (it is the mixed-version-cluster compatibility guarantee).
- Updated test `04202_set_default_preserves_max_threads_auto` and the
`test_cgroup_limit` integration test to match the clean form.
- The upgrade check (`upgrade_runner.sh`) now ignores `auto(...)` values
when diffing settings against the previous release, so the
quoted-vs-unquoted rendering is not reported as a setting change.
- Updated the `system.settings` example in the docs.
Related: https://github.com/ClickHouse/ClickHouse/issues/68748
Related: https://github.com/ClickHouse/ClickHouse/pull/68979
Related: https://github.com/ClickHouse/ClickHouse/pull/52503
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The upgrade check asserts that the server log contains no <Error> lines.
Test 04210_show_remote_databases_in_system_tables creates two remote database
engines pointing at the deliberately unreachable host 192.0.2.1 (RFC5737
TEST-NET-1):
- ENGINE = PostgreSQL('192.0.2.1:5432', ...)
- ENGINE = MySQL('192.0.2.1:3306', ...)
PostgreSQL: DatabasePostgreSQL::loadStoredObjects activates the
removeOutdatedTables cleaner task, whose periodic pool->get() times out against
the unreachable host and logs <Error> for each retry, including right after the
upgrade restart reloads the persisted engine.
MySQL: the engine probes the server while loading the persisted object after the
upgrade restart, logging <Error> from mysqlxx::Pool, the PoolWithFailover retry
logger, and the DatabaseMySQL ALL_CONNECTION_TRIES_FAILED exception.
The upgrade-test environment has no real PostgreSQL or MySQL server, so these
connection errors are fixture noise, not a compatibility regression (real
PostgreSQL/MySQL regressions are covered by integration tests with a live server).
Allowlist them in upgrade_runner.sh by requiring the component AND the
connection-failure symptom together, so genuine DatabasePostgreSQL/DatabaseMySQL
errors (logic, parsing, a different error code) still surface.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
The filesystem cache background-download worker (CacheMetadata,
src/Interpreters/FileCache/Metadata.cpp) continues prefetching a
partially-downloaded file segment. Under stress the underlying S3 object can
be concurrently removed (DROP, mutation, part removal), so the GET returns
Code: 499 NoSuchKey. The worker catches this, marks the segment download
failed and logs "Error during background download"; the data is simply
re-read from the source later, so it is benign.
The stress critical-error scan greps for "Code: 499 ... specified key does not
exist" and already excludes the foreground form of this event (ReadBufferFromS3,
AsynchronousBoundedReadBuffer, "ReadBuffer is canceled by the exception"), but
not the background-download form, so it was flagged as a lost s3 key.
Add "Error during background download" to the grep -v allowlist. The token is
unique to the CacheMetadata background-download log line, so a genuinely lost
key reported from any other path is still flagged.
Runtime regression from the final lowercase pass: it renamed <clickhouse> opening tags to
<datastore> but the / exclusion (protecting /clickhouse paths) left </clickhouse> closing tags,
so embedded + shipped configs became <datastore>...</clickhouse> -> SAXParseException 'Tag
mismatch' at runtime (compile was fine; XML is data). Renamed all <clickhouse/</clickhouse XML
tags -> datastore (101 closes across configs, embedded strings, keeper configs). Path strings
(/var/lib/clickhouse) have no angle bracket and are untouched. Build compiled; this fixes runtime
config parsing.
Real fork of ClickHouse/ClickHouse with the Hanzo overlay applied on top of the
26.5 release lineage (parent is upstream/26.5, so future upstream syncs are clean
3-way merges). Overlay: cmd/zap-bridge (:9999 ZAP duplex), Dockerfile.hanzo +
hanzoai/debian13 bases, hanzo/ config + schema, white-label hanzo-datastore
branding, arcd CI, hanzo-* packaging.
FOLLOW-UP: re-apply Phase-1 CMake decisions (single-binary ENABLE_DATASTORE_KEEPER=OFF;
disabled external engines) + full build on hanzo-build-linux-amd64.
`download_release_packages` now exits nonzero when a required
previous-release package is missing or fails to download, but the only
shell caller, `tests/docker_scripts/upgrade_runner.sh`, ran it inside an
`&& ... || ...` list, so `set -e` did not stop on the nonzero status. The
follow-up guard only checked `clickhouse-common-static_*.deb` and
`clickhouse-server_*.deb`, so a missing `clickhouse-client_..._amd64.deb`
or `clickhouse-common-static-dbg_..._amd64.deb` could still slip through
to `install_packages` and die there with an opaque `dpkg` glob error -
exactly the failure mode this PR set out to make clear and attributable.
Handle the download exit code explicitly: on failure, record the `FAIL`
row, write a `failure` status to `check_status.tsv`, and `exit 1` before
`install_packages` can run; on success, record the `OK` row and continue.
Add `ci/tests/test_upgrade_runner_download_boundary.py`, which extracts
the real download block from the runner (between stable anchor comments)
and runs it under bash with a stub `download_release_packages`, proving a
failed download stops before `install_packages` is reached and a
successful one proceeds. Extracting the live block keeps the test from
drifting from the script it guards.
Addresses review feedback:
https://github.com/ClickHouse/ClickHouse/pull/107916#discussion_r3444526378
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The setting lives in `<coordination_settings>`, which the server parses
strictly: an older binary that does not know it refuses to start with
`UNKNOWN_SETTING`. The upgrade check starts the previous release with the
current config, so it failed to start ("Cannot start clickhouse-server").
Gate the `keeper_port.xml` setting strip on a new `--previous-release` flag in
addition to `--bugfix-validation`, and pass `--previous-release` from the
upgrade check's previous-release `configure` (not the post-upgrade one). Strip
`nuraft_use_bg_thread_for_snapshot_io` there; drop the `use_xid_64` and
`remove_recursive` strips, which the previous release already supports.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
The stress smoke check runs CREATE DATABASE ... ON CLUSTER. With thread-pool
fault injection enabled before the server starts, it could hit
CANNOT_SCHEDULE_TASK while the Replicated database loaded; the server then
broke the chunked HTTP response and the whole stress job aborted (recurring
~weekly across unrelated PRs).
stress_runner.sh no longer exports THREAD_POOL_FAULT_INJECTION before the
server starts. stress.py installs cannot_allocate_thread_injection.xml and
reloads config only after the smoke check passes, then verifies
cannot_allocate_thread_fault_injection_probability is non-zero (fail-close).
The upgrade check keeps its previous behavior of running without it.
The upgrade-check post-restart `<Error>` scrub catches a transient lock-timeout
emitted by `SystemLog<MetricLogElement>::flushImpl` when the background `MetricLog`
flush loop races with shutdown work on `system.metric_log` during the upgrade restart
sequence:
```
<Error> void DB::SystemLog<DB::MetricLogElement>::flushImpl(...) [LogElement = DB::MetricLogElement]: Failed to flush system log system.metric_log with N entries up to offset M: Code: 473. DB::Exception: WRITE locking attempt on "system.metric_log" has timed out! (60000ms) Possible deadlock avoided. Client should retry. Owner query ids: . (DEADLOCK_AVOIDED), Stack trace ...
```
Another worker (DROP/RENAME/DETACH on `system.metric_log`, or a parallel
mutation/merge) holds the table-level write lock, the flush blocks for the
60s timeout, and then aborts with code 473 (`DEADLOCK_AVOIDED`). The 473 code is
the engine's own "gave up to avoid a deadlock, not a crash" signal — losing a few
`MetricLogElement` samples during shutdown is harmless; the upgrade-check stage is
not asserting on metric continuity.
CIDB confirms this is chronic CI noise, not a code regression: 4 hits across 4
distinct PRs in the last 30 days, 0 master, 100% on `Upgrade check (amd_release)`.
Reported by @ alexey-milovidov on PR #102017 (cited #106425, #104561, #106202).
The added regex requires BOTH the `SystemLog` flush wrapper for `metric_log` AND
the `DEADLOCK_AVOIDED` error code together, mirroring the narrowing approach of
the existing `rdk:FAIL.*Connect to.*failed: Connection refused` and
`StorageKafka2.*Broker transport failure` filters. Real `metric_log` regressions
(other flush errors) and unrelated `DEADLOCK_AVOIDED` hits on other system_log
tables (`query_log`, `text_log`, etc.) still surface.
Related: https://github.com/ClickHouse/ClickHouse/pull/102017
Related: https://github.com/ClickHouse/ClickHouse/pull/106563
Related: https://github.com/ClickHouse/ClickHouse/pull/101576
Co-Authored-By: Claude Opus 4.8 <[email protected]>
zeekayandHanzo Dev