Replace blanket `catch (...)` with specific exception types based on
analysis of what each try block can actually throw:
- `const Exception &` for catch blocks where only ClickHouse operations
are involved (Field operations, expression evaluation, parsing, etc.)
- `const std::exception &` for catch blocks involving standard library,
Poco, or third-party code (filesystem, HTTP, thread creation, etc.)
Blocks that genuinely need `catch (...)` are left unchanged:
- try-pattern functions (`tryGetPosition`, `tryDeserializeText`, etc.)
- Exception formatting utilities in `Exception.cpp`
- Signal/terminate handlers
- FFI callbacks (DeltaLake)
- Dual-handler patterns (already have specific + catch-all)
- `noexcept` functions with user-supplied callbacks
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Enhance the `catch_all` style check in two ways:
1. Recognize more legitimate exception-handling patterns as "handled":
`getCurrentExceptionMessage`, `getCurrentExceptionCode`,
`getCurrentExceptionMessageAndPattern`, `ExecutionStatus::fromCurrentException`,
`onBackgroundException`, `storeException`, `STDERR_FILENO`, `writeRetry`,
`handle_exception`, `handleException`, `finishWithException`.
2. Fix function-try-block detection for destructors like
`Exception::~Exception() try { } catch (...) { }` — the enclosing-scope
scan now falls through to the function-try-block scan when it hits a
namespace/class scope (no parentheses in signature).
For the remaining 108 catch-all blocks, add `// Ok: <reason>` comments
explaining why each is intentional. Categories include:
- Try-patterns (`tryDeserializeText`, `tryGetPosition`, etc.)
- Exception handling/reporting utilities (`Exception.cpp`)
- Non-essential system probes (`Server.cpp` Linux /proc and /sys checks)
- C callbacks where exceptions cannot propagate (zip archives)
- Background thread exception saving (`onBackgroundException`)
- Fallback/retry logic (`DatabaseOverlay`, `ReadSchemaUtils`)
- Best-effort cleanup or error reporting
- Tests that expect exceptions
Replace `// NOLINT(bugprone-empty-catch)` with `// Ok:` comments on all
catch blocks that were flagged by the new check. Fix Allman-style brace
violations in `convertFieldToType.cpp` and `ClientBase.cpp`.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add optional <handlers> config key to type=http entries in <protocols>,
so each HTTP port can use a different set of http_handlers. This enables
use cases like running an alternative HTTP API (e.g. ES-compatible) on
a separate port with its own routing rules, without affecting the
default HTTP port.
Config example:
<protocols>
<alt_http>
<type>http</type>
<port>8124</port>
<handlers>http_handlers_alt</handlers>
</alt_http>
</protocols>
<http_handlers_alt>
<rule>...</rule>
</http_handlers_alt>
If <handlers> is omitted, the default <http_handlers> is used (fully
backward compatible). Config reload (updateServers) also respects the
per-protocol handlers key.
Netlink-based metric collection is problematic in containerized environments
(Docker, LXC, Kubernetes) where it can hang indefinitely. Benchmarks show
procfs has better tail latency and lower total CPU overhead under contention.
Since all production deployments already use the procfs path, remove netlink
entirely rather than adding a setting to disable it.
Mark getObjectStorage as const.
Add docs and tests.
Fix.
Fix drop replica.
Fix getQueryStatus.
Fix test.
Fix zookeeper usage.
Fix codes review.
Split the tests.
- 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]>
Allow controlling jemalloc's `lg_prof_sample` via a server setting,
which determines the average interval between allocation samples.
The default value of 19 (512 KiB) matches jemalloc's built-in default.
When a non-default value is configured, `Jemalloc::setup` calls
`prof.reset` to apply the new sampling rate.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
`Server.cpp` included `StorageReplicatedMergeTree.h` but never used the
`StorageReplicatedMergeTree` class directly. The only dependency was on
`MergeTreeBackgroundExecutor` for thread pool management (accessed via
`Context::getMergeMutateExecutor` etc.).
`StorageReplicatedMergeTree.h` is an extremely heavy header that
transitively pulls in `MergeTreeData.h`, `SortCursor.h` (with expensive
`std::variant` instantiations for sorting queues), `MergeTask.h`,
`MergeFromLogEntryTask.h`, `TextIndexUtils.h`, and `DataPartsExchange.h`.
The time trace showed 3.9s just for parsing `StorageReplicatedMergeTree.h`
plus ~4.3s of template instantiations from `SortCursor.h` variant types.
Replacing it with the targeted `MergeTreeBackgroundExecutor.h` reduces
`Server.cpp.o` compile time from 34.8s to 28.1s (-19%). Since `Server.cpp`
is on the critical path (last .o file to finish before linking), this
directly reduces wall time.
Build time: 5m6.0s (was 5m16.7s, -10.7s / -3.4%)
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The config reload callback (`ConfigReloader`) fires during construction with
`initial_loading=true`. After the change to use `updateLevels(config(), ...)`
instead of `updateLevels(*config, ...)`, the callback reads `logger.level`
from the layered configuration, where the writable `MapConfiguration` at
`PRIO_APPLICATION` still holds the startup level override ("trace"). This
keeps the log level at "trace" until the explicit restoration after
`server.start()`, creating a window where HTTP requests get logged at
trace level.
Move the startup level restoration to before `server.start()` so that
no requests are ever served with the elevated startup log level.
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=80295&sha=c7c2c8bd852fc40cf3640ee5810c7976dcfda96c&name_0=PR&name_1=Integration%20tests%20%28amd_tsan%2C%201%2F6%29
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Fix the ZK includes reload path in `Server.cpp` to use
`config().replace` instead of `removeConfiguration`/`add`/`setConfig`,
consistent with the reload callback. This ensures command-line overrides
are preserved when ZK includes are loaded.
Add integration test `test_config_not_overriding_args` that verifies
command-line arguments are not overridden by config file values
(including when ZK includes are used). Based on the test from #71452.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The `replaceByLabel` calls in `Server.cpp` and `Keeper.cpp` passed
`shared = false`, which means the `AutoPtr` inside `LayeredConfiguration`
took the raw pointer without incrementing the reference count. But the
caller's `loaded_config` `AutoPtr` also holds a reference. When the
callback's `loaded_config` goes out of scope, its destructor decrements
the reference count, potentially freeing the object while the
`LayeredConfiguration` still holds a pointer to it.
Passing `shared = true` makes the `AutoPtr` constructor call `duplicate`,
properly incrementing the reference count so the `LayeredConfiguration`
has its own counted reference.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
`BaseDaemon::initialize` performs up to 3 `chdir` calls during daemon
startup (to application path, log path, and cores directory). After
`initialize` returns, `Server::main` reads path settings from config.
If those paths are relative, they resolve against the cores directory
instead of the original startup directory.
Fix by capturing the original working directory at the start of
`BaseDaemon::initialize` and using it to resolve the main `path`
setting to absolute. All secondary path settings (like `user_files_path`,
`tmp_path`, `format_schema_path`, etc.) are resolved relative to the
now-absolute main `path`.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The config reload callback in `Server.cpp` and `Keeper.cpp` was doing
`find` + `removeConfiguration` + `add` on the shared `LayeredConfiguration`
from the `ConfigReloader` thread. This is not thread-safe because concurrent
readers (calling `getString`, etc.) iterate the same internal list without
synchronization against structural modifications.
`AbstractConfiguration` already has `mutable Poco::Mutex _mutex` that all
public read methods lock. The new `replaceByLabel` method locks the same
`_mutex` and atomically swaps the config pointer within the matching list
node — no list insert/erase needed for the common case, guaranteeing mutual
exclusion with concurrent readers.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The previous order (add new, find, remove) was incorrect because
`LayeredConfiguration::add` inserts at the same priority position
before existing entries, and `find` returns the first match.
This means the newly added config was found and removed instead
of the old one.
Changed to match the correct order used in Keeper.cpp:
find old, remove old, add new.
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]>
