patch-port: v26.3.26.2 - #47
Merged
Merged
Conversation
Single bootstrap commit for the v26.3.15.4 reslice: the durable orchestration system (docs/aiven AGENTS.md, README, schema, skills, runbooks, proposals, plans) plus the Cursor safety hooks (.cursor/hooks.json + .cursor/hooks/). Category-A material only — no patch code, no patch dossiers (those are the code and docs buckets). Reproduces the bootstrap tree from v26.3.15.4-lts-aiven-dev verbatim. (cherry picked from commit 39386b8)
This patch removes the default registration of the /replicas_status HTTP endpoint to reduce the default attack surface and prevent exposure of replication state information by default. The /replicas_status endpoint provides information about the status of replicated MergeTree tables, including replication lag and detailed state information. While useful for monitoring, this information should not be exposed by default for security and privacy reasons. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit 1151af4) (cherry picked from commit 3507427)
Main service users are allowed the SHOW DATABASES access because it is necessary for their operation. It is implicitly granted by ClickHouse when giving access to a database. However, we do not want to give them access to SHOW CREATE DATABASE. This query shows the entire create statement, unredacted. This is actually a useful feature for superusers, but can leak credentials to other users. Also only show the create query in system.tables to users that were able to create that table. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit 654f61e) (cherry picked from commit 931e184)
Author: Joe Lynch <joelynch112@gmail.com> Committer: Joe Lynch <joelynch112@gmail.com> (cherry picked from commit 71478f5)
SettingsChangesHistory in the 25.3 build contains entries cherry-picked from 25.5/25.6 that reference settings not present in 25.3 (e.g. parallel_replicas_connect_timeout_ms). Setting compatibility to any value causes applyCompatibilitySetting to call get() on these unknown settings, crashing with UNKNOWN_SETTING. Fix: skip history entries for settings that don't exist in the current build. (cherry picked from commit aec2378) (cherry picked from commit 406526f)
Fixes `Code: 139. DB::Exception: No macro 'shard'` thrown on server
startup when a `DatabaseReplicated` database contains a
`ReplicatedMergeTree` table whose ZooKeeper path includes the `{shard}`
macro, in deployments where the server config does NOT provide a
global `<shard>` macro (the per-database shard name from the
`DatabaseReplicated` engine arguments is the authoritative source).
The patch extends the existing `if (is_replicated_database)` branch in
`TableZnodeInfo::resolve` (`src/Storages/TableZnodeInfo.cpp:58`) to also
fire when `query.attach` is true, so the `clickhouse-server`-startup
loader's local `ATTACH` operations populate `info.shard`/`info.replica`
from the containing `DatabaseReplicated` instead of relying solely on
the configuration macros.
Source SHA on `v25.8.18.1-lts-aiven`: `22e03c9d9d6cf9929aec824b724e09ea5c58653f`.
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2025-12-07.
Co-author: Kevin Michel <kevin.michel@aiven.io>.
Committer (25.8 carry): Aliaksei Khatskevich, 2026-03-12.
## Upstream-drift conclusion
Still-needed-and-applies. Zero upstream commits to
`src/Storages/TableZnodeInfo.cpp` in the `v25.8.18.1-lts..v26.3.10.62-lts`
range. The file's blob on HEAD (`b829b25dfeb`) is byte-identical to the
source commit's pre-patch blob; the patch's `@@ -55,7 +55,7` hunk
applies at the same physical lines. Cherry-pick is clean
(`git cherry-pick --no-commit -x` exits 0 with no markers);
`git patch-id --stable` matches byte-for-byte
(`108c7f731670ff9ceb744dd951544cb31ada0353`); `byte_equivalent: true`.
No superseding upstream fix: `rg` for `{shard}`, `shard macro`,
`restart.*replicated` across `src/Storages/`, `src/Databases/`,
`src/Interpreters/` in the same revision range returned zero matches.
## Integration test added
`tests/integration/test_aiven_replicated_database_attach_with_shard_macro/`,
naming per `docs/aiven/runbooks/testing-suites.md` §4.4 (the Aiven
integration-test convention introduced in the preceding Bootstrap
commit). Two nodes with `<replica>` macro only (no `<shard>`),
Keeper backing, `DatabaseReplicated` with literal shard
`aiven_shard_a`, `ReplicatedMergeTree` table created via the default
`default_replica_path` (which still contains `{shard}`), restart of
both nodes via `node.restart_clickhouse(kill=True)`, final SELECT
assertion.
### Pre/post evidence pair (VERIFIED 2026-05-26)
| Run | Result | Wall-clock |
|---|---|---|
| Post-patch #1 | PASS | 41.7 s |
| Pre-patch | FAIL | 99.3 s |
| Post-patch #2 (sanity after restore) | PASS | 43.9 s |
Pre-patch failure mode (verbatim from the failing node's
`clickhouse-server.err.log`):
Code: 139. DB::Exception: No macro 'shard' in config while
processing substitutions in '/clickhouse/tables/{uuid}/{shard}'
at '27' or macro is not supported here: Cannot attach table
`testdb`.`t` from metadata file ... from query ATTACH TABLE
testdb.t UUID '...' (`x` UInt32) ENGINE = ReplicatedMergeTree
('/clickhouse/tables/{uuid}/{shard}', '{replica}') ORDER BY x
SETTINGS index_granularity = 8192.
(NO_ELEMENTS_IN_CONFIG)
The server's startup load job fails; the python helper's
`restart_clickhouse` raises `Exception: Cannot start ClickHouse`. This
is the customer-reported failure mode in the exact form they observed.
### Calibration note
The T3.6 worker and parent preflight both predicted `Code: 62. DB::
Exception: No macro 'shard'`. The empirically-observed error code is
`Code: 139. NO_ELEMENTS_IN_CONFIG` — same root cause, different
codepath that throws. The dossier §4 records this as a calibration
note for future preflights. The test's assertion is on the python-side
`Exception: Cannot start ClickHouse`, so the calibration doesn't
weaken the test.
### C++ review noted limitations
Per `docs/aiven/skills/cpp-review-checklist.md` §2/§6, the patch
introduces a documented limitation: when post-patch the if-block
enters via `query.attach=true` on a NON-`Replicated` database (a
hypothetical edge),
`getReplicatedDatabaseShardName(database)` performs
`assert_cast<const DatabaseReplicated *>(database.get())`. In release
builds `assert_cast` is `static_cast` (UB on type mismatch); in
debug/sanitizer builds it calls `throwBadAssertCast`. Pre-patch the
same scenario threw a clean `DB::Exception: No macro 'shard'` from the
subsequent `Macros::expand`. The post-patch failure mode for this
non-Replicated-DB edge is therefore different and (in debug builds)
less informative. The integration test does NOT exercise this edge
(it uses a `DatabaseReplicated` happy path, which is the customer's
shape); the dossier §3 documents the limitation for future
maintenance.
The same idiom (`query.attach` as a sufficient guard) is used at
`src/Storages/TableZnodeInfo.cpp:28` for `allow_uuid_macro` — i.e.,
this patch literally extends the precedent in the surrounding code by
one line.
## Dossier
`docs/aiven/patches/006-replicated-database-attach-with-shard-macro.md`
captures the full upstream-drift analysis, the C++ review, the
test-design rationale (including the three rejected SQL triggers from
the T3.6 worker's escalation), the assert_cast limitation, the
rollback plan, and the per-uplift notes.
Per `docs/aiven/runbooks/commit-hygiene.md` §1, this commit is pure
category (C) patch-port: src + test + dossier + uplift log row, all
telling one story. Bootstrap-class additions (the integration-tests
runbook and the `test_aiven_<slug>/` naming convention) landed in the
preceding Bootstrap commit per §2's dependency-order rule.
(cherry picked from commit d2f78fd)
…abaseReplicated Reduce the default value of setting from 1000 to 300 for DatabaseReplicated databases. This reduces ZooKeeper resource consumption by ~70% while maintaining a 6x safety margin over max_replication_lag_to_enqueue (50). Context: Previously, we implemented a server-level setting (replicated_database_logs_to_keep) to centralize control of this value. However, after analysis, we determined that: 1. Customers do not have ALTER_DATABASE_SETTINGS permission, so they cannot modify database settings via ALTER DATABASE MODIFY SETTING 2. The simpler approach of changing the database-level default is sufficient 3. No additional readonly checks are needed since access control already prevents customer modifications This change affects only newly created databases. Existing databases retain their current logs_to_keep value stored in ZooKeeper. The default value of 300 provides adequate recovery buffer while significantly reducing ZooKeeper memory usage in multi-database managed provider environments. Co-authored-by: Khatskevich <khatskevich@aiven.io> Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2025-12-08. (cherry picked from commit 199db08) (cherry picked from commit 0cf4c9c)
…d exponential backoff When ZooKeeper restarts (especially during version upgrades), it can be unavailable for approximately 6 seconds. ClickHouse previously failed queries if ZooKeeper was not available within ~3 seconds, leading to inconsistent database state because DDL operations are not fully atomic. This change improves ZooKeeper connection resilience by: - Increasing minimum retry attempts from 3 to 6 - Adding exponential backoff between retry attempts (100ms, 200ms, 400ms...) - Capping maximum backoff at 10 seconds to prevent excessive delays The total retry window now covers typical ZooKeeper restart times (~6 seconds), allowing ClickHouse to successfully reconnect after ZooKeeper restarts without requiring manual intervention. This is particularly important during version upgrades when ZooKeeper nodes restart sequentially, as it prevents DDL operations from failing mid-execution and leaving the database in an inconsistent state. Regression test added at tests/integration/test_aiven_zk_connect_retry/ isolates the connection-retry layer by exercising the first ZK use (CREATE TABLE of a fresh ReplicatedMergeTree) on a freshly-restarted server while the 3-node Keeper cluster is briefly unavailable. Pre-patch the test fails with Coordination::Exception: All connection tries failed (3 tight retries with no inter-attempt sleep); post-patch it passes because the 6-retry exponential-backoff loop covers the 10s outage. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2025-12-06. (cherry picked from commit 6a37150) (cherry picked from commit 940ecef)
`alter code` is detached from `create table` code, which makes it necessary to copy field initializaiton logic. This commit makes `alter table` to produce the same `sorting key` ZooKeeper metadata as `create table`. This commit was applied from the patch file 0087-Fix-alter-order-by.patch Source-only port, shipped without a stateless test. Three verification attempts on direct ORDER BY mutation paths (T3.10 `MODIFY ORDER BY`, T3.11 `ADD COLUMN + MODIFY ORDER BY`, T3.12 `RENAME COLUMN`) failed to produce evidence-of-causation on 26.3: each scenario is either upstream-rejected by `MergeTreeData::checkProperties` / `MergeTreeData::checkAlterIsPossible` before the patched line is reached, or reaches the line but is observationally inert because `AlterCommands::apply`'s `MODIFY_ORDER_BY` branch (`src/Storages/AlterCommands.cpp:653-666`) writes through `primary_key.definition_ast` without restoring the `nullptr` invariant that the patched `isPrimaryKeyDefined()` predicate depends on. This is not a regression on 26.3. The `AlterCommands.cpp` divergence reached its current shape in upstream commit `465c4b65b72` (2020-06-12), ~5.5 years before the source patch was authored (`93c2be960f9`, 2026-01-13). So the patch was observationally inert on direct `MODIFY ORDER BY` on the 25.8 source branch too. Source is byte-equivalent (patch-id `27037b121ff8df19a7d61bcd20cfed16a53364b2` matches), surrounding-code context at the patched line is byte-stable (per dossier §2), and the only material upstream-divergence that defeats the predicate predates the patch. Shipping on 26.3 produces the same observable behavior as shipping on 25.8. Full verification record and no-regression proof: docs/aiven/patches/060-alter-order-by-sorting-key-zk-metadata.md §4. Worker reports for the two unsuccessful verification dispatches: docs/aiven/uplifts/26.3/reports/T3.11-postpatch-fail.md, docs/aiven/uplifts/26.3/reports/T3.12-postpatch-fail.md. Co-authored-by: Aliaksei Khatskevich <alex.khatskevich@aiven.io> Original author: Tilman Moeller <tilman.moeller@aiven.io> (cherry picked from commit 93c2be9) (cherry picked from commit 5f5d8be)
…shard macro in the target table
Refreshable materialized views use ZooKeeper coordination paths that are
expanded from server settings like default_replica_path and default_replica_name.
These paths can contain macros such as {shard}, {database}, {table}, and {replica}
that need to be expanded to actual values.
When a refreshable materialized view is created in a DatabaseReplicated database,
the coordination path may contain the {shard} macro. However, the macro expansion
was not including the shard name in MacroExpansionInfo, causing the {shard} macro
to remain unexpanded in the coordination path.
This fix:
- Retrieves the database from DatabaseCatalog
- Checks if it's a DatabaseReplicated database
- If so, sets info.shard to the shard name from the database
- This ensures {shard} macros are properly expanded in coordination paths
Without this fix, refreshable materialized views in DatabaseReplicated databases
would fail to coordinate correctly across replicas when the coordination path
contains shard macros, leading to incorrect ZooKeeper paths and coordination
failures.
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2026-01-07.
Co-authored-by: Joe Lynch <joe.lynch@aiven.io>
(cherry picked from commit cc745f5)
(cherry picked from commit 6e1b184)
When a replicated table is created and then deleted, the immediate parent ZooKeeper znode may remain empty and not be cleaned up, causing a node leak in ZooKeeper. This can lead to accumulation of orphaned empty znodes over time, polluting the ZooKeeper namespace. The existing `dropAncestorZnodesIfNeeded()` method in `TableZnodeInfo` removes ancestor znodes from the table path up to `path_prefix_for_drop`, but it may not handle the immediate parent znode in all cases. Fix by adding a new method `dropAncestorTableZnodeIfNeeded()` that specifically removes the immediate parent znode of the table path if it becomes empty after table deletion. This complements the existing cleanup logic and ensures no orphaned znodes are left behind. Co-authored-by: Joe Lynch <joe.lynch@aiven.io> (cherry picked from commit 0d6eb5b) (cherry picked from commit 7716ed0)
…tions validation Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2026-01-02. We need to keep track of existing named collections. In order to do that, we wish to add some metadata to each collection. The metadata keys are added as optional collection parameters for the storages and functions that validate the keys. This change allows `integration_id` and `integration_hash` keys to be present in named collections without triggering validation errors. These metadata keys are whitelisted in the validation function, allowing integrations to track which collections they own or manage without breaking existing validation logic. Changes: - Added whitelist check for `integration_id` and `integration_hash` keys in validateNamedCollection() function - These keys are now silently ignored during validation, allowing them to be stored in named collections without being listed as required or optional keys Co-authored-by: Aris Tritas <aris.tritas@aiven.io> (cherry picked from commit 65248bd) (cherry picked from commit 6201bdd)
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2026-01-09. The metadata_version.txt file was added to each part folder to help with concurrency issues when mutating the table schema, but it was not included in the frozen files when creating backups. We need to include this file in the frozen shadow/ folder because we use these files to know what to backup. When using object storage, this file is actually a pointer to the file in object storage. We use these pointers to know which files are still referenced by ClickHouse and should not be deleted. By omitting this file, we would not know about it, and delete the metadata_version.txt file in object storage. This was causing latent issues: ClickHouse doesn't immediately notice the missing file, but instead complains loudly when adding a new replica - the new replica tries to download this file when syncing from existing replicas. The fix sets keep_metadata_version = true in ClonePartParams when freezing parts, ensuring the metadata_version.txt file is preserved in frozen backups. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit b18cff8) Port note: the cherry-pick conflicted on whitespace only (the freeze block is nested one indentation level deeper on 26.3 than on 25.8); resolved by hand keeping HEAD's indentation. `git patch-id --stable` matches the source (3d8926365444c527b71cd5607ecee994d8252604), so the change is semantically byte-equivalent. Test: tests/queries/0_stateless/9056_freeze_include_metadata_version.sh. (cherry picked from commit 29b9ef6)
During a maintenance upgrade, we do not wait for the completion of merges and mutations since they are not required to make sure we have all the data from the previous nodes. We do wait for `GET_PART` and similar tasks, since we need the new nodes to get parts from the old nodes. This is implemented using `SYSTEM SYNC REPLICA ... LIGHTWEIGHT`. Some merges and mutations prevent the execution of `GET_PART` tasks that overlap the range of the merge or the mutation. So, even if we are not waiting for `MERGE_PARTS` tasks, these tasks can slow down the completion of a maintenance upgrade. We need to execute `MERGE_PARTS` tasks to avoid having too many parts in the same partition or table. We also need to execute the TTL delete rules and ensure disk usage does not grow too much (TTL deletes are implemented as a subtype of the merge tasks). A possible tradeoff is to only execute merge tasks if they are not too large, the small merges are the ones that keep the number of parts low when the are many small INSERT queries. We can't use the normal merge tree settings like `max_bytes_to_merge_at_min/max_space_in_pool` because they would be applied with `ALTER TABLE` which would step onto user-managed objects and become persisted as part of the table definition. They are also replicated, we can't set them only on new nodes. The patch implements per-server overrides, that we can use to limit the size of merge and mutate tasks, both when a node creates new tasks and when it decides which task to execute. This is only usable with a `LIGHTWEIGHT` sync: if we don't execute some tasks, we also need to not wait for them, or we would wait forever. `SYNC REPLICA` with the `LIGHTWEIGHT` flag allows us to not wait for these tasks. This commit was applied from the patch file 0078-Global_merge_and_mutate_override.patch Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2026-01-06. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit 4a05c78) Port note: still-needed-but-rewrite (not a clean cherry-pick). Upstream renamed the target functions Size->Bytes (getMaxSourcePartsSizeForMerge -> getMaxSourcePartsBytesForMerge, getMaxSourcePartSizeForMutation -> getMaxSourcePartBytesForMutation), so the size-cap logic was hand-applied into the renamed functions, preserving HEAD's static_cast<double>(disk_space) cast and HEAD's '<' log-comment wording. The source patch's setMaxPendingMutationsToWarn SharedLockGuard->std::lock_guard hunk was dropped: it is already std::lock_guard on HEAD (obsoleted-by-upstream). Tested by a new integration test (tests/integration/test_aiven_per_server_max_bytes_merge_mutate_override/) since the feature is two server settings (config-file only); verified by a behavioral pre-fail/post-pass pair. (cherry picked from commit 81e3fbf)
…g DDL queries Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2026-01-09. The constraints were previously only checked when executing the query from the replicated DDL queue. At that point the DDLWorker was using the system profile for settings constraints. This meant that a non-admin user, using the default profile, could use a MergeTree settings that was not allowed by its profile. Fix that by checking the MergeTree settings constraints before the query is enqueued, when we still have the query context attached to the user running the query. We check both Replicated*MergeTree and apparently non-replicated MergeTree because this check happens before we rewrite the query to enforce Replicated*MergeTree engine types. This commit fixes a security vulnerability where users could bypass profile-based settings restrictions by submitting DDL queries to DatabaseReplicated. The fix ensures that: 1. Constraints are checked early (before enqueuing) when user context is still available 2. User's profile restrictions are properly enforced, not system profile 3. Both CREATE TABLE and ALTER TABLE SETTING queries are validated 4. All MergeTree variants are covered (before query rewrite) Port note (26.3-aiven): the cherry-pick conflicted on the #include block only. Upstream relocated <Parsers/ASTUpdateQuery.h> (now after <Parsers/ASTFunction.h>), so <Parsers/ASTSetQuery.h> was inserted by hand in HEAD's alphabetical include order; the HEAD side of the conflict was empty (no competing change). Both code hunks (checkTableEngine for CREATE, checkQueryValid for ALTER ... MODIFY SETTING) applied cleanly. The staged diff's `git patch-id --stable` differs from the source only in include-context lines; the +/- content lines are byte-identical (decomposition empty), so the change is semantically equivalent. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> Co-authored-by: Aliaksei Khatskevich <alex.khatskevich@aiven.io> (cherry picked from commit bb04848) (cherry picked from commit e2fbcee)
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2026-01-05. This is a convenience patch to avoid polluting logs on developer laptops with GPU drivers that advertise non-working thermal sensors. The patch disables initialization of EDAC (Error Detection And Correction) and hardware monitoring chip sensors by removing the calls to openEDAC() and openSensorsChips() from the AsynchronousMetrics constructor. Port note (26.3-aiven): clean cherry-pick, staged diff patch-id-identical to the source. No automated test ships with this patch: the LOG_WARNING it suppresses is emitted only when a hwmon/EDAC sensor node exists but cannot be read, which CI runners do not have, and AsynchronousMetrics reads absolute /sys paths that a stateless test cannot fault-inject. Classified test_design_blocked and accepted as a deliberately-carried, low-risk 2-line removal. The update-loop openEDAC()/openSensorsChips() calls are catch-only recovery paths that never fire when the vectors start empty, so removing the constructor calls is sufficient and the purpose is preserved on 26.3. See docs/aiven/patches/037-ignore-unreadable-sensors.md. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit 7a7058e) (cherry picked from commit 9cc2448)
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2026-01-05. This patch disables the ThreadFuzzer pthread wrapping feature by always setting THREAD_FUZZER_WRAP_PTHREAD to 0, regardless of platform or sanitizer settings. The pthread wrapping feature has compatibility issues with newer glibc versions (especially glibc 2.36+) and is a testing feature that should not be enabled in production builds. This patch simplifies the code by removing conditional compilation logic and ensuring consistent behavior across all platforms. Port note (26.3-aiven): clean cherry-pick, staged diff patch-id-identical to the source. Classified no_justified: it is a compile-time toggle of test-only instrumentation (ThreadFuzzer pthread wrapping) with no runtime-configurable behavior and no SQL surface, so causation is verified at the build-artifact level rather than via a query. nm on ThreadFuzzer.cpp.o built both ways shows the pthread_mutex_lock/pthread_mutex_unlock interposers and 16 tuning statics present pre-patch and absent post-patch; the pre-patch object's U __pthread_mutex_lock@GLIBC_2.17 forward-reference corroborates the glibc-2.36+ rationale. ninja -C build clickhouse exits 0. See docs/aiven/patches/039-disable-thread-fuzzer.md. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit 32e9abc) (cherry picked from commit e09482b)
Port of `8fc1c96ae0` from `v25.8.18.1-lts-aiven`. Adds a per-disk `<ca_path>`
setting so an S3 disk (or `<s3>` endpoint) can trust a private or self-signed CA
for TLS without weakening the global `openSSL.client` verification or installing
the CA into every node's system trust store. The bundle is loaded into a
per-client `Poco::Net::Context` (`VERIFY_RELAXED`, depth 9, `loadDefaultCAs =
false`) and threaded through `makeHTTPSession` into the HTTP connection pool.
Classified `still-needed-but-rewrite`: the 25.8 patch carried separate get/put
request throttlers, whereas 26.3 consolidates them into a single
`request_throttler` struct, so the conflict was resolved onto the 26.3 shape. The
`makeHTTPSession` signature gains a trailing context parameter, so all of its
callers are updated to pass a default context.
This commit also fixes two bugs present in the original patch:
- Connection-pool trust isolation. `EndpointPoolKey` did not include the SSL
context, so connections to one endpoint shared a pool regardless of trust
anchor ("first context wins"): a disk with no `ca_path`, or a different
`ca_path`, could reuse a connection that was verified against another CA. The
pool key now includes the context, so distinct trust anchors get distinct
pools; a null context keeps the previous default pool.
- The `Poco::Net::Context` was rebuilt (CA file re-read and re-parsed) inside the
per-attempt request loop on every request and redirect. It is now built once
per client via `makeCAContext` and stored in `ca_context`. Besides removing the
per-request work, this gives the context a stable pointer identity, which is
what lets the pool-key fix above reuse pools instead of spawning a new pool per
request.
Note: with `loadDefaultCAs = false`, a `ca_path`-configured disk trusts only the
supplied bundle and cannot also reach a public-CA endpoint. This matches the
patch intent and is kept as-is.
Adds integration test `test_aiven_s3_custom_ca_path`: a positive case (a disk
with `ca_path` reaches a self-signed MinIO under strict global verification) and
a negative case (a disk without `ca_path`, pointed at the same endpoint, must
fail certificate verification, which also exercises the pool-key isolation). The
MinIO/CA certificate is generated fresh at test start so it cannot expire.
See `docs/aiven/patches/012-s3-custom-ca-path.md`.
(cherry picked from commit 00c3fa4)
Port of the Aiven feature "Allow custom CA certificate path for Azure Blob Storage connections" (25.8-aiven 2f70d49) onto 26.3. Classified still-needed-but-rewrite: the original patch configured the Azure SDK's Curl transport (curl_options.CAInfo on CurlTransportOptions), but 26.3 migrated Azure HTTP to ClickHouse's own Poco client (PocoAzureHTTPClient), so the original mechanism no longer exists. This rewrite mirrors the S3 patch 012 on the Poco transport: - RequestSettings.ca_path read from the per-disk <ca_path> config key; - threaded into PocoAzureHTTPClientConfiguration; - PocoAzureHTTPClient builds a Poco::Net::Context once in its constructor (makeCAContext, internal linkage to avoid an ODR clash with S3's) and passes it to makeHTTPSession instead of an empty context. Trust isolation is inherited from patch 012: the HTTP connection pool is keyed by the SSL context, so a custom-CA Azure disk gets its own pool. Disks without <ca_path> are unchanged (empty context -> global default client context). User-facing config is identical to the original patch; only the internal transport differs. Ships a proportionate integration test (test_aiven_azure_custom_ca_path) against the HTTP Azurite harness: a valid <ca_path> is accepted and the disk operates; a nonexistent <ca_path> fails with "File not found: <path>", proving the value is read and consumed by makeCAContext. End-to-end TLS-verification evidence for the shared makeHTTPSession/HTTPConnectionPool/Poco::Net::Context machinery is inherited from patch 012's S3 test. See docs/aiven/patches/013-azure-custom-ca-path.md. (cherry picked from commit 9a05310)
Port of `e65f68836b` from 25.8-aiven. Applied cleanly onto 26.3 (no
conflicts; all anchors present), so this is a faithful port plus a small
style cleanup and a stateless test.
The vulnerability: a user constrained by a restrictive `default` profile but
holding `CREATE USER` / `CREATE SETTINGS PROFILE` can escape its constraints
by creating a user (or profile) that points directly at a less restrictive
profile (e.g. an `admin` profile) and then connecting as that user. The
profile-switch verification that normally blocks moving to a less restrictive
profile is skipped when a profile is assigned directly at create time.
The fix adds an `allow_non_default_profile` setting (default `true`, i.e.
protection off) and enforces, when it is `false`, that every profile assigned
via SQL must be the configured `default` profile or one of its descendants.
The enforcement lives in the shared `SettingsConstraints::check` and is not
keyed on `SettingSource`, so the single check covers `CREATE`/`ALTER` of
profiles (`SettingSource::PROFILE`), users (`SettingSource::USER`) and roles
(`SettingSource::ROLE`) — closing the user-creation vector, not only profile
creation. A deployment enables the protection by setting
`allow_non_default_profile = false` (locked) in the `default` profile, while
the `users.xml`-defined `admin` profile keeps it `true`.
`InterpreterCreateSettingsProfileQuery::execute` additionally:
- rejects inheritance cycles on `ALTER` (unconditional, not gated by the
setting);
- auto-injects the `default` profile as a parent when the guard is on and no
parent is specified, so operators need not spell out `INHERIT default`.
Known limitation, kept faithful to 25.8: the profile-graph traversal
`SettingsProfilesCache::isExpectedProfileOrDescendantLocked` recurses without
a visited-set, so a cyclic profile graph that reaches the cache by a path that
bypasses the new `ALTER` cycle check (a `users.xml` definition, or a profile
persisted by an older binary and reloaded after upgrade) can overflow the
stack while the cache mutex is held. SQL cannot create such a cycle, the
realistic trigger is operator misconfiguration, profile counts are bounded in
practice, and the same code shipped on 25.8 without incident. A visited-set /
depth-cap hardening is recorded as a follow-up in the dossier (§3.C) rather
than mixed into this port.
Adds stateless test `09078_allow_non_default_profile_escape`: toggling the
guard in-session (which drives the same enforcement branches as a `CONST`
constraint) it covers the blocked escape, allowed `default` inheritance,
auto-injection of `default`, cycle rejection, and the opt-out path. The
blocked-vs-allowed pair pins the assertion to `allow_non_default_profile`.
See `docs/aiven/patches/014-default-profile-escape.md`.
Co-authored-by: Kevin Michel <kevin.michel@aiven.io>
(cherry picked from commit a3f551f)
Port of `d3b5e9016f` from `v25.8.18.1-lts-aiven`. Adds a per-disk
`signature_delegation_url` S3 auth setting: when set, ClickHouse delegates AWS
SigV4 signature generation to an external HTTP service instead of signing
locally. It `POST`s the AWS canonical request as `{"canonicalRequest": "..."}`
and uses the `{"signature": "<hex>"}` it gets back, so a proxy can gate, audit,
or centrally control which S3 requests are signed.
Submodule-coupled: `contrib/aws` is redirected to Aiven's fork
`https://github.com/aiven/aws-sdk-cpp`, branch `aiven/clickhouse-v26.3.10.62` at
`c930cb8e8c51d4010dca68e01edf73ae1bb15af0` (upstream
`22f694afbdc7e9766894998c3745e23f004f8b86` plus the delegated-signer SDK commit
and an IPv6 host fix). That fork exposes `Aws::S3::S3ClientConfiguration` and the
`AWSAuthSignerProvider`-based `Aws::S3::S3Client` constructor this change needs.
Classified `still-needed-but-rewrite`:
- The new `AWSAuthV4DelegatedSigner` subclasses `Aws::Client::AWSAuthV4Signer`
and overrides `GenerateSignature` to POST the canonical request and parse the
returned signature; on any error it returns "" (fail closed).
- `S3::Client` switches to the `AWSAuthSignerProvider`-based `Aws::S3::S3Client`
constructor via `createSignerProvider`, dropping the `sign_payloads` ctor
parameter/member. The endpoint provider is passed as `nullptr` (the SDK
defaults a fresh `S3EndpointProvider`), because the 26.3 fork's SDK does not
expose a public `Aws::S3::S3Client::ALLOCATION_TAG`.
- `PocoHTTPClientConfiguration` is reparented to `Aws::S3::S3ClientConfiguration`
and gains a `signature_delegation_url` member. This overlaps patch 012
(`dfff5e80905`), whose `ca_path`/`ca_context` and consolidated
`request_throttler` on the same struct are preserved.
- The patch's `diskSettings.cpp` hunk is retargeted to its 26.3 location under
`src/Disks/DiskObjectStorage/ObjectStorages/S3/`.
Build note: this change alters a struct layout and a function signature in
widely-included headers, and this `build/` has no ninja header-dep tracking
(`#deps 0`), so the transitive include closure was force-recompiled before the
final relink to avoid a false-green incremental build (see
`docs/aiven/runbooks/build-and-test.md` §7).
Adds integration test `test_aiven_s3_signature_delegation`: a positive case (a
disk whose `signature_delegation_url` points at an in-container SigV4 signing
proxy can `CREATE`/`INSERT`/`SELECT`, and the proxy records the signing request)
and a negative case (a proxy endpoint that returns a wrong signature makes the
write fail with a signature/authorization error). MinIO runs over plain HTTP; no
secrets are committed. See docs/aiven/patches/015-s3-signature-delegation.md.
Co-authored-by: Kevin Michel <kevin.michel@aiven.io>
(cherry picked from commit aebade4)
The process is accessed over HTTP and the URL to access it can be
configured with the `signature_delegation_url` parameter in the Azure
disk configuration. ClickHouse makes a POST request with a JSON body
`{"stringToSign": "..."}` and expects a 200 JSON response
`{"signature": "..."}`. The string to sign matches Azure's signature
format and carries enough information (path, operation, ...) for the
proxy to decide whether the request is allowed and to account for its
cost. Only the final SharedKey HMAC step is delegated; the SDK still
builds the canonical Azure StringToSign locally.
Ported from v25.8.18.1-lts-aiven onto v26.3.10.62-lts-aiven-dev as
`still-needed-but-rewrite`:
- contrib/azure repointed to aiven/azure-sdk-for-cpp @ 98519bd324
(branch aiven/clickhouse-v26.3.10.62; upstream base 0f7a2013f7 plus
the Aiven delegated-signer SDK commit that makes
`SharedKeyPolicy::GetSignature` virtual and `m_credential` accessible).
- Reconciled with patch 013 (Azure Curl->Poco transport migration):
`account_name` and `signature_delegation_url` are placed in
`RequestSettings` next to 013's `ca_path` (no `#if` Curl block);
the `getRequestSettings`/`getClientOptions` reads were relocated
accordingly.
- Azure disk files moved to
src/Disks/DiskObjectStorage/ObjectStorages/AzureBlobStorage/.
- `ConnectionParams::delegated_signature` is initialized to `false`
(the source left it uninitialized).
- Dropped the `AzureObjectStorageConnectionInfo.cpp` wiring hunk: the
`ObjectStorageConnectionInfo` family was removed upstream between 25.8
and 26.3; the in-class `delegated_signature = false` default makes the
drop a provable no-op.
- Added integration test
tests/integration/test_aiven_azure_signature_delegation (positive:
delegation is used and the disk works; negative: a wrong delegated
signature is rejected by Azurite).
See docs/aiven/patches/016-azure-signature-delegation.md for the full
dossier (drift findings, reconciliation, C++ review, test design).
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2025-12-13.
Co-authored-by: Kevin Michel <kevin.michel@aiven.io>
(cherry picked from commit 38e54d3)
(cherry picked from commit 15a7175)
…ce (config-gated) Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2025-12-13. Co-authored-by: Joe Lynch <joe.lynch@aiven.io> (cherry picked from commit ae35b0c) Rewritten to gate the enforcement behind a new server setting `enforce_https_for_url_storage` (a `ServerSetting`, `Bool`, default `false`). Because it is a `ServerSetting` it has no `SET` path, so a tenant cannot override it in a session, a profile, a query `SETTINGS` clause, or a dictionary `<settings>` block — the enforcement can only be turned on in the server configuration. Default `false` keeps byte-for-byte upstream behavior out of the box; Aiven enables it via the managed config overlay. The original patch guarded only `StorageURL` and `HTTPDictionarySource`. Enforcement is extended here to `StorageURLCluster` (the `urlCluster` table function has its own constructor that does not route through `StorageURL`, so it would otherwise be a bypass) — a 4th file beyond the original three. When enabled, the `URL` table engine, the `url` and `urlCluster` table functions reject non-`https://` URLs with `BAD_ARGUMENTS`, and HTTP dictionary sources reject them with `UNSUPPORTED_METHOD`. (cherry picked from commit e3cd27f)
Lets a configured admin user (`user_with_indirect_database_creation`, e.g.
`avnadmin`) create and drop Replicated databases via plain SQL, with the
cluster parameters auto-filled and a narrowly-scoped, temporary privilege
elevation. Mechanism is unchanged from the source:
1. The configured user's `CREATE DATABASE d` is rewritten during execution to
the full `ON CLUSTER <cluster_database> ENGINE = Replicated(...)` form and
run on a cloned, privilege-elevated context (`createReplicatedDatabaseByClient`,
`Context::setGlobalContext`); the user is then granted a curated privilege set.
2. New `GRANT DEFAULT REPLICATED DATABASE PRIVILEGES` statement expands to that
fixed set (table/dictionary DML/DDL, `DROP DATABASE`, never `CREATE DATABASE`
/ `ACCESS MANAGEMENT` / `SYSTEM SHUTDOWN`), `WITH GRANT OPTION`.
3. Three config-only server settings (default ""): `reserved_replicated_database_prefixes`,
`user_with_indirect_database_creation`, `cluster_database`.
4. `DatabaseReplicated` stores the pre-expansion `shard_macros` for reuse.
5. `ON CLUSTER` enforced for non-admin `DROP`/`DETACH DATABASE`.
26.3 port differences from the source (all documented in the dossier):
- Drift-repair (no semantic change): on 26.3 internal queries are registered in
the process list (upstream dropped the `!internal` guard in `executeQuery`), so
the patch's nested internal queries collided on the inherited `query_id`
(`QUERY_WITH_SAME_ID_IS_ALREADY_RUNNING`). Each internal sub-query is now given
a fresh id via `setCurrentQueryId("")` at the two nesting sites
(`createReplicatedDatabaseByClient` and the expand-grant in `InterpreterGrantQuery`).
Restores 25.x behaviour on the drifted base. Without it the feature throws on
every use on 26.3.
- Security hardening (invisible on valid input): grantee and shard-macro are now
identifier/string-quoted (`backQuote` / `escapeString`) in the generated SQL.
- Default-off correctness: the non-admin `DROP` enforcement is a no-op when
`cluster_database` is empty, so an unconfigured server behaves exactly as
upstream (the source threw `SETTING_CONSTRAINT_VIOLATION` even with the feature
off).
- The reserved-prefix Replicated-only scope and the `skip_distributed_checks`
behaviour are carried unchanged as intended.
Ships integration test `test_aiven_indirect_database_creation` (11 cases,
Keeper-backed): 11/11 pass, including the non-escalation hard gate
`test_f_non_escalation` (proves the cloned-context elevation does not leak into
the invoking session) and `test_h_grantee_injection`; no regression. byte_equivalent: false.
Note: patch 020 extends the `GRANT DEFAULT REPLICATED DATABASE PRIVILEGES` set
with `CHECK` (the 019->020 chain).
(cherry picked from commit 05d8148)
(cherry picked from commit 04f7356)
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2025-12-13. Adds `CHECK` to the curated privilege set granted by `GRANT DEFAULT REPLICATED DATABASE PRIVILEGES` (introduced by 019). `CHECK TABLE` requires an explicit grant on 26.3 (`AccessType.h`: `M(CHECK, "", TABLE, ALL)`; not implied by `SELECT`/`SHOW`), so without it a holder of the default set cannot run `CHECK TABLE` on tables in its replicated database. Applied as a direct one-line edit rather than a cherry-pick: the source diff's surrounding `… TO escapeString(grantee) …` context line diverges from the 26.3 port of 019 (which hardened it to `backQuote(grantee)`); the inserted `"CHECK, "` line is identical to the source's intent. Tail of the `019->020` chain. Validated by extending 019's integration case C to assert `CHECK` in the granted set; full `test_aiven_indirect_database_creation` module 11/11 pass, non-escalation hard gate `test_f` unaffected. byte_equivalent: false. Co-authored-by: Aliaksei Khatskevich <alex.khatskevich@aiven.io> (cherry picked from commit c5b03b2) (cherry picked from commit 426848a)
… MySQL connections
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2025-12-15.
Co-authored-by: Joe Lynch <joe.lynch@aiven.io>
Adds SSL/TLS configuration for outbound PostgreSQL and MySQL connections,
together with a certificate-validation fix in the MariaDB connector.
1. MariaDB Connector/C security fix:
- Redirects `contrib/mariadb-connector-c` to the `aiven/mariadb-connector-c`
fork, which fixes the `X509_check_host` call to pass the hostname length.
Without the length, the check can be bypassed, allowing a certificate
whose name does not match the host to validate.
2. PostgreSQL SSL configuration:
- Adds the `SSLMode` enum (`DISABLE`, `ALLOW`, `PREFER`, `REQUIRE`,
`VERIFY_CA`, `VERIFY_FULL`).
- Adds two user-overridable `Settings` (session-level, not `ServerSettings`):
`postgresql_connection_pool_ssl_mode` (default `PREFER`) and
`postgresql_connection_pool_ssl_root_cert` (default empty).
- Threads the SSL mode and CA path through `PoolWithFailover` and
`formatConnectionString` (which now emits `sslmode`/`sslrootcert`), across
`DatabasePostgreSQL`, `DatabaseMaterializedPostgreSQL`, `StoragePostgreSQL`,
`StorageMaterializedPostgreSQL`, `TableFunctionPostgreSQL`, and
`PostgreSQLDictionarySource`.
3. MySQL SSL configuration:
- Adds the `MySQLSSLMode` enum (`DISABLE`, `PREFER`, `VERIFY_FULL`).
- Threads the SSL mode through `mysqlxx`'s `Connection`, `Pool`, and
`PoolWithFailover`.
- Adds `ssl_mode`/`ssl_root_cert` to `StorageMySQL::Configuration` and the
MySQL dictionary named-collection keys, wired through `MySQLHelpers` and
`StorageMySQL`.
The default `PREFER` mode preserves backward compatibility: it attempts SSL and
falls back gracefully when the server does not offer it.
(cherry picked from commit 934b35c)
26.3 port notes:
- Faithful carry: the SSL knobs remain session-level `Settings` exactly as in
the source (they govern outbound connections to the user's own external
PostgreSQL/MySQL); they are not promoted to `ServerSettings`. The original
commit message called them "server settings"; the implementation uses
`DECLARE` in `Settings.cpp`, i.e. user-overridable `Settings`.
- Submodule fork-redirect adapted from the source's 25.8 values to 26.3:
`.gitmodules` `url = https://github.com/aiven/mariadb-connector-c`,
`branch = aiven/clickhouse-v26.3.10.62`, gitlink
`2914d3fbce4f82b0f0d66034eb7afd1dd3dc5c70` ("fix call for `X509_check_host`").
- Kept the 26.3 `MySQLDictionarySource` non-named-collection branch
(replica/host-filter + `PoolFactory`); the source's deletion-to-throw was not
replayed. `ssl_mode` is threaded only through the named-collection path, and
`"ssl_mode"` is added to `dictionary_allowed_keys`.
- `M(CLASS_NAME, SSLMode)` inserted in alphabetical order in `Settings.h`.
- Adds integration test `tests/integration/test_aiven_external_db_ssl/`
(decoupled authoring per runbook §7.3): post-patch 10/10 pass; pre-patch the
five ClickHouse-side cases fail because the SSL settings/keys are
unrecognized (the evidence-of-causation pair).
(cherry picked from commit a63036c)
Redirects from `https` to `http` might create attack opportunities: a trusted HTTPS endpoint could bounce ClickHouse to an arbitrary internal plain-HTTP resource (a secure-transport downgrade / SSRF vector). Both the URL read path and the S3 client now refuse to follow such a redirect, throwing `UNACCEPTABLE_URL`. - `ReadWriteBufferFromHTTP::callWithRedirects` (url() storage / table function / URLCluster / HTTP dictionary): reject the downgrade before advancing to the redirect target. - `PocoHTTPClient::makeRequestInternalImpl`: reject an `https`->`http` `HTTP_TEMPORARY_REDIRECT` after the remote-host-filter check. The guard keys off the original request scheme, so it blocks any downgrade away from the secure origin, not merely a single hop. (cherry picked from commit 6fe07b9) 26.3 port notes: - Clean cherry-pick (line-number offsets only; no manual conflict resolution). `UNACCEPTABLE_URL` already exists (error code 491). - Faithful unconditional carry, exactly as the source: the prohibition is not gated behind a server setting. The blast radius is narrow (only a genuine https->http downgrade during a redirect; plain-http URLs and same-scheme redirects are unaffected) and security-positive. - Adds integration test `tests/integration/test_aiven_https_to_http_redirect/` (not present in the source). Helper HTTPS/HTTP redirect servers run as localhost subprocesses inside the node container; the client SSL config uses `verificationMode=none` so the handshake to the self-signed origin reaches the redirect-follow code. Evidence pair: post-patch 2 passed; pre-patch the rejection case fails because unpatched ClickHouse follows the downgrade and the SELECT returns data (the vulnerability), while the http->http control passes in both. (cherry picked from commit 69eb41d)
Port of the Aiven 25.8 patch "Fix IPv6 Azure object storage host" (1abdc71, Tilman Moeller, co-authored by Kevin Michel) to 26.3. The Azure SDK does not fully support IPv6 in hostnames because the escaping brackets are not parsed and removed at the right time, and `validateStorageAccountUrl` rejected IPv6 hosts because they do not look like DNS name segments. This broadens the URL-validation regex in `validateStorageAccountUrl` to accept a bracketed IPv6 literal (e.g. `[2001:db8::1]`) and an optional port, while still rejecting non-URL input (anchored `FullMatch`). 26.3 port notes: - The source commit had two legs. The `contrib/azure` gitlink bump is already applied: it was folded into `patch-port(016)` (azure HEAD `98519bd3` "Improve IPv6 support"), because the 26.3 `aiven/azure-sdk-for-cpp` branch stacks the Aiven changes linearly and 016 already pinned the gitlink to the branch HEAD. Only the ClickHouse-side regex leg is carried here. - The file moved on 26.3 from `src/Disks/ObjectStorages/...` to `src/Disks/DiskObjectStorage/ObjectStorages/...`, so the change is a 1-line direct edit at the new path rather than a cherry-pick. Adds a stateless test `9024_ipv6_azure_storage_account_url` that exercises the validation through a dynamic Azure disk and asserts a bracketed IPv6 host is accepted while a non-URL string is still rejected. Verified with a worktree-flip evidence pair (post-patch accepts IPv6, pre-patch rejects it). Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit 53b8335)
Port of the Aiven 25.8 patch "Add support for Azure object storage path prefix" (fdb7a14, Tilman Moeller, co-authored by Kevin Michel) to 26.3. Azure storage does not support a prefix before the stored object key, which makes shared containers unusable when one container is partitioned across projects / backup sites. This adds a `storage_prefix` disk-config option that works with the `storage_account_url` or `connection_string` configuration methods; the prefix is prepended to all blob operations, so multiple ClickHouse instances or projects can share one Azure container under disjoint path prefixes. Previously a prefix could only be specified by embedding it in the `endpoint` URL path; `storage_prefix` makes it explicit and readable. The option is opt-in (the new config read only fires when `storage_prefix` is present), so existing disk configurations are unaffected. 26.3 port notes: - The file moved on 26.3 from `src/Disks/ObjectStorages/...` to `src/Disks/DiskObjectStorage/ObjectStorages/...`, so the 3-line change is a direct edit at the new path rather than a cherry-pick. Adds an Azurite integration test `test_aiven_azure_storage_prefix` that writes real parts through a dynamic Azure disk and uses the Azure SDK client to assert the blob keys land under the configured prefix (with a control that asserts keys stay at the container root without the option). Verified with a worktree-flip evidence pair (post-patch both tests pass; pre-patch the prefix-applied test fails because the option is a no-op). Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit 9ad04ea)
Port of the Aiven 25.8 patch "Add Backup disk type" (3139011, Tilman Moeller, co-authored by Kevin Michel) to 26.3. Adds a `backup` disk type that wraps an object-storage disk. Instead of physically deleting objects it writes a local deletion-marker file per object at `<base>/<escapeForFileName(remote_path)>`; an external Aiven backup/GC system then physically deletes only the objects that no backup references. `exists` / `listObjects` / `iterate` filter out soft-deleted objects so the table view stays consistent. This decouples ClickHouse's object lifecycle from physical deletion, so backups can reference object versions that ClickHouse considers deleted. Classified `still-needed-but-rewrite`: 26.3 has no equivalent, and a verbatim cherry-pick is impossible because 26.3 (a) relocated the object-storage subtree, (b) moved disk layering from the object-storage level to the disk level, and (c) changed the `IObjectStorage` interface. 26.3 port notes: - Subtree relocation: `src/Disks/ObjectStorages/...` moved under `src/Disks/DiskObjectStorage/...`, so the decorator and register file live at the new path. - Disk-level layering: in 26.3 a layer is a named `DiskObjectStorage` linked to its inner disk via `wrapped_disk`, so `wrapWithBackup` is `const` and returns a new disk, and the existing disk-chain walk in `getCacheLayersNames` already enumerates backup layers by disk name. `MergeTreeData` only needs its layer-enumeration gate flipped from `supportsCache` to `supportsLayers`. The 25.8 object-storage-level plumbing (`getCacheLayersNames` -> `getLayersNames` rename, `IObjectStorage::getLayerName` / `getWrappedObjectStorage`) is obsolete and not carried. - Interface reconciliation: `BackupObjectStorage` is rebuilt against the 26.3 `IObjectStorage` pure-virtual set (`getObjectMetadata(path, with_tags)`, 3-arg `readObject`, `createKeyGenerator`, a filtering `iterate`, and forwarding of the S3 / Azure client accessors), modeled on the sibling `CachedObjectStorage`. - Scope (single-location only): 26.3 generalized `DiskObjectStorage` to back a disk by multiple object storages keyed by location (the `Replication/` subsystem). The backup layer soft-deletes only at the local location, so it would be unsound on a multi-location disk; `wrapWithBackup` rejects that case with `BAD_ARGUMENTS`. Ordinary tiered-storage disks are the single-location case and are unaffected. This commit also fixes a correctness defect introduced by 26.3's deferred-deletion model. On 26.3 a DROP / merge / mutation / TTL-delete enqueues blobs into a single removal queue owned by `metadata_storage`, drained later by a per-disk `BlobKillerThread`. The backup disk and its wrapped inner disk share that one queue, so the inner disk's killer would physically unlink blobs and defeat the soft-delete markers. The fix makes the backup disk's killer the sole drainer: `wrapWithBackup` detaches the inner killer from the chain and disables it (`BlobKillerThread` `detachWrapped` / `disable`). Adds an integration test `test_aiven_backup_disk` that asserts both the deletion markers and physical survival of the soft-deleted blobs across all normal removal paths (DROP, merge, mutation, TTL-delete), with a fix-isolating evidence pair (post-fix passes; with the fix reverted each path fails on the physical-survival assertion). (cherry picked from commit 71b1697)
This commit changes client database creation logic: 1. wait till database created on all replicas 2. forward database creation errors to the caller Original author: Aliaksei Khatskevich <alex.khatskevich@aiven.io>, 2026-03-26. (cherry picked from commit 92ab446) Body hunk re-anchored over the 26.3 `setCurrentQueryId("")` query-id-collision fix; the inserted lines are byte-identical to source (tier-2 decomposition empty). (cherry picked from commit 4a22c8e)
…eeper (rather than ClickHouse Keeper)
Refreshable materialized views previously required the MULTI_READ feature which
is only available in ClickHouse Keeper, not in standard Apache ZooKeeper. This
prevented users with ZooKeeper clusters from using refreshable materialized views.
The MULTI_READ feature provides atomic operations for reading multiple ZooKeeper
paths simultaneously. However, for refreshable materialized views, this atomicity
is not strictly necessary:
1. Znode creation: The `running` znode uses ephemeral mode, which means only one
replica can create it at a time, providing natural coordination. The other
persistent znodes (coordination path, replicas directory, paused znode) can be
created independently without atomicity concerns.
2. Znode reading: The only multi-read operation reads three paths:
`coordination.path`, `coordination.path + "/running"`, and
`coordination.path + "/paused"`. Minor inconsistencies in these reads are
acceptable:
- `running` znode: Ephemeral, checked separately anyway
- `paused` znode: Timing issues are acceptable (may miss early refresh or
refresh once after pause)
This patch removes the MULTI_READ requirement by:
- Replacing atomic `multi(ops)` with async `asyncTryCreateNoThrow()` calls
- Removing MULTI_READ checks in constructor and readZnodesIfNeeded
- Using the same async pattern already used elsewhere in ClickHouse
(e.g., StorageReplicatedMergeTree)
The asyncTryCreateNoThrow() method works with both ZooKeeper and ClickHouse
Keeper, enabling refreshable materialized views to work with either coordination
service. Each create operation is handled independently, with ZNODEEXISTS errors
gracefully handled (idempotent operations).
Changes:
- Commented out unused `attach` parameter in RefreshTask constructor
- Removed MULTI_READ feature check in constructor (lines 119-121)
- Replaced `multi(ops)` with `asyncTryCreateNoThrow()` futures pattern
- Removed MULTI_READ feature check in readZnodesIfNeeded (lines 945-946)
- Added error handling for individual async create operations
This enables refreshable materialized views to work with standard ZooKeeper
clusters without requiring migration to ClickHouse Keeper, providing better
deployment flexibility.
26.3.26.3 replay adaptation: keep upstream unconditional watches from ClickHouse#108234;
drop MULTI_READ throw in readZnodesIfNeeded so RMV works on Apache ZooKeeper
(tryGet falls back to per-path reads). Do not restore watch_active flags.
Co-authored-by: Aliaksei Khatskevich <alex.khatskevich@aiven.io>
Co-authored-by: Joe Lynch <joe.lynch@aiven.io>
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2026-01-08.
(cherry picked from commit 63f05c4)
(cherry picked from commit 52ef688)
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2025-12-07. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit d6e78ab) Augmented for the 26.3 uplift: the feature is gated behind a new default-off server setting `aiven_enable_replication_queue_size_limit` (clause (v) blast-radius resolution; see docs/aiven/proposals/2026-06-15-aiven-settings-naming-convention-and-queue-size-guard.md). queue_size_monitor and the four queue thresholds are carried verbatim. Drift fixes vs the 25.8 source: createTask 3-arg (StorageID), LoggerPtr idiom, and a Coordination::setCurrentComponent guard (26.3 enforce_component_tracking) in ReplicatedMergeTreeQueueSizeThread. (cherry picked from commit dd0d261)
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2026-01-06. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> (cherry picked from commit 7e08e44) Route the large initial-sync GET_PART/ATTACH_PART fetches (queue entries with an empty source_replica, enqueued when a fresh replica downloads pre-existing data) to a separate "early" background pool, so they no longer monopolize the normal fetch pool and starve the many small insert-driven GET_PART tasks. The early pool is bounded by a dedicated server setting; per-table routing is controlled by a MergeTree setting. Renamed both settings under the aiven_ naming convention for Aiven-introduced settings (docs/aiven/AGENTS.md): use_early_fetch_pool -> aiven_use_early_fetch_pool MergeTreeSetting, Bool, default true. Declared with DECLARE_WITH_ALIAS keeping the old name use_early_fetch_pool as a backward-compatible alias, so 25.8 tables that persisted SETTINGS use_early_fetch_pool = ... still ATTACH (no UNKNOWN_SETTING) and carry the value forward. background_early_fetches_pool_size -> aiven_background_early_fetches_pool_size ServerSetting, UInt64, default 8. Plain rename, no alias: server settings are read by known key, so a stale config key is silently ignored and the pool reverts to the default until the operator renames it (operator-owned config migration). Internal identifiers stay unprefixed (early_fetch_executor, getEarlyFetchesExecutor, scheduleEarlyFetchTask, the CurrentMetrics BackgroundEarlyFetchesPoolTask/Size, and the "EarlyFetch" executor label). Drift fixes vs the 25.8 source: use .empty() (HEAD idiom) instead of == "" in scheduleDataProcessingJob and canExecuteFetch; in canExecuteFetch wrap only the pool-saturation check in the early/normal branch, leaving HEAD's throttler and broken-part blocks intact. The 26.3 OrdinaryBackgroundExecutor constructor takes a ThreadName enum (not a string literal), so a MERGETREE_EARLY_FETCH value mapping to "EarlyFetch" is added to setThreadName.h. Default-on (aiven_use_early_fetch_pool = true) is test-neutral: the split only changes which background pool runs a fetch, not results. Registered in SettingsChangesHistory.cpp so the new-settings golden list (02995) recognizes it. Ships integration test test_aiven_early_fetch_pool (two-table differential: aiven_use_early_fetch_pool = 1 routes initial-sync fetches to the early pool, sized 1 via aiven_background_early_fetches_pool_size, so surplus entries are postponed with "early fetches already executing"; = 0 falls back to the normal pool and never shows that reason). (cherry picked from commit 9c85cd2)
Original authors: Tilman Moeller <tilman.moeller@aiven.io> (062, 2026-01-29), Aliaksei Khatskevich <alex.khatskevich@aiven.io> (063 2026-02-02, 064 2026-02-05). (cherry picked from commit 50a42b7) (cherry picked from commit cc71839) (cherry picked from commit c95ef0c) Squash of three strict-order Aiven patches: 062 ("Prohibit .tmp table creation", 50a42b7), 063 ("Use .tmp for all fake temporal tables", cc71839) and 064 (".tmp create or replace" internal exemption, c95ef0c). They are landed as one commit because the intermediate state is runtime-broken: 062+063 without 064 throws on every `CREATE OR REPLACE`. Purpose. Reserve the `.tmp*` table-name namespace for engine-internal use so user tables cannot collide with the "fake" temporary tables that refreshable materialized views and `CREATE OR REPLACE` create, and so those temporaries are skippable by name in backup. As 062's body says: temporal tables for refreshable materialized views are deliberately not marked temporary (they must replicate), so the usual temporary-table exclusion does not apply and a name-prefix convention is the simple, consistent handle. - 062 rejects non-internal `CREATE` of a `.tmp*` table (`InterpreterCreateQuery::doCreateTable`) and non-internal `RENAME ... TO` a `.tmp*` name in a `Replicated` database (`InterpreterRenameQuery::executeToTables`, keyed on `isInternalQuery`). It adds a `setInternal`/`internal` member to `InterpreterRenameQuery` and marks the `DatabaseReplicated::recoverLostReplica` create and the `StorageMaterializedView::exchangeTargetTable` rename internal. - 063 renames the `CREATE OR REPLACE` temp prefix `_tmp_replace_` -> `.tmp_replace_` (literal + regex in `TemporaryReplaceTableName.cpp`) so it falls under the reserved `.tmp*` namespace (the refresh temp `.tmp.inner_id.*` already does). - 064 marks the inner `CREATE OR REPLACE` create internal so the 062 guard exempts the engine's own `.tmp_replace_*` create. Gated (parent policy call, clause-(v) blast-radius resolution). Only 062's two throws are wrapped in a new default-`false` server setting `aiven_prohibit_tmp_table_creation` (Bool), declared in `src/Core/ServerSettings.cpp` next to patch 008's `aiven_enable_replication_queue_size_limit` (generic loading; second use of the AGENTS-8 `aiven_` naming convention). With the gate OFF (the default) the build behaves byte-identically to stock 26.3: users may still create/rename `.tmp*` tables. Everything else in the chain ships unconditional/ungated: the `setInternal`/`internal` member, the two internal-flips, all of 063, all of 064 -- these are engine-internal and a no-op for users when the gate is off. Aiven opts in by setting `aiven_prohibit_tmp_table_creation=true` in managed server config (server-level, cannot be overridden per-session). Reconciliation with the 26.3 base: - 062's `#include <Common/StringUtils.h>` was re-anchored: HEAD inserted `#include <Common/NamedCollections/NamedCollectionsFactory.h>` between the original `AccessRightsElement.h -> typeid_cast.h` anchor, so `StringUtils.h` is added after `typeid_cast.h`. `#include <Core/ServerSettings.h>` is added for the gate (062 was ungated upstream and had no such dependency). - `StorageMaterializedView::exchangeTargetTable` had already been refactored by the staged 066+078 squash into a named `auto interpreter = InterpreterRenameQuery(...)` (066 dropped its own `setInternal` call there because the base `InterpreterRenameQuery` no longer exposed it). 062 re-adds the member, so `interpreter.setInternal(true)` is re-inserted between the existing lines instead of replacing a bare `.execute()`. Tests. Because `aiven_prohibit_tmp_table_creation` is a server setting (cannot be set per-session), the ON-state proof is an integration test `tests/integration/test_aiven_prohibit_tmp_table` (config drop-in enabling the guard on `node_on`, default-off `node_off`), 4/4 passing: - guard ON: `CREATE TABLE ".tmpfoo"` throws BAD_ARGUMENTS / "reserved for internal use"; guard OFF control: the identical CREATE succeeds. - guard ON: `RENAME TABLE ... TO ".tmpbar"` in a Replicated DB throws BAD_ARGUMENTS; guard OFF control: succeeds. (Each node uses its own Replicated DB / ZooKeeper path so they do not replicate to each other.) - guard ON: `CREATE OR REPLACE TABLE` succeeds twice (064 exemption). - guard ON: a refreshable MV refresh lands its rows (internal `.tmp.inner_id.*` create exempt). The asserts check BOTH the BAD_ARGUMENTS code AND the Aiven-specific message substring, using a `.tmp` name upstream would accept, distinguishing the Aiven gate from any unrelated BAD_ARGUMENTS. OFF-state neutrality is also covered by stateless `09081_prohibit_tmp_table_creation_off_neutral` (default server accepts a `.tmp`-prefixed table). The unconditional changes (063's temp rename, the internal flips) were checked neutral against the CREATE OR REPLACE + refreshable-MV stateless suites; two heavy `.sh` refreshable-MV tests (02932/03258) and 01157 that do not pass in the bare smoke environment were proven environmental (not regressions) by an A/B against a baseline binary built with the `src/` changes stashed -- they time out / fail identically on the unpatched build. Co-authored-by: Tilman Moeller <tilman.moeller@aiven.io> Co-authored-by: Aliaksei Khatskevich <alex.khatskevich@aiven.io> (cherry picked from commit 92210bf)
…ated databases (Aiven-gated) Port of Aiven patch 004 (226ed6c) onto 26.3, wrapped in a new default-off server setting and hardened with three gap fixes. When `aiven_replace_mergetree_with_replicated` is enabled, a table created in a `Replicated` database with a non-replicated `*MergeTree` engine is automatically rewritten to its `Replicated*` twin on the replica-execution (`SECONDARY_QUERY`) path, so every table in a Replicated database is self-replicating even if the caller wrote a plain `MergeTree` engine. The setting is a server-level switch (not session-overridable) and is disabled by default, so a stock build behaves exactly like upstream. Changes vs. the original 25.8 patch: - Gate the whole rewrite behind the default-off server setting `aiven_replace_mergetree_with_replicated` (clause (v) compliance). - Exclude `ATTACH`: never re-engine a table that adopts existing on-disk data (the original patch had no `!attach` guard). - Missing-twin safety: only rewrite to a `Replicated*` name that is actually registered in `StorageFactory`; otherwise leave the engine name unchanged. - Stored-DDL consistency is preserved: the rewrite mutates the same `ASTCreateQuery` that is persisted by `database->createTable`, so all replicas converge to identical `Replicated*` metadata (asserted in tests). The non-`MergeTree` disk-engine gap is intentionally handled by config, not code: production additionally sets the upstream `database_replicated_allow_only_replicated_engine = 1` to reject any non-replicated disk engine the rewrite does not convert. Adds integration test `test_aiven_replace_mergetree_with_replicated` and a `REPL-6` ledger entry in docs/aiven/uplifts/26.3/major-upstream-changes.md. Co-authored-by: Kevin Michel <kevin.michel@aiven.io> Co-authored-by: Joe Lynch <joe.lynch@aiven.io> Co-authored-by: Dmitry Potepalov <dmitry.potepalov@aiven.io> (cherry picked from commit f59f3d7)
Original author: Joe Lynch <joe.lynch@aiven.io>, 2026-06-09. Extends the protected-entity mechanism (the per-entity `Protected` flag, the `PROTECTED_ACCESS_MANAGEMENT` privilege, and the `PROTECTED` keyword, all landed for users by `patch-port(022)`) to ROLE entities. A role marked `PROTECTED` can only be created, altered, renamed, replaced, dropped, moved between storages, or have its grants changed by a principal holding `PROTECTED_ACCESS_MANAGEMENT`; the check runs on the initiator before any `ON CLUSTER` dispatch so it cannot be laundered through `DDLWorker`. This lets service-managed roles (e.g. `aiven_admin_role`) exist as real SQL roles that ordinary cluster users cannot remove or tamper with. Motivation: stock 26.3 has no protected-role grammar, so the harness statement `CREATE ROLE aiven_admin_role ... PROTECTED` fails on 26.3 while succeeding on 25.x. This is the only Aiven patch added to v25.8.24.21-lts-aiven after our 26.3 fork point f4552b1 (merged 2026-06-17, PR #29). Carried unconditional (additive keyword + privilege check; no new server setting), matching the 022 precedent. byte_equivalent: false. The cherry-pick was reshaped in three resolved conflicts: (a) Role.h / Role.cpp merge HEAD's `fetched_from_remote_at_ms` member additively with the new `protected_flag`; (b) in InterpreterMoveAccessEntityQuery.cpp the protected-ROLE move check is HOISTED above the `ON CLUSTER` dispatch (mirroring our 26.3 022 USER hoist) rather than the source's post-dispatch placement, which would otherwise reopen a `MOVE ROLE ... ON CLUSTER` bypass. See docs/aiven/patches/079-protected-roles.md sections 3 and 6. Tests: stateless 9079_protected_roles (local + ON CLUSTER denial matrix, pre/post causation pair) and integration test_aiven_protected_roles (cross-node ZooKeeper flag replication + the hoisted MOVE ROLE denial, local and ON CLUSTER; 4/4). (cherry picked from commit 827d8eb) (cherry picked from commit 50a8b00)
…list collision Root-cause fix in ProcessList::insert: for internal queries only, regenerate the query_id while it is already registered, instead of throwing QUERY_WITH_SAME_ID_IS_ALREADY_RUNNING (Code 216). 26.3 registers internal queries in the process list (the !internal insert gate was dropped) but the registration maps are keyed by query_id and require global uniqueness, so an internal sub-query inheriting a live parent id self-collides. Skipping the guard is unsafe: a duplicate key desyncs the maps and ~ProcessListEntry calls std::terminate. Regenerating keeps the invariant; the user-facing guard is unchanged. Also fixes the latent per-site bug in createReplicatedDatabaseByClient that the downstream trace exposed: the inner CREATE's BlockIO (process_list_entries) keeps its entry alive for the whole scope, so the inner GRANT reusing new_context's id self-collides on the generated UUID. Re-stamp a fresh id before the GRANT. (cherry picked from commit 3d5e613)
Original author: Tilman Moeller <tilman.moeller@aiven.io>, 2025-12-18. Re-instates the behavior of 8ed6167 (dropped in the 26.3 uplift as obsoleted-by-upstream, which was wrong for the non-readonly CREATE path). 26.3 actively probes Azure (GetProperties / CreateBlobContainer) at CREATE TABLE ... ENGINE = AzureBlobStorage(...), stalling against unreachable endpoints. Per clause (v) the behavior is now gated behind a new default-off server setting aiven_skip_azure_container_creation instead of the original unconditional line; off by default, behavior-identical to upstream. (cherry picked from commit 8ed6167) (cherry picked from commit a783198)
Re-resolve the squashed 066+078 ("Fix MV refresh in sharded environment" +
"Fix MV refresh task race condition") against the v26.3.15.4-lts refresh
loop, which upstream had rewritten via the ClickHouse#104051 keeper-connection-loss
backport and SYSTEM PAUSE VIEW.
Kept upstream's two-task doScheduling/executeRefresh loop (preserving
Keeper blip" duplicate-refresh avoidance) and re-introduced 066's
shard-leader/global-leader coordination and the deferred UUID-keyed
EXCHANGE inside it, preserving the five .62 port-time defect fixes and
PAUSE VIEW. StorageMaterializedView::exchangeTargetTable re-adds 066's
block_io/CompletedPipelineExecutor "wait for all replicas" block while
keeping committed 062's setInternal(true).
Data-loss-relevant reconciliation fix: 066's coordination znode is shared
across shards ({uuid}-keyed, not {shard}), but ClickHouse#104051 keys the running
owner on last_attempt_replica == replica_name, and the stock
default_replica_name ({replica}) collides between shards, so a peer shard
could think it owned another shard's refresh and clobber it. Qualify
replica_name with the shard name (<shard>/<replica>) in the RefreshTask
constructor: globally unique across the shared subtree, stable per process,
used only as znode data or an equality operand, never as a path segment.
Deliberate divergence from the .62 port: the split-out deferred
exchangeTargetTableAfterRefresh is version-guarded on the root znode
(mirroring ClickHouse#104051's CREATE-side check), stronger than the .62 unguarded
exchange.
Build green; 6/6 stateless refreshable-MV neutrality tests pass and the
2-shard test_aiven_mv_refresh_sharded causation test passes end-to-end
(per-shard retention shard1.tgt={1,2,3}, shard2.tgt={10,20,30}). The
single-node vs coordinated error-reporting asymmetry (dossier section 11)
is carried forward unchanged for maintainer decision.
(cherry picked from commit 7ace1a1)
…eReplicated Re-introduce Aiven patch 009 (source 110900c / aiven a2c312b), extending DatabaseReplicated::shouldReplicateQuery to return true for PartitionCommand::MOVE_PARTITION so the move is routed through the database DDL log. This reverses the earlier 26.3 DROP, which was reasoned only about MOVE ... TO TABLE (a leader-only DDL task, hence no data benefit). It did not weigh MOVE ... TO VOLUME/DISK, which is NOT leader-only (DDLWorker::taskShouldBeExecutedOnLeader excludes isMovePartitionToDiskOrVolumeAlter): without the patch a tiered-storage move runs only on the receiving replica and the replicas' storage tiers diverge; with it the move replays on every replica. That is the 25.8 -> 26.3 regression this restores. Verbatim semantic port; the only deviation is the repo style restyle (Allman braces, angle-bracket include, no redundant else). Note: the predicate matches all MOVE_PARTITION, so TO TABLE routing is re-enabled too -- the leader-only case carrying a DDL-queue head-of-line-stall caveat (tighten to move_destination_type in {DISK,VOLUME} to avoid it). Test: tests/integration/test_aiven_move_partition_to_volume_replicated -- a 2-replica single-shard DatabaseReplicated with a tiered storage policy (move_factor=0). Evidence-of-causation pair: with the patch it passes (node2's part lands on ext_disk); reverting the one block makes it fail at the node2 disk_name assertion ('ext_disk' != 'default'). Ref: a2c312b (cherry picked from commit 5870091)
…d_database_internal Re-fix Aiven patch 004 (source 226ed6c) after the v26.3.15.4-lts rebase silently turned it into a no-op. The rewrite was gated on query_kind == ClientInfo::QueryKind::SECONDARY_QUERY, but upstream decoupled the Replicated-database DDL-log execution path from query_kind and now signals it via ClientInfo::is_replicated_database_internal (set in DDLTask::makeQueryContext -> setQueryKindReplicatedDatabaseInternal). The old gate was therefore always false on the DDL-log path, so a plain *MergeTree engine created in a Replicated database was no longer rewritten to its Replicated* twin and replicas could diverge in metadata. Gate the rewrite on is_replicated_database_internal instead, matching upstream's own detection in InterpreterCreateQuery::assertOrSetUUID. Also refresh the StorageFactory.h contract comment and the aiven_replace_mergetree_with_replicated setting description, which still named the old SECONDARY_QUERY path. Test: tests/integration/test_aiven_replace_mergetree_with_replicated -- with the fix the suite goes 3/8 -> 8/8; reverting the one-line gate makes the conversion cases fail again (engine s (cherry picked from commit efeace9)
…_default_replication_path
Reverse the 26.3 drop of Aiven patch 058 (`9a2883592c`). The drop relied on
upstream `database_replicated_allow_replicated_engine_arguments`, but that guard
is session-overridable and only fires for tables inside a `Replicated` database,
whereas 058 enforces unconditionally and also covers standalone
`ReplicatedMergeTree` tables (the scope our downstream tests exercise).
Per the "stick to the original patch" decision, 058's `expand_special_macros`
helper and the two `BAD_ARGUMENTS` throws in the `expand_macro` lambda of
`extractZooKeeperPathAndReplicaNameFromEngineArgs` are reinstated verbatim and
wrapped in a new default-off server setting `aiven_enforce_default_replication_path`.
With the gate off the binary is byte-identical to upstream; Aiven's managed
config turns it on.
`expand_special_macros_only` expands only the `{database}` / `{table}` macros, so
the comparison leaves `{uuid}` / `{shard}` / `{replica}` intact and the
default-args creation path does not self-reject. Carried caveat, accepted for
parity with 25.8: the expanded comparison can reject `CREATE TABLE t1 AS t2`.
Test: `tests/integration/test_aiven_enforce_default_replication_path` (5 cases)
proves gate-on rejects a foreign ZooKeeper path / replica name, accepts the
managed default, and gate-off stays upstream-neutral.
(cherry picked from commit cb2e7b0)
…oKeeper The `.10.62 -> .15.4` intra-LTS rebase pulled in upstream backport ClickHouse#104051/ClickHouse#105589 ("Refreshable MV: avoid duplicate refresh on brief keeper connection loss"), which changed the ephemeral `.../running` znode create in `RefreshTask::updateCoordinationState` to `ignore_if_exists=true`. That flag serializes the create as the ClickHouse-Keeper-only `CreateIfNotExists` op (`OpNum` 502). Apache ZooKeeper does not implement that op and cannot even deserialize it inside a `multi` (`MultiOperationRecord.deserialize` -> "Invalid type of op"). ClickHouse classifies the response as a transport-level `Marshalling error` (hardware error) and finalizes the shared ZooKeeper session, so every `ReplicatedMergeTree` on the node observes `Connection loss` / `Session expired` and drops into readonly. The session reconnects, the refresh retries, and the cycle repeats -- a self-sustaining outage triggered by a single coordinated refreshable materialized view. Aiven runs production clusters on plain ZooKeeper, so this breaks them; upstream CI only runs ClickHouse Keeper, so the regression is invisible upstream. This is the second ZooKeeper-compatibility fix for refreshable MVs after the ported patch 050 (which removed the construction-time `MULTI_READ` requirement). Patch 050's "ZooKeeper-compatible: set/create/check only" audit was correct when written and was invalidated by this newer backport; N02 restores that invariant. Fix: gate the create on `zookeeper->isFeatureEnabled(CREATE_IF_NOT_EXISTS)`. On ClickHouse Keeper the flag is advertised and behavior is byte-identical to upstream. When it is not advertised (real ZooKeeper), emulate the op's "no-op if already present" semantics with a plain create guarded by an `exists` pre-check, keeping the batch a set/check/create that ZooKeeper accepts. The pre-check (rather than tolerating `ZNODEEXISTS` post-hoc) is required because `multi` is atomic: a plain create that hit `ZNODEEXISTS` would roll back the sibling set/check too, silently failing to update the coordination state. No new setting -- the path is selected automatically by the keeper's advertised feature flags, mirroring the rest of the ZooKeeper client (`createAncestors`, etc.). Test: `tests/integration/test_aiven_refreshable_mv_create_if_not_exists` (embedded per-node Keeper with `create_if_not_exists` forced off to simulate Apache ZooKeeper deterministically, same technique as patch 050's `multi_read` test; Keeper rejects the unsupported `multi` sub-op server-side via `KeeperContext::isOperationSupported`). Verified FAIL -> PASS: pre-patch differential `1 failed in 136s` (the replicated `INSERT` stalls into `TIMEOUT_EXCEEDED`); post-patch `2 passed` (differential plus a Keeper control proving the gating does not regress the ClickHouse-Keeper path). (cherry picked from commit d813b26)
Coordinated refreshable materialized views in sharded Replicated databases could let peer replicas observe a refresh directory before the global leader had reserved its own shard. This could leave a refresh missing data from a shard, fail SYSTEM WAIT VIEW, or retry after stale Keeper state. Elect the global leader first, then publish the refresh directory only after the leader has reserved its shard. (cherry picked from commit 8b7c055)
Consolidated documentation bucket for the v26.3.15.4 reslice: every patch dossier (docs/aiven/patches) and the 26.3 uplift record (docs/aiven/uplifts — inventory, execution-plan, work log, retrospectives, reports), plus the repo-root .gitignore additions. Separated from code so the patch commits stay cleanly cherry-pickable. (cherry picked from commit 662593c)
Add a Buildkite pipeline that builds amd_debug and amd_binary with praktika, reuses build artifacts for stateless and integration tests, and adds targeted Aiven stateless/integration lanes that skip cleanly when no matching tests exist. (cherry picked from commit 0474291)
…ebug abort and silent unprotect The protected-user/role query AST carried protection as a plain `bool protected_flag`, which cannot distinguish "NOT PROTECTED" from "not mentioned". That caused two defects: 1. Debug abort (CI-blocking). `ASTCreateUserQuery::formatImpl` emitted the token only for CREATE (`protected_flag && !alter`), so `ALTER USER x PROTECTED` formatted back to `ALTER USER x` -- an empty ALTER that does not parse. The `#ifndef NDEBUG` AST round-trip check in `executeQueryImpl` turned that into `LOGICAL_ERROR: Inconsistent AST formatting` and aborted the server. Reproduced by `09080_protected_user_management` (`ALTER USER ... PROTECTED`) on the amd_debug stateless lane (Buildkite build 42). Release compiles the check out, so this was debug/CI-only. 2. Silent unprotect (release). The interpreters wrote the entity flag unconditionally (`user.protected_flag = query.protected_flag`), so any unrelated `ALTER USER x SETTINGS ...` (parsed with the default `false`) cleared a user's protection. Fix: restore the 25.3 design. The query field becomes `std::optional<bool>` (unset / true / false). The formatter emits `PROTECTED` / `NOT PROTECTED` whenever the option is engaged, matching what the parser already accepts for both CREATE and ALTER; the interpreters touch the entity flag only when the option is engaged, preserving existing protection on unrelated alters. `SHOW CREATE` (assigns only `true`) and the on-disk / ZooKeeper entity definitions are unchanged, so there is no migration. Verified: `formatQuery` round-trips all six user/role protection forms; both `09079_protected_user_extra_statements` and `09080_protected_user_management` pass. (cherry picked from commit d43d33c)
Drop the exclusive-create requirement when writing the soft-delete removed marker so that removing an already-removed object is a no-op, matching the `removeObjectsIfExist`/`removeObjectIfExists` contract. Previously `FS::createFile` opened with `O_CREAT | O_EXCL` and threw `CANNOT_CREATE_FILE` (`EEXIST`) when a marker already existed, causing `SYSTEM RESTORE REPLICA` to fail on retries. Original author: Joe Lynch <joelynch112@gmail.com> (cherry picked from commit 968d8bd)
…eeper With the ZooKeeper-backed named-collections metadata storage each collection is a child znode under the storage root, and replicas detect changes by watching the root's children and its `cversion`. A `set` on a child znode (an in-place `ALTER NAMED COLLECTION ... SET/DELETE`) fires neither the parent children-watch nor bumps its `cversion` on ZooKeeper, so in-place edits were never observed by other replicas. Also track and bump the root node's data `version` on every in-place write (`bumpRootVersion` via `set(root_path, "")`) and watch it in `list`/`shouldUpdate`, so the change propagates in both directions. The root node is created empty and its data is never read for meaning, so the bump is a pure watch/version trigger. Original author: Joe Lynch <joelynch112@gmail.com> (cherry picked from commit dc7cf2a)
The early-fetch pool (patch 046) added scheduleEarlyFetchTask, routing initial-sync GET_PART/ATTACH_PART entries (empty source_replica) to a separate early_fetch_executor, but did not add the matching drain in BackgroundJobsAssignee::finish. That method removes a storage's pending and in-flight tasks from the moves/fetches/merge_mutate/common executors on per-storage shutdown (DROP/DETACH TABLE, DROP DATABASE, DatabaseReplicated recovery re-attach, replica removal), and blocks until the storage's active tasks finish — the invariant that lets the fetch lambda safely capture a raw `this`. Because the early executor was never drained, an in-flight initial-sync fetch could keep running against a destroyed StorageReplicatedMergeTree and dereference a freed MergeTreeData (`getDisks` on a NULL `this`) → SIGSEGV. Add the missing early_fetch_executor drain, matching the other executors. This defect was present in the 25.8 carry of the patch (7e08e44) and every 25.8 point release since (through v25.8.26.11-lts-aiven); the 25.3 carry (68fbdb5) already had the drain, so the line was lost when the patch was re-authored 25.3 -> 25.8. Our 26.3 port (9c85cd2) cherry-picked from 25.8 and inherited the omission. (cherry picked from commit 2b9804d)
This allows an atomic partial revoke: the grant and the carve-out are
applied in a single statement, avoiding race conditions and the
partial-failure window of a separate GRANT followed by REVOKE.
GRANT SELECT ON db.* EXCEPT SELECT ON db.secret TO user
SHOW GRANTS keeps emitting the canonical two-statement (GRANT + REVOKE)
form.
(cherry picked from commit 042b106)
…tial dependencies Backport of upstream ClickHouse PR ClickHouse#108388, applied fresh on 26.3 (no 25.8 Aiven source-index, hence patch-new). A `TimeSeries` table can reference external `DATA`, `TAGS` and `METRICS` target tables (`CREATE TABLE ts ENGINE = TimeSeries DATA d TAGS tg METRICS m`). Unlike a materialized view's external `TO` table, those targets were not recorded as referential dependencies by `DDLDependencyVisitor`, so: * `DatabaseReplicated::recoverLostReplica` placed an external target and its parent `TimeSeries` table at the same level of the `TablesDependencyGraph` and created them concurrently. On a 25.8 -> 26.3 upgrade this races the `StorageTimeSeries` constructor against a not-yet-created external target -- the ordering that surfaces a missing-target null dereference during replica recovery; and * with `check_referential_table_dependencies = 1` an external target could be dropped while a `TimeSeries` table still referenced it. Handle `ViewTarget::Kind::Data`, `Kind::Tags` and `Kind::Metrics` in `DDLDependencyVisitor::visitCreateQuery` the same way as an external `TO` target: external targets (non-empty table name) become dependencies of the `TimeSeries` table; inner targets do not. The only adaptation versus upstream is the enum name -- upstream `ViewTarget::Kind::Samples` is `Kind::Data` on 26.3. Ships stateless test `04410_time_series_referential_dependencies` (renamed from upstream `04409` to avoid a number collision on this branch): each external target is protected from `DROP` while the `TimeSeries` table exists and can be dropped once it is gone. This fixes the recovery-ordering half only; the unguarded `tryGetTable(...)->getInMemoryMetadataPtr()` null dereference in the `StorageTimeSeries` constructor is a separate, following commit (N04). Upstream PR: ClickHouse#108388 Changelog: https://clickhouse.com/docs/resources/changelogs/oss/2026 (cherry picked from commit f05c455)
…t on ATTACH The `StorageTimeSeries` constructor validated that an external `METRICS` target table has no `LowCardinality` columns by doing `DatabaseCatalog::tryGetTable(target.table_id, ...)->getInMemoryMetadataPtr()` with no null check and on every load mode. `tryGetTable` returns null when the target is not present, which happens on the `ATTACH` path: during `DatabaseReplicated::recoverLostReplica` on a 25.8 -> 26.3 upgrade the external target may not be created yet, and on any restart it may have been dropped (referential-dependency enforcement is off by default). The unchecked dereference then crashes the server with `SIGSEGV` while the loader re-attaches the table -- during recovery this turns into a crash-loop that keeps the replica from rejoining. Gate the check on `mode < LoadingStrictnessLevel::ATTACH` and null-check the lookup. This is a creation-time constraint: it is enforced when the table is created (where the target is guaranteed present) and skipped on attach/recovery/restore, exactly like the column validation a few lines above. The `TimeSeries` engine owns no external data and never drops or truncates its external targets, so allowing the attach to complete cannot lose data; a still-missing target instead surfaces as a normal query-time error. Complements N03 (which fixes the recovery *ordering* and adds drop-protection): N04 is the memory-safety guarantee that the constructor never dereferences a null target pointer, independent of ordering or the `check_referential_table_dependencies` setting. Reproduced by integration test `test_aiven_time_series_recovery`: it drops the external metrics target and restarts, so the startup loader re-attaches the `TimeSeries` table with the target absent -- SIGSEGV on the unguarded binary, clean startup with this fix. Changelog: https://clickhouse.com/docs/resources/changelogs/oss/2026 (cherry picked from commit 56c7c87)
Remove the optional debug / observability web UIs (/binary, /merges,
/jemalloc, /clickstack) from the default HTTP handler set to shrink the
unauthenticated HTTP surface of the managed service. None is required
for server operation:
- /jemalloc is a static UI page, not the jemalloc profiler (SYSTEM
JEMALLOC / settings / system.jemalloc* are unaffected);
- /clickstack, /binary and /merges are static SPA / debug UIs.
The corresponding `http_handlers` rule types are kept, so any endpoint
can be re-enabled explicitly per config. This mirrors patch 040
(disable /replicas_status). /, /ping, /play, /dashboard and /js/ are
retained (/dashboard loads its assets from /js/).
ACME is deliberately left byte-identical to upstream (config-only
posture). ClickHouse's built-in ACME client provides automatic TLS
certificate issuance/renewal via the HTTP-01 challenge, served at
/.well-known/acme-challenge/... Both the client and that challenge
handler are already gated on the <acme> config section: Client::initialize
early-returns when <acme> is absent, and the handler is only attached
inside `if (server.config().has("acme"))`. Aiven never emits <acme> --
TLS certificates are provisioned externally (Let's Encrypt handled by the
control plane, PEMs written to disk) and wired into ClickHouse via
<openSSL> (privateKeyFile/certificateFile), a path completely separate
from ClickHouse's ACME client / CertificateReloader. Consequently the
ACME subsystem is dead code in this deployment: removing its handler
would be a runtime no-op while <acme> is unset, would only break HTTP-01
issuance if <acme> were ever enabled, and would add permanent fork
divergence (recurring merge cost) for no security gain. So we harden by
not configuring <acme>, not by forking ACME code.
Landing page (programs/server/index.html) drops the now-dead links.
Tests: remove upstream 02952_binary / 03256_merges / 03916_clickstack
(they assert content no longer served) and add
09082_aiven_disabled_web_ui_endpoints asserting 404 for the removed
paths and 200 for the retained ones.
(cherry picked from commit 7cee0c6)
…ft-deleted blobs Patch 026 makes a `backup` disk soft-delete: instead of unlinking a blob it writes a deletion marker and leaves physical removal to external GC. On 26.3 blob deletion became deferred - each disk's `metadata_storage` owns an in-memory removal queue that the disk's own `BlobKillerThread` drains through its RAW object storage - so in the production stack `backup -> cache -> object_storage` the wrapped disks' killers physically unlink blobs the backup layer only marked. `wrapWithBackup` already disabled the directly-wrapped disk's killer, but three gaps remained. `SYSTEM RELOAD CONFIG` resurrected a disabled killer: `applyNewSettings` re-read `data_background_cleanup.enabled` (default true) and re-enabled it, after which the next removal physically unlinked the marked blobs. `disable` now sets a sticky `force_disabled` that `applyNewSettings` honors. Server shutdown deleted the marked blobs: `shutdown` runs a final `executeBlobsCleanup` over the WHOLE queue (`max_to_remove=0`) through the raw object storage. It is now skipped for a `force_disabled` killer. Disks below the directly-wrapped one each own a SEPARATE removal queue that the backup killer never drains, so every DROP/merge/mutation/TTL grew them for the process lifetime (unbounded memory growth). Their killers are now disabled too, and they stop recording removals at the source via `setRecordRemovals`, so nothing is enqueued that nobody will drain. The transaction-local removal list is untouched, so the backup layer still soft-deletes every removed blob. `setRecordRemovals` is deliberately non-virtual on the two concrete queue-owning metadata storages and reached through `dynamic_cast` from `wrapWithBackup`: adding a virtual to `IMetadataStorage` would shift every subclass's vtable slots. Tests: * `gtest_metadata_local_disk`: `TestRecordRemovalsEnqueuesByDefault` and `TestSetRecordRemovalsSuppressesEnqueue` pin that suppression leaves the removal queue empty while the transaction-local list still carries the blob. * `test_aiven_backup_disk_cache_layer`: blobs survive DROP/merge/mutation/TTL through a cache layer. * `test_aiven_backup_disk_cache_layer_reload`: survival holds across `SYSTEM RELOAD CONFIG`. A config-defined stack is required because the wrapped layers of an SQL-inline disk are internal and not re-processed on reload. (cherry picked from commit 5206b4b)
… disks
Aiven's external GC requires that ClickHouse never physically deletes remote
blobs: removals must become marker files that an out-of-band process reconciles.
This was implemented as a `backup` disk type that wrapped an already-constructed
disk, plus machinery to suppress the physical deletions the layers underneath it
would otherwise still perform.
That approach could not be made correct. By the time the `backup` disk wrapped
another disk, the raw object storage had already been handed out to several
owners the wrapper had no way to reach:
* the inner disk itself, which stays live in the global `DisksMap`;
* the inner disk's `BlobCopierThread`, which was never disabled;
* the inner disk's `BlobKillerThread` one level deeper than the wrapped disk -
`wrapWithBackup` only disabled the disk it directly wrapped, so in the
production `cache -> object_storage` stack the base killer kept draining its
own removal queue through the raw object storage;
* `plain` and `plain_rewritable` metadata storages, which capture the object
storage by value in `MetadataStorageFactory` and call `removeObjectsIfExist`
directly, bypassing the removal queue entirely.
The last one has no possible fix at the disk level. It happens not to fire today
only because the deployment uses `metadata_type = local`, whose
`MetadataStorageFromDisk` holds no object storage pointer at all - an accident of
configuration, not a property of the design.
Wrap at construction instead. `RegisterDiskObjectStorage` now applies
`SoftDeleteObjectStorage` to the object storage as it is created, before it is
placed in the router. Every downstream consumer - the router, metadata storage
factory, blob killer, blob copier, transactions - receives the decorated storage
and no component can hold an undecorated one. The invariant becomes structural
rather than something maintained by disabling things after the fact.
Configuration moves onto the object storage disk itself:
<disk_name>
<type>object_storage</type>
<object_storage_type>s3</object_storage_type>
<soft_delete>1</soft_delete>
<soft_delete_markers_path>...</soft_delete_markers_path>
</disk_name>
`soft_delete` is rejected on multi-location disks, where a removal is only
complete once every location has dropped the blob and a single marker cannot
express that.
Because nothing can bypass the soft-delete layer any more, all of the
compensating machinery is removed rather than ported:
* the `backup` disk type and `registerDiskBackup`;
* `DiskObjectStorage::wrapWithBackup` and `stopRecordingRemovals`;
* `BlobKillerThread::detachWrapped`, `disable` and the sticky `force_disabled`
flag that had to survive `SYSTEM RELOAD CONFIG`;
* `setRecordRemovals` / `record_removals` in `MetadataStorageFromDisk` and
`MetadataStorageFromCacheObjectStorage` - removals are enqueued
unconditionally again.
`BackupObjectStorage` is renamed to `SoftDeleteObjectStorage`, which describes
what it does rather than what it was for; the disk is no more a backup than any
other, it just defers deletion.
Tests: `test_aiven_backup_disk` becomes `test_aiven_soft_delete` and
`test_aiven_backup_disk_cache_layer` becomes `test_aiven_soft_delete_cache_layer`,
both configuring the flag inline on the object storage disk. The cache-layer test
is the interesting one - it pins the shape that used to be unsound, where a
killer below the wrapper unlinked blobs the layer above had only marked.
`test_aiven_backup_disk_cache_layer_reload` is deleted outright: it existed only
to prove the sticky disable flag survived a config reload, and there is no longer
a disable flag to make sticky.
Supersedes patch-fix (026).
(cherry picked from commit 3517bf9)
tilman-aiven
marked this pull request as ready for review
August 31, 2026 15:07
joelynch
approved these changes
Sep 1, 2026
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.
No description provided.