- Remove `CurrentThread.h` from high-fan-out headers: `ProcessList.h`,
`IQueryPlanStep.h`, `DDLWorker.h`, `TCPHandler.h`, `AggregatingTransform.h`,
`ParallelParsingInputFormat.h`, `StorageTableFunction.h`, `parseSyslogLevel.h`,
`EmbeddedRocksDBBulkSink.h`, `BlockIO.h`, `LocalConnection.h`,
`StorageMaterializedView.h`, `HTTPHandler.h`
- Extract `QueryScope` from `CurrentThread` into standalone `Common/QueryScope.h`
with its own `.cpp`, replacing the nested `CurrentThread::QueryScope` class
- Extract `getCurrentThreadGroup` free function into `Common/ThreadGroupSwitcher.h`,
replacing `CurrentThread::getGroup()` in template headers:
`threadPoolCallbackRunner.h`, `ParallelFormattingOutputFormat.h`,
`HashedDictionaryParallelLoader.h`, `UniqExactSet.h`, `HashedDictionary.h`,
`StatusRequestsPool.h`
- De-inline `MemoryTrackerSwitcher` ctor/dtor into `MemoryTrackerSwitcher.cpp`
- De-inline `ResourceGuard::Request::wait` into `ResourceGuard.cpp`
- De-inline `ManyAggregatedData` destructor into `AggregatingTransform.cpp`
- De-inline `QueryStatus::updateProgressIn/Out` into `ProcessList.cpp`
- Add explicit `CurrentThread.h` includes to `.cpp` files that relied on
transitive includes through the trimmed headers
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Move the `#if USE_NURAFT` guard into Context, avoiding preprocessor
directives inside `SCOPE_EXIT_SAFE` macro arguments (which is undefined
behavior). This also fixes the FreeBSD build where NuRaft is not
available.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
`tryGetKeeperDispatcher` is only available when NuRaft is enabled,
which is not the case on FreeBSD builds.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
During shutdown, `KeeperTCPHandler` connections run an infinite loop
with no shutdown detection. When the server stops the Keeper TCP
listening socket, existing handler threads keep processing heartbeats
indefinitely. `KeeperDispatcher::shutdown` (which sets the shutdown
flag) is only called after waiting for connections to close, creating
a circular dependency that blocks shutdown until the external timeout
kills the server.
Add a `shutting_down` flag to `KeeperDispatcher` that is set before
waiting for connections. `KeeperTCPHandler` checks this flag after
each `poll` return and exits cleanly when set.
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=98447&sha=79d30b439bc793fc9aa276b28aec105750847880&name_0=PR&name_1=Stateless%20tests%20%28arm_asan%2C%20azure%2C%20parallel%29
Co-Authored-By: Claude Opus 4.6 <[email protected]>
clang-tidy `bugprone-empty-catch` fires on catch blocks with empty
bodies regardless of the exception type. Re-add the NOLINT annotation
on the catch lines that were previously suppressed with `catch (...)`
and now use specific exception types.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
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]>
The previous implementation used `std::call_once` which fired in the
parent process during `BaseDaemon::initialize`. After the watchdog
`fork`, jemalloc state (e.g. `background_thread`) resets in the child
but the `once_flag` remains set, so the verification check saw a
mismatch and fired `chassert`.
Split `setup` and `verifySetup` into separate functions:
- `setup` always applies settings (idempotent), called in
`BaseDaemon::initialize` and re-applied after watchdog fork
- `verifySetup` checks jemalloc state matches server settings,
called from `Server::main` and `Keeper::main`
Also extract config key names into constants in `Jemalloc.h` to keep
them in sync between `BaseDaemon` and `ServerSettings`.
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=98232&sha=b69293471789706abde22ec86f33104dbfaf0382&name_0=PR&name_1=Stateless%20tests%20%28amd_debug%2C%20parallel%29
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]>
