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]>
