Support Startup & Shutdown Log Levels#85967
Merged
kssenii merged 5 commits intoClickHouse:masterfrom Aug 28, 2025
Merged
Conversation
Contributor
|
Workflow [PR], commit [7e4d800] Summary: ❌
|
Contributor
Author
|
Example log (from running locally) with: |
Blokje5
force-pushed
the
support-startup-and-shutdown-log-levels
branch
2 times, most recently
from
717b939 to
2046590
Compare
kssenii
added
the
can be tested
Member
|
Could you please write an integration test for this feature? See |
Blokje5
force-pushed
the
support-startup-and-shutdown-log-levels
branch
3 times, most recently
from
880d26b to
4b62bf3
Compare
Contributor
Author
|
The test seems flaky although I haven't yet figured out why. @kssenii do you have any suggestions for debugging? I can't see the Clickhouse logs from the test results. It could be shutdown is too fast and the test is unable to get the shutdown logs. |
Member
Logs pop up when you press on |
Member
|
Contributor
Author
|
@kssenii I meant the logs of the Clickhouse binary itself. Indeed the tests logs are clear on why it failed. I'll take a look at the tests today. |
Member
|
Logs of clickhouse server are also there, inside integration_run_flaky0_0.tar.zst |
Blokje5
force-pushed
the
support-startup-and-shutdown-log-levels
branch
3 times, most recently
from
074f289 to
42f8635
Compare
Contributor
Author
|
Ok the issue was indeed the cluster.shutdown call killing clickhouse. I now trigger a SIGTERM instead so there is time to wait for the shutdown. Last test failed due to [UPDATE] |
Blokje5
force-pushed
the
support-startup-and-shutdown-log-levels
branch
from
42f8635 to
a4d7019
Compare
Alter the log levels on startup & shutdown if the respective values are set. This allows for a higher log level during startup & shutdown of clickhouse. Fixes: ClickHouse#85958
Blokje5
force-pushed
the
support-startup-and-shutdown-log-levels
branch
from
a4d7019 to
315ade5
Compare
Contributor
Author
|
@kssenii I decided to remove the shutdown check, it caused stability issues with the test. The test still checks startup + revert of log level to configured log level after startup. Let me know if you want me to add anything else to the PR. Otherwise this PR is ready for review. |
Member
e.g. the test was flaky? |
Contributor
Author
Yeah with the shutdown it became flaky. And I suspect it was more related to the shutdown & restart of CH inside of the container rather then the test case per-se. I got connection errors on the previous PR run. |
Member
As it was force-pushed, I cannot see what you tried to use, but I do not see why would it not work. Also I personally do not like to use |
Contributor
Author
Ah that is a good point, it would have been better to separate the commits. I wasn't aware wait_for_log_line would wait for calls after the call is made, that probably explains why the check was flaky in the first place. I have pushed a change with
No it shouldn't |
kssenii
reviewed
Aug 25, 2025
Comment on lines
+35
to
+37
| instance.exec_in_container( | ||
| ["bash", "-c", "pkill -{} clickhouse".format(signal)], user="root" | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Yes, that is probably cleaner.
Contributor
Author
|
@kssenii the tests now pass 🎉 I just a fuzzer failure (probably not related to this PR based on what I could see) & a failure in the Finish Workflow: Do you know where I can add the relevant docs? Or are the comments in config.xml sufficient? |
kssenii
approved these changes
Aug 26, 2025
tommyzli
reviewed
Aug 26, 2025
programs/server/Server.cpp
Outdated
| { | ||
| /// Set the root logger level to the shutdown level. | ||
| /// This is useful for debugging shutdown issues. | ||
| /// The root logger level will be reset to the default level after the server is fully initialized. |
There was a problem hiding this comment.
nit: this comment doesn't make sense here
auto-merge was automatically disabled
August 27, 2025 07:17
Head branch was pushed to by a user without write access
Contributor
Author
|
@kssenii I guess the push I did to update the comment disabled the auto-merge? |
Merged
via the queue into
ClickHouse:master
with commit Aug 28, 2025
19c8bb4
121 of 122 checks passed
robot-ch-test-poll4
added
the
pr-synced-to-cloud
fm4v
reviewed
Sep 9, 2025
| <clickhouse> | ||
| <logger> | ||
| <level>none</level> | ||
| <startupLevel>trace</startupLevel> |
Member
There was a problem hiding this comment.
@Blokje5 , configuration keys should be snake_cased, not camelCased. Please update this
fm4v
reviewed
Sep 9, 2025
| | `async` | When `true` (default) logging will happen asynchronously (one background thread per output channel). Otherwise it will log inside the thread calling LOG | | ||
| | `async_queue_max_size` | When using async logging, the max amount of messages that will be kept in the the queue waiting for flushing. Extra messages will be dropped | | ||
| | `startupLevel` | Startup level is used to set the root logger level at server startup. After startup log level is reverted to the `level` setting | | ||
| | `shutdownLevel` | Shutdown level is used to set the root logger level at server Shutdown. | |
1 task
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alter the log levels on startup & shutdown if the respective values are set. This allows for a higher log level during startup & shutdown of clickhouse.
Fixes: #85958
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
New configuration options:
logger.startupLevel&logger.shutdownLevelallow for overriding the log level during the startup & shutdown of Clickhouse respectively.This can be useful to help investigate startup & shutdown issues.
Documentation entry for user-facing changes