Address review: a positive max_server_memory_usage_to_ram_ratio used to
opt the server into runtime hard-limit shrinking with no way to keep the
previous static cap, while setting the ratio to 0 removed the cap
entirely. The new memory_worker_dynamic_hard_limit server setting
(enabled by default) gates only the runtime adjustment: when disabled,
max_server_memory_usage_to_ram_ratio caps the hard memory limit
statically at startup and on configuration reload, exactly as in
previous versions.
Co-Authored-By: Claude Fable 5 <[email protected]>
New keeper-only system table with one row per on-disk Raft log file
(changelog_<from>_<to>.bin[.zstd]) tracked by the in-process Keeper.
Mirrors the plumbing of system.keeper_snapshots.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Address review: the shared `setupEchoAndHighlightSettings` made `--verbose`
imply query echoing for all clients, but historically only `clickhouse-local`
behaved that way. `clickhouse-client` and the embedded client defaulted
`echo_queries` from `--echo` only, so `clickhouse-client --verbose --query ...`
emitted just the result.
Make the implication opt-in via a `verbose_implies_echo` parameter (passed
`true` only from `LocalServer`), restoring the prior contract that existing
behavior stays identical unless the new echo options are used.
Extend `04308_echo_options_granular` to cover both cases: `--verbose` echoes
in `clickhouse-local` but not in `clickhouse-client`.
Implement https://github.com/ClickHouse/ClickHouse/issues/95479.
Previously, the client echoed queries differently depending on the mode
with no way to control it granularly: in interactive mode it always
printed a formatted and highlighted version of the query and the
`query_id`, while in batch mode `--echo` printed the raw query text only.
This change makes the behavior configurable while keeping the defaults
identical to before:
- `--echo` now takes an optional boolean value and controls echoing of
queries in both interactive and batch mode. It defaults to on in
interactive mode and off in batch mode.
- `--echo-formatted` is a new boolean option controlling whether the
echoed query is formatted. It defaults to on in interactive mode and
off in batch mode, so highlighting non-formatted queries is now
possible as well.
- `--echo-query-id` is a new boolean option controlling whether the
`query_id` is printed before execution. It defaults to on in
interactive mode and off in batch mode. When enabled in batch mode, a
fresh `query_id` is generated per query so that the printed value
matches the one used for execution.
- `--hilite` (`--highlight`) now controls highlighting of both the
command prompt and the echoed queries. Highlighting is only applied
when writing to a terminal.
The echoing logic for all modes is unified in the new `echoQuery`
method, and the per-client defaults are resolved in
`setupEchoAndHighlightSettings`.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
The constructor body of ThreadFromGlobalPoolImpl was a fully-inlined
lambda referencing DB::ThreadStatus and SCOPE_EXIT, which forced
ThreadPool.h to pull Common/ThreadStatus.h, Common/StackTrace.h and
base/scope_guard.h into ~329 TUs that include it transitively.
Hoist the nested State struct to a non-template ThreadFromGlobalPoolState
and move the lambda body into a free function startThreadFromGlobalPool
that lives in ThreadPool.cpp. The compile-time
global_trace_collector_allowed branch becomes a runtime bool (single
check per thread start, invisible cost).
Drop the now-unused includes of Common/ThreadStatus.h and
Common/StackTrace.h from ThreadPool.h.
A positional file argument is converted to --queries-file inside
ClientApplicationBaseParser via options.emplace, which populates the
boost options map (and the queries_files member) but never the Poco
config. The conflict check in processConfig used config.has("queries-file"),
so the positional case slipped through and the -q query was silently
dropped instead of erroring.
Check queries_files directly so both forms are rejected consistently.
Closes#104735
`createClickHouseLocalDatabaseOverlay` looked up the persisted database UUID
at `metadata/default` (hardcoded). `DatabaseAtomic` actually creates the
symlink at `metadata/<escapeForFileName(name)>`, so when the user passes
`--default_database=mydb` the lookup misses the previous run's symlink, a
fresh UUID is minted, and the previous run's data is orphaned. The table
then shows up as `UNKNOWN_TABLE` on the next invocation despite the data
still sitting in `store/`.
Use `DatabaseCatalog::getMetadataDirPath(name_)` instead, which is the same
helper that `DatabaseAtomic` uses for its metadata symlink path.
Closes#101831.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
* Make client rainbow parentheses optional
* Make client rainbow parentheses optional
* Make client rainbow parentheses optional
* Make client rainbow parentheses optional
* Fix style and clang-tidy CI failures
- In `programs/format/Format.cpp`, replace `options.count(...)` with
`options.contains(...)` to satisfy the `readability-container-contains`
clang-tidy check that fires in the `arm_tidy` build.
- In `src/Client/ClientBaseHelpers.cpp`, drop redundant braces around
single-statement `if (!rainbow_parentheses) ...` blocks. One of them
had the opening brace on the same line, which the C++ style check
rejects (Allman braces are required).
CI report: https://github.com/ClickHouse/ClickHouse/pull/103851
---------
Co-authored-by: Alexey Milovidov <[email protected]>
Add a server setting (`min_allocation_size_to_throw_on_memory_limit`) to
allow any allocation to throw `MEMORY_LIMIT_EXCEEDED`, this should help
with preventing OOMs
Right now, any `new` cannot throw, this is due exception safety issues,
and only explicitly marked can throw `MEMORY_LIMIT_EXCEEDED`.
But this can lead to real OOMs, i.e. when kernel will kill
clickhouse-server, even though we are trying hard to track memory
properly.
Add a server setting `min_allocation_size_to_throw_on_memory_limit`
(UInt64 bytes, default 0). When non-zero, the throwing `operator new`
variants are allowed to raise `MEMORY_LIMIT_EXCEEDED`. _The assumption
is that code with big enough allocations does not exception safety
issues._
Note, places that allows (Allocator)/never allows (malloc)
`MEMORY_LIMIT_EXCEEDED` before are not affected.
Three follow-up fixes for `MemoryWorker`'s dynamic hard-limit logic, based on
re-review comments from `clickhouse-gh`.
Reload race in `setHardLimit` (TOCTOU between generation check and apply).
The previous design captured `settings_generation` at the start of the worker
tick and re-checked it just before `setHardLimit`. But the actual apply was
not under any lock, and `Server.cpp` performed its own `setHardLimit` outside
the worker's view, so a worker tick that observed the old generation could
still call `setHardLimit` *after* a reload had installed its own value,
overwriting it with a stale number computed against the old ratio. With
`max_server_memory_usage_to_ram_ratio = 0`, subsequent ticks skip the
adjustment and the stale limit could persist indefinitely.
Fixed by:
* Adding `dynamic_hard_limit_apply_mutex` that both `setDynamicHardLimitSettings`
and the worker's apply step take. The mutex serializes the two writers, so
the worker's "re-check generation then `setHardLimit`" is now atomic with
respect to a reload's "store new settings then `setHardLimit`".
* Moving the `total_memory_tracker.setHardLimit(max_server_memory_usage)`
call from `Server.cpp` / `LocalServer.cpp` into `setDynamicHardLimitSettings`
so the hard limit is installed under the same mutex.
Cgroup v1 "no limit" sentinel.
`memory.limit_in_bytes` uses a huge numeric sentinel (`PAGE_COUNTER_MAX`,
~2^63) to mean "no limit". The previous code treated any numeric `> 0` as
finite, so the sentinel made `any_finite == true` and the dynamic limit
stayed pinned to the startup ceiling on v1-unlimited hosts instead of
falling back to `/proc/meminfo`. Captured host RAM at construction (raw
`sysconf(_SC_PHYS_PAGES) * _SC_PAGESIZE`, not the cgroup-aware
`getMemoryAmount`) and skip any limit `>= host_memory_bytes`, the same way
the v2 `"max"` token is skipped.
Skip-when-pressure path actually leaves us with the old, larger limit.
Under high memory pressure the formula can produce `new_hard_limit <= used`.
The previous code skipped the tick to avoid rejecting in-flight allocations
— but that left the previous (often much larger) limit in place, breaking
the contract of shrinking the budget as available memory drops. Replaced
the skip with a clamp: `new_hard_limit = max(formula, used + 64 MiB)`,
still capped at `ceiling`, so the shrink takes effect with a small headroom
for in-flight queries instead of being thrown away.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Addresses clickhouse-gh[bot] review on PR #105008 (id 3247356705).
Previously only `user_directories.users_xml.path` was rebased to the
loaded config's directory; the top-level `<users_config>` was passed
to `ConfigProcessor` verbatim and resolved against the current
working directory. So `~/.clickhouse-local/config.xml` with
`<users_config>users.xml</users_config>` could silently load an
unrelated `./users.xml` from cwd instead of failing fast when the
configured file is missing.
Move the relative-path rebase out of the `user_directories` branch so
it applies uniformly to both `users_config` and
`user_directories.users_xml.path`.
Extend `04245_clickhouse_local_home_config.sh` with a matching sub-case
for the `users_config` form: a `~/.clickhouse-local/config.xml` that
references a relative `users.xml` which is missing next to the config,
with a rogue `users.xml` planted in cwd, must surface
`FILE_DOESNT_EXIST` and not silently load the rogue file. Verified
locally: test fails on master HEAD without this commit, passes
50/50 with full randomization with it.
Also trims the prior verbose 5-line security comment to 3 lines, per
@Algunenano's earlier feedback on comment verbosity throughout the PR.
Introduce "drivers" for executable user-defined functions. A driver is a
small adapter program declared in a server config under
`<user_defined_executable_function_drivers_config>` that knows how to take a
user-supplied code snippet and produce a runnable executable user-defined
function configuration.
New CREATE FUNCTION syntax:
CREATE [OR REPLACE] FUNCTION [IF NOT EXISTS] name [ON CLUSTER c]
ARGUMENTS (a UInt8, b String) RETURNS UInt64
ENGINE = DriverName(key1 = 'value1', key2 = 42)
AS '...code body...'
On CREATE FUNCTION, the corresponding `create_command` of the driver is
invoked: arguments are passed via `--name`, `--return`, `--args` and the
declared engine arguments; the body is sent on stdin; the driver writes the
resulting executable UDF configuration (XML or YAML) to stdout, which is
saved into the new `<dynamic_user_defined_executable_functions_path>` and
picked up by the existing executable UDF loader. The corresponding SQL is
persisted as `ATTACH FUNCTION ...` in `<user_defined>` and re-evaluated on
restart: if the dynamic configuration is still present, the driver is not
invoked; otherwise the driver is re-run to materialize it.
On DROP FUNCTION, the `drop_command` (if any) is invoked, then the dynamic
configuration file and per-function working directory are removed.
Includes a proof-of-concept driver `c_function_body`, which compiles a user
C body into a binary (inside Docker by default for build-time isolation) and
runs it inside Docker at query time with strict sandboxing (`--network=none`,
`--read-only`, dropped capabilities, no-new-privileges, unprivileged user,
resource limits) in `executable_pool` mode for batched per-row evaluation.
See https://github.com/ClickHouse/ClickHouse/issues/71172
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Previously the rebase to `config_dir` was guarded by an `fs::exists` check.
When the configured `users.xml` was missing next to the loaded config (for
example a stale path or a TOCTOU race where the file appears later), the
relative path was passed unchanged to `ConfigProcessor`, which resolves it
against the current working directory. A `users.xml` sitting in the cwd
would then be silently loaded instead of the one the user configured -
and that file could trivially grant `access_management` or otherwise widen
permissions for the `default` user.
Drop the `exists` guard and always rebase relative `users_xml.path` values
against the config's own directory. If the file is missing, `ConfigProcessor`
now throws `FILE_DOESNT_EXIST` with the configured absolute path, surfacing
a clear error instead of falling through to an unrelated file.
Verified locally: with `~/.clickhouse-local/config.xml` pointing at
`users.xml` and that file missing, the previous binary silently picked up
`./users.xml` from cwd (`max_threads = 99` in my repro); the fixed binary
throws `Configuration file <abs-path>/users.xml doesn't exist`. New
regression coverage in `04245_clickhouse_local_home_config.sh` asserts this
behavior.
Addresses clickhouse-gh[bot]'s review and @Algunenano's request to verify
the bot's claim on https://github.com/ClickHouse/ClickHouse/pull/105008.
`LocalServer::initialize` loads the configuration file from one of:
- `--config-file` command-line flag
- `./config.xml` in the current working directory
- `getLocalConfigPath` discovery: `./clickhouse-local.{xml,yaml,yml}`,
`~/.clickhouse-local/config.{xml,yaml,yml}`, `/etc/clickhouse-local/config.{xml,yaml,yml}`
But `LocalServer::setupUsers` only recognised the first two sources via its
gating condition:
if (getClientConfiguration().has("config-file") || fs::exists("config.xml"))
For configs discovered by `getLocalConfigPath` (the documented user-config path
introduced by #67135), the condition was false and `setupUsers` fell through to
`minimal_default_user_xml`, silently dropping the user's `profiles`, `users`,
and `quotas`. Setting `max_threads = 42` in `~/.clickhouse-local/users.xml`
under the default profile was ignored, and the built-in default was returned.
The fix tracks the actual loaded config path in `LocalServer::loaded_config_path`
during `initialize`, and uses it in `setupUsers` both as the gate and as the
source of `config_dir` (which resolves relative `user_directories.users_xml.path`
entries against the config's own directory rather than the current working
directory). All three search paths plus `--config-file` and `./config.xml` now
share the same user-config loading code path.
A new regression test `04245_clickhouse_local_home_config.sh` exercises the
`HOME/.clickhouse-local/config.xml` and `./clickhouse-local.xml` discovery paths
and verifies that `--config-file` and the no-config fallback still work.
Closes#104932
The previous commits used `total_memory_tracker.get()` as the "memory we
already own" term of the `(used + available) * ratio` formula. That value
counts only allocations that went through `Allocator` -- jemalloc-internal
fragmentation, mmap'd pages, page cache, and any untracked allocation are
excluded. Under load, the tracker can be orders of magnitude smaller than
the actual RSS, which makes the formula compute a new hard limit barely
above `tracked` while RSS is far higher -- pinning the limit at (or below)
current RSS and rejecting every subsequent allocation.
Reproduced on `Stateless tests (amd_asan_ubsan, distributed plan, parallel,
2/2)` (https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=104964&sha=ced8215d96e5342ac0619ba94e7582618c0b60ca&name_0=PR&name_1=Stateless%20tests%20%28amd_asan_ubsan%2C%20distributed%20plan%2C%20parallel%2C%202%2F2%29):
168 tests failed with `Connection reset by peer` after the lead query
got `(total) memory limit exceeded: would use 780.35 MiB (attempt to
allocate chunk of 1.01 MiB), current RSS: 28.67 GiB, maximum: 28.67 GiB`.
The hard limit had been driven to 29 GiB by `(tracked: 780 MiB +
available: 31 GiB) * 0.9 = 28.6 GiB` while actual RSS was 28.67 GiB. PR:
https://github.com/ClickHouse/ClickHouse/pull/104964
Three changes:
* Use `resident` (the jemalloc/cgroup RSS already computed at the top of
the worker tick) as the baseline. `(resident + available) * ratio`
always exceeds `resident` when `available > resident * (1/ratio - 1)`
and tracks actual memory pressure faithfully.
* Refuse to apply a new hard limit that lies at or below `resident`,
even after the ceiling clamp. Shrinking under RSS cannot succeed (the
server has no way to release memory instantly) and would only break
in-flight queries -- the purpose of the adjustment is to leave room
for *other* processes, not to throttle our own work.
* Roll the previous `setExternalHardLimit(ceiling)` setter into
`setDynamicHardLimitSettings(ceiling, ratio)`, so a config-reload
change to `max_server_memory_usage_to_ram_ratio` takes effect on the
next worker tick. The previous design captured the ratio once in the
constructor; subsequent reloads updated `total_memory_tracker`'s hard
limit but left the worker's formula running with the stale ratio.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The dynamic hard-limit adjustment added in the previous commit read
`MemFree + Cached` from `/proc/meminfo`. That source describes the
**host**, not the cgroup, so when ClickHouse runs in a small cgroup
on a big host (8 GiB cgroup on a 192 GiB host) the formula
`(tracked + free + cached) * ratio` inflates the global hard limit
to ~145 GiB on every tick, overriding the cgroup-derived 7.2 GiB
that `Server.cpp` set at startup. `MemoryTracker` then never throws
`MEMORY_LIMIT_EXCEEDED` and the kernel OOM-killer fires at 8 GiB
RSS, killing the server.
The same `setHardLimit` call also overrode an explicit
`max_server_memory_usage` from the config, since the dynamic
adjustment ran every 50 ms regardless of the value `Server.cpp`
had just installed.
Two changes:
* When `MemoryWorker` has a cgroup reader, the dynamic adjustment
reads the cgroup's `memory.max` paired with the cgroup-aware
usage from `cgroups_reader->readMemoryUsage` -- the same sources
`AsynchronousMetrics` reports as `CGroupMemoryTotal` and
`CGroupMemoryUsed`. The available-memory term becomes
`memory.max - cgroup_used`. The `/proc/meminfo` path is kept as
a fallback for the no-cgroup case.
* `Server.cpp` and `LocalServer.cpp` now hand the configured
`max_server_memory_usage` to `MemoryWorker` via the new
`setExternalHardLimit` setter. The dynamic adjustment caps its
computed value at this ceiling, so it can only shrink the budget
below the user's setting, never raise it above. The setter is
re-called from the config-reload callback whenever the setting
changes. Before it is called for the first time, the dynamic
adjustment is suppressed, so the worker cannot inflate the limit
during the brief window between `MemoryWorker::start` and the
first reload.
Reproduced and verified with an 8 GiB systemd-run scope on a 192
GiB host: before the fix, five memory-heavy queries all triggered
the cgroup OOM-killer; after the fix, all five are caught cleanly
by `MemoryTracker` and the server stays alive across the battery.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
`MemoryWorker` now periodically reads `MemFree + Cached` from
`/proc/meminfo` and updates the global hard memory limit to
`(tracked + free + cached) * max_server_memory_usage_to_ram_ratio`.
This shrinks ClickHouse's effective limit when other processes on
the host consume memory, reducing the risk of OOM-killing.
The ratio is the existing `max_server_memory_usage_to_ram_ratio`
server setting (default 0.9 in Server and Local); setting it to 0
disables the dynamic adjustment.
Keeper does not expose the ratio, so dynamic adjustment is off there.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Move three `static` helpers in `LocalServer.cpp` into anonymous namespaces
(`createMemoryDatabaseIfNotExists`, `createClickHouseLocalDatabaseOverlay`,
`getConfigurationFromXMLString`) to match the file's existing convention.
In `QueryProfiler.cpp`, document why the `-Wdisabled-macro-expansion` pragma
spans the full `sigaction` setup: `sigaction`, `SA_SIGINFO`/`SA_RESTART`,
`sigemptyset`, and `sigaddset` are all macro-expanded names on supported
platforms, so the suppression must cover the entire block.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- `programs/local/LocalServer.cpp`: replace file-level
`-Wdisabled-macro-expansion` with a narrow push/pop around the two
`fmt::print(stderr, ...)` lines that need it on musl.
- `src/AggregateFunctions/Moments.h`: drop the file-level
`-Wdouble-promotion` pragma. `CorrMoments::get` and the two
`getStandardError` helpers now use `std::sqrt`, and the `boost::math`
call sites use `double` literals so no implicit float→double
promotion remains. `cmath` is now included explicitly.
- `src/AggregateFunctions/AggregateFunctionGroupNumericIndexedVectorDataBSI.h`:
drop the file-level `-Wdouble-promotion` pragma — the file already
performs all float/int conversions through `static_cast`.
- `src/Columns/Collator.cpp`,
`src/Storages/System/StorageSystemUnicode.cpp`,
`src/Storages/System/StorageSystemStackTrace.cpp`,
`src/Functions/DivisionUtils.h`: keep the file-level pragma but add a
short comment explaining why the broad scope is needed (ICU's
`U_ICU_ENTRY_POINT_RENAME` wrapper macros, glibc's `errno` macro, and
the templated `apply` methods that mix `Float32`/`Float64` operands
throughout the file).
Alexey Milovidov