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
* 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.
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.
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
A default `clickhouse-local` invocation has no system log sections in the config.
Initializing `SystemLogs` regardless meant the unique pointer in `ContextSharedPart`
was written during construction and again during teardown, racing under TSan with a
concurrent read from a `BackgroundSchedulePool` worker that calls
`getBackgroundSchedulePoolLog` after a task finishes. The race surfaced as a
"Scraping system tables" failure in the `Stateless tests (amd_tsan, parallel, 1/2)`
job, since CI scrapes system tables via short-lived `clickhouse-local` processes.
Gate the call on the config actually containing at least one entry from
`LIST_OF_ALL_SYSTEM_LOGS`. The custom config used by `04093_system_log_alter_column`
declares `query_log` and `processors_profile_log`, so the regression test continues
to exercise the original fix; default invocations stop touching `system_logs`.
CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=102316&sha=665b5c743abc6d9c6ac9946eb60d70df95ce08c8&name_0=PR&name_1=Stateless%20tests%20%28amd_tsan%2C%20parallel%2C%201%2F2%29
PR: https://github.com/ClickHouse/ClickHouse/pull/102316
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
When `--no-system-tables` is set, the system database is not attached,
so initializing system logs would fail or violate the flag semantics.
Skip the call in that case, consistent with how system table attachment
is gated.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Add `initializeSystemLogs` call to `LocalServer` so that
`clickhouse-local` creates system log tables (like `query_log`,
`processors_profile_log`) when they are configured via `--config-file`.
Without a config, this is a no-op.
Rewrite the test to run entirely in `clickhouse-local` with a custom
config, avoiding ALTER on the shared server where materialized views
prevent column modifications.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The original PR #96025 was reverted (#98038) because:
1. The install completion message showed wrong relative paths like
`var/run/clickhouse-server` instead of absolute `/var/run/clickhouse-server`.
This happened because `options.contains()` returns true even for options
with default values in boost::program_options. Fix: use `defaulted()`
to only include options that the user explicitly specified.
2. The install completion message did not suggest `sudo` for the
start/restart command. This happened because `getuid() != 0` is false
when install runs as root via sudo, but the user returns to their
unprivileged shell afterward. Fix: always suggest sudo (matching the
original behavior).
Also address review feedback:
- Fix `clickhouse-client` help: remove misleading "initial table definition"
(a `clickhouse-local` concept) from usage line
- Fix typo: "as command line tool" → "as a command line tool"
- Fix `clickhouse-local` help grammar: "After you can execute" → "After that,
you can execute", "in first query" → "in the first query"
- Make keeper `--help` test robust for builds without `ENABLE_CLICKHOUSE_KEEPER`
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
`MemoryWorker` was only started in `clickhouse-server` but not in
`clickhouse-local`. This caused three problems:
1. The userspace page cache stayed stuck at `page_cache_min_size`
(100 MiB by default) and never grew to `page_cache_max_size`,
because `PageCache::autoResize` — which adjusts the cache size
based on available memory — was never called. The result was severe
cache thrashing: blocks evicted immediately after fetching, turning
every "hot" run into a cold run with full S3 re-downloads.
2. RSS was never reported to the global `MemoryTracker`, so memory
tracking in `clickhouse-local` was less accurate.
3. Jemalloc dirty pages were never purged under memory pressure.
Start `MemoryWorker` unconditionally (not only when page cache is
enabled), matching `clickhouse-server` behavior.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The userspace page cache (`page_cache_max_size`) was never initialized
in `clickhouse-local`. The `setPageCache` call only existed in
`programs/server/Server.cpp` but was missing from
`programs/local/LocalServer.cpp`.
This adds the same page cache initialization (including validation
of `page_cache_min_size` vs `page_cache_max_size`) to `clickhouse-local`,
enabling the page cache for S3/object storage queries.
Closes https://github.com/ClickHouse/ClickHouse/issues/99499
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- `Intrinsic::getDeclaration` renamed to `Intrinsic::getOrInsertDeclaration`
in LLVM 22 (abs.cpp, bitCount.cpp)
- `createTargetMachine` first argument changed from `StringRef` to
`const Triple&` — construct `llvm::Triple` explicitly (CHJIT.cpp)
- `std::__gcd` internal removed from libc++ — use `std::gcd` from
`<numeric>` (AggregateFunctionKolmogorovSmirnovTest.cpp)
- Added `(void)` casts for new `[[nodiscard]]` on `std::stod`,
`std::filesystem::is_directory`, and `future.get()`
(KQLGeneralFunctions.cpp, LocalServer.cpp,
ZooKeeperRemoveByList.cpp, EphemeralLockInZooKeeper.cpp)
Assisted-by: Claude Opus 4.6 via GitHub Copilot
- Add missing `extern` for `jemalloc_profiler_sampling_rate` in
`Keeper.cpp`, `LocalServer.cpp`, and `Server.cpp`
- Pass `jemalloc_profiler_sampling_rate` as 5th argument to
`Jemalloc::setup` in `Keeper.cpp` and `LocalServer.cpp`
- Expose `prof.lg_sample` via `system.asynchronous_metrics` as
`jemalloc.prof.lg_sample`
- Add integration test `test_jemalloc_profiler_sampling_rate`
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Extract `ErrnoException` into `Common/ErrnoException.h` to reduce the
include footprint of `Exception.h`. This removes `<base/errnoToString.h>`
and `<cerrno>` from `Exception.h`, which is included by ~95% of all
translation units.
All files referencing `ErrnoException` now include the new header
directly. Files that used `errnoToString` without `ErrnoException`
got an explicit `<base/errnoToString.h>` include.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Allow overriding the default access control improvements via an
explicitly provided config file, instead of unconditionally hardcoding
them to true.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Wrap long lines in `getHelpHeader` and `getHelpFooter` for `clickhouse client`
and `clickhouse local` to keep lines within reasonable width,
fixing the 00910_client_window_size_detection test
- Use `createOptionsDescription` instead of raw `po::options_description` constructor
in `git-import` and `static-files-disk-uploader` to handle zero terminal width
properly, fixing the `git-import --help` abort in debug builds
(03823_clickhouse_help_return_codes test)
Co-Authored-By: Claude Opus 4.6 <[email protected]>
This commit improves the help output for all ClickHouse applications:
- Add `clickhouse help` command that lists all available subcommands
- Return exit code 0 for all help messages (previously some returned 1)
- Output help messages to stdout instead of stderr
- Use "clickhouse <cmd>" format instead of argv[0] in help messages
- Add dynamic `app_name` detection for `clickhouse client` and `clickhouse local`
to show correct invocation name based on how the binary was called
- Add `--no-sudo` option for `clickhouse start` and `clickhouse restart`
for environments without sudo (e.g., Docker containers)
- Add detailed help headers and footers for client and local with usage examples
Closes#58244
Co-Authored-By: Claude Opus 4.5 <[email protected]>
