A manifest walk over an Iceberg table with many manifests was bounded by
`iceberg_manifest_decode_concurrency`: fetching a manifest (a remote round trip)
and parsing it (CPU, about 2 ms per manifest) were one task on a dedicated pool,
so only that many round trips overlapped (issue #127061: 671 manifests, 9.6 s at
4, 4.0 s at 32).
Manifest files are now read ahead on the IO thread pool, 64 at a time and at most
64 MiB of manifests not in the metadata files cache, and nothing is parsed there.
The read goes through `getOrSetManifestFile` as before, so concurrent queries
still share one read per manifest and `SYSTEM CLEAR ICEBERG METADATA CACHE` keeps
its semantics; the cached cell holds the bytes and parses them on the first
`getDeserializer()` call (`std::call_once`).
The threads that call `IcebergIterator::next` parse the manifests: each one
continues an idle partially read manifest or parses the next one, so parsing
keeps the parallelism of the reading threads; with one reading thread the files
come in the manifests' order as before. Delete manifests are parsed by the
threads that need them, and the last one sorts the deletes. `totalRows` reads
ahead and parses on the calling thread.
Removed: the manifest decode thread pool and its metrics, the producer thread,
the bounded queue and the stop flag of `ManifestFileIterator`,
`ParallelManifestDecode.h`. The query settings `iceberg_manifest_decode_concurrency`
(and its old name `iceberg_delete_manifest_decode_concurrency`) and
`iceberg_file_entries_queue_size`, and the server settings
`max_iceberg_manifest_decode_thread_pool_size`,
`max_iceberg_manifest_decode_thread_pool_free_size` and
`iceberg_manifest_decode_thread_pool_queue_size` are obsolete: accepted and ignored.
The integration tests of the decode concurrency now vary `max_threads` instead;
the test of the removed pool and the timing test of the removed setting are
deleted.
Closes#127061
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve conflicts in the `dlopen` guard in `programs/main.cpp` and
`programs/keeper/keeper_main.cpp`: keep the guards identical (as master now
requires) and keep `USE_GPU` from this branch in both.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`loadMetadataSystem` attaches the persisted tables of the `system` database
before the other databases, so a table in `system` with a projection using
`dictGet` on an XML dictionary was still attached while the dictionary was
unknown. Register the dictionaries right after the temporary database is
loaded, and run the regression test for a table in `system` too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stopping the DDL worker joins the task it runs, including ones that are not
backups or restores. Before the connection wait, a `TRUNCATE ... ON CLUSTER`
waiting in `lockExclusively` for a lock held by a native `INSERT` that waits for
client data kept the shutdown waiting until `lock_acquire_timeout`, instead of
exiting forcefully after `shutdown_wait_unfinished` as master does (66.0 s vs
5.5 s with `shutdown_wait_unfinished = 5` and `lock_acquire_timeout = 60`).
The first kill stays where master has it and still spares the DDL worker's
parts, which keep running during the connection wait. After the forced exit
check, the remaining queries are cancelled again, as `ContextSharedPart::shutdown`
does before it stops the DDL worker, then the DDL worker is stopped and all
backups and restores are waited for, still before the AsyncLoader stops. Both
`shutdown_wait_unfinished_queries` modes now use the same place.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Basically two minor changes
1. Make the setting `collect_connection_metrics` reloadable at the
runtime (via SYSTEM RELOAD CONFIG for example). To make this
possible, now we check for registeration before each query during
HTTP. Common how it's done in other places
2. Make connection_Id UUID instead of Uint64. To avoid complexities and
conflict during cluster mode.
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
The parts of ON CLUSTER backups and restores that the DDL worker registers after
the shutdown wait were spared by both shutdown kills and left to
`BackupsWorker::shutdown`. That runs in `Context::shutdown`, after the scope
guard in `Server.cpp` has already shut down the AsyncLoader. A late
`RESTORE DATABASE ... ON CLUSTER` of a dropped database then fails on the host
being restarted: `InterpreterCreateQuery::createDatabase` schedules the
database's startup job, and `AsyncLoader::schedule` throws
`ASYNC_LOAD_CANCELED: AsyncLoader was shut down: While creating database mydb`.
Drain those parts in `Server.cpp`, before the AsyncLoader stops: kill the other
queries as before, stop the DDL worker (which makes every part it delivered
registered), and wait for all backups and restores again. By default this
happens right after the existing kill. With `shutdown_wait_unfinished_queries`
there is no kill there, so the same kill and drain run after the queries were
waited for. Late initiators are still cancelled before the DDL worker stops,
because an initiator's own part needs it.
A DDL worker whose startup job has not run yet must not be started after it is
stopped, so the drain first removes that job from the AsyncLoader (cancelling
it if pending, waiting for it if running).
Since no such part outlives the drain, the `ContextSharedPart::shutdown` reorder
and its sparing kill are reverted to master. The only path that skips the drain
is a startup failure, where the DDL worker is not running.
The restore shutdown test now restores a dropped database, so it covers the
database and table creation, and it runs twice: restarting node2 (default
settings) and restarting node3, which has `shutdown_wait_unfinished_queries`.
- master shutdown code: RESTORE_CANCELLED (both cases)
- previous head (541c81bf): RESTORE_FAILED, ASYNC_LOAD_CANCELED (both cases)
- this commit without the drain after the query wait: RESTORE_CANCELLED in the
`shutdown_wait_unfinished_queries` case
- this commit (9120585a): RESTORED (both cases, 3/3 each)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The definitions of the dictionaries from the configuration files were added to
`ExternalDictionariesLoader` only after `loadMetadata`, so a table referring to
such a dictionary was attached while the dictionary was unknown: the dependency
was dropped by `TablesLoader::removeUnresolvableDependencies` as nonexistent,
and a projection using `dictGet` could not be analyzed and was skipped.
Now the configuration repository is registered before `loadMetadata`, while
automatic loading is disabled in the loader until `loadOrReloadDictionaries`,
so no dictionary starts loading earlier than before.
Closes: https://github.com/ClickHouse/ClickHouse/issues/67521
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A lookup that misses no longer repeats the node search of every XML layer: `XMLConfiguration`
already reports a node that has children, so `LayeredConfiguration::getRaw` probes only the flat
`MapConfiguration` layers, and `MapConfiguration::enumerate` starts at `lower_bound(prefix)`
instead of scanning the whole map. On a server-shaped configuration (programs/server/config.xml,
the command line of CI's replica server, the application and system layers; debug build) a
missing key now costs 1036 ns, against 907 ns on master and 1934 ns before this change; a key
that exists is unchanged (318 ns).
A section given only on the command line reads as "" through `getString(section, default)`, and
`getInt(section, default)` throws; the comment in `getRaw` states this.
`remote_servers` stays file-only. `Cluster` takes the server's own `tcp_port` from that
configuration to find its local replicas, so with the command line on top, a server started with
`-- --tcp_port` treats the replicas on the file's port as remote: measured `is_local` 1 -> 0 and
`cluster()` failing with `ALL_CONNECTION_TRIES_FAILED`, which is the shape the performance job
uses (servers on 9001/19001, `test_cluster_two_shard_three_replicas_localhost` on 9000).
https://github.com/ClickHouse/ClickHouse/pull/124915#issuecomment-6192247516
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts were only in the AI function integration tests: the cancellation
tests from master run alongside this branch's concurrency tests, pinned to one
request per thread.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Conflicts with the query id change: the executor path now sets `query_id` and
`initial_query_id` and submits `AIChatRequest`; `sanitizeForLog` is `sanitize`.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`config()` has a writable layer at `PRIO_APPLICATION` where `Poco::Util::Application` puts
`application.name`, `application.dir` and the other `application.*` properties. The reload
snapshot of the file, the command line and `SystemConfiguration` did not have it, so
`${application.name}` stayed literal on reload: a masking rule written with it masked at
startup and silently stopped masking after the first `SYSTEM RELOAD CONFIG`, for a rule in
the file as well as for one on the command line.
Copy the `application.*` properties at the start of `Server::initialize`, before
`BaseDaemon::initialize` adds the command line and the file, and add that copy to every
reload snapshot at `PRIO_APPLICATION`. Copying them from `config()` at reload time instead
would also capture an `application.*` key of the previous file (a file can have one only with
`skip_check_for_incorrect_settings`) above the reloaded file. The snapshot now has the same
sources as `config()`: the command line, the application properties, the file and the system
properties.
The masking test now uses `${application.name}_${system.env.MASK_PATTERN}`, so it covers both
property sources: it fails on the previous head after the reload and passes here.
Reported by the review bot on #124915:
https://github.com/ClickHouse/ClickHouse/pull/124915#discussion_r4270445050
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
At startup the query masking rules, ZooKeeper and the listeners read `config()`,
which has the `SystemConfiguration` layer that Poco's `${system.env.NAME}`
expansion resolves against. On reload the main config reloader callback passes
them a snapshot of the file and the command line instead, which had no system
layer, so `${system.env.NAME}` stayed literal: a masking rule written that way
masked at startup and silently stopped masking after the first
`SYSTEM RELOAD CONFIG`, leaking the value into `system.query_log` and the text log.
Add a `SystemConfiguration` at `PRIO_SYSTEM` to the snapshot, owned by it
(`shared = false`, as `Application::setup` does for `config()`). The validation of
the reloaded file keeps reading the file alone, so key enumeration there does not
see a `system` node.
Reported by the review bot on #124915:
https://github.com/ClickHouse/ClickHouse/pull/124915#discussion_r4263740871
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reload callback checks the constant labels of the Prometheus endpoints
before it installs the new configuration, so that a colliding label rejects the
reload as a whole. The check read the parsed file alone, so a label that the
command line sets (for example `--prometheus.labels.disk=...`) was not seen. A
file change that makes such a label collide, such as switching
`asynchronous_metrics_key_values_mode` to `key_values` or turning `info` on, then
passed the check, the configuration was replaced, and only the rebuild of the
endpoint failed: the reload reported an error but the new mode was already
published, and the HTTP listener of an `http_handlers` rule stayed stopped.
Build the file plus command-line view before the check and give it the same
view the reloaders below already get.
Measured with a file `prometheus` rule on `http_handlers` (`info` 0, then 1) and
`--http_handlers.rule.handler.labels.name=x`: the previous head rejected the
reload after replacing the configuration (HTTP port refused afterwards); now
the reload is rejected before anything changes and the endpoint keeps serving,
as it does when the same label is in the file. Startup is unchanged: a
command-line-only rule with a reserved label already refused to start, because
the endpoint factory checks the merged configuration.
The new `test_command_line_label_rejects_the_reload_and_changes_nothing`
integration test passes now, fails on the previous head (the key-value form is
published by the rejected reload), and fails on the PR base, where the
command-line label is not applied at all.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`updateServers` compares the configuration remembered by the previous reload
(`latest_config`) with the live layered `config()`. `latest_config` held the
parsed file alone, so a section that the command line sets or overrides (for
example `http_handlers` or `prometheus.handlers`) looked changed on every
`SYSTEM RELOAD CONFIG`, and the HTTP or Prometheus listener was stopped and
started again each time. Remember the same file plus command-line view that the
ZooKeeper and masking-rules reloaders get.
Measured with two reloads each: command-line-only `http_handlers`,
command-line-only `prometheus.handlers`, and a file `http_handlers` rule with a
leaf overridden on the command line stopped the listener 2 times on the PR base
and on the previous head, and 0 times now. A change of the file section still
restarts the listener once and serves the new content.
The new `test_command_line_handlers_survive_reload` integration test serves a
command-line-only rule and checks that two reloads stop no listener: it passes
now, fails with 2 restarts on the previous head, and fails on the PR base, where
the rule is not served at all.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
At startup the ZooKeeper clients and the query masking rules read the merged
configuration, which includes the arguments after `--`, but a config reload
handed them the parsed file alone. Now that a section given only on the
command line is visible at startup, masking rules from the command line were
dropped by the first `SYSTEM RELOAD CONFIG` (masking silently turned off), and
an auxiliary ZooKeeper from the command line became unknown after it. An
override inside a file section (`-- --zookeeper.node.port=N`) also made the
reload reconnect to the file's port.
The reload now gives these three readers the file with the command line on
top, parsed the same way `BaseDaemon` builds that layer, and the ZooKeeper
reload is gated on the same configuration it reads.
`test_zookeeper_session_on_config_reload` now overrides the file's
`session_timeout_ms` on the command line; before this change the first
`SYSTEM RELOAD CONFIG` opened a new session.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A server can configure several stateless worker clusters: the flat
`stateless_worker_client` block stays the default cluster, and named clusters
live under `stateless_worker_client.clusters.<name>`. A query picks one with the
new `distributed_plan_worker_cluster` setting.
`IStatelessWorkersProvider` abstracts a worker source. Providers are built by
`StatelessWorkersProviderFactory` from registered provider kinds; this build
registers `static`, a fixed `cluster` (single-shard `remote_servers` entry) or
`host`. `StatelessWorkersProviderRegistry` owns one provider per cluster, builds
it on first use, and on config reload retires a cluster whose configuration
changed (queries still holding it drain on it) without ever throwing.
The query context resolves its cluster's provider once, so plan sizing and the
allocation (`StatelessWorkerAllocation`) see one provider and one worker set.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`argsToConfig` stores the arguments after `--` as flat keys of a
`MapConfiguration` layer (`merge_tree.max_suspicious_broken_parts` -> `7`).
Such a layer has no key for the parent node, so `has("merge_tree")` was
false on the merged `LayeredConfiguration` while `keys("merge_tree")`
already returned the command-line children. Every reader of the form
`if (!config.has(section)) return; config.keys(section, ...)` therefore
silently dropped a section that existed only on the command line:
`MergeTreeSettings`, `DistributedSettings`, `DatabaseReplicatedSettings`,
Keeper coordination settings, `macros` in clickhouse-local,
`remote_url_allow_hosts`, `http_options_response`, system logs,
`zookeeper`, and `filesystem_caches` (which threw at startup instead).
`LayeredConfiguration::getRaw` now reports a node that has children in
some layer as present with an empty value, after the exact-key lookup
over all layers has failed, so a real value is never shadowed whatever
the layer order. The per-site workaround in `LocalServer` that
materialized `http_options_response` for the same reason is removed.
The ZooKeeper reload on a configuration change is gated on the parsed
file it then reads, so a ZooKeeper section given only on the command
line keeps its startup session instead of failing every later reload.
Closes#124747
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts with https://github.com/ClickHouse/ClickHouse/pull/121671 resolved by
keeping its startup/shutdown race fix in `StorageMergeTree` while dropping the
statistics refresh task (`startStatisticsCache`, `stopStatisticsCache`,
`refresh_stats_task_mutex`), which this PR removes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve conflicts in `MergeTreeData.cpp` with #124420: keep the per-part
`StatisticsCache` and the removal of `refreshStatistics`, take master's
removal of `readLockParts` around `loadStatistics` and the new
`LoadedStatistics` profile event. With the cache, `LoadedStatistics`
counts the parts whose statistics are collected for the estimator, from
the cache or from the part, so its description and the test comments are
updated accordingly.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The shipped `config.xml` is also embedded into the generic installer used on
macOS and FreeBSD, where `<seccomp>trap</seccomp>` was silently inert: no
startup warning, and `system.server_settings` showed `trap`. Now
`installSeccompFilter` exists on every OS; outside Linux it installs nothing,
records `disabled` as the mode in force, and the server logs a warning.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts: master reworked the `startup`/`shutdown` race handling and guarded `refresh_stats_task`
with a mutex; this PR removes the periodic statistics refresh task entirely, so master's
`startup`/`shutdown` structure is kept without `startStatisticsCache`/`stopStatisticsCache`.
Alexey Milovidov