Make log level checks lock-free via permanent loggers. - #5349
Merged
Conversation
dmkozh
force-pushed
the
logging_opt
branch
2 times, most recently
from
July 14, 2026 23:29
1165292 to
6b85eb5
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Makes disabled log-level checks lock-free by retaining permanent logger instances and reconfiguring their sinks.
Changes:
- Adds permanent per-partition loggers with mutable distribution sinks.
- Returns non-owning logger pointers to avoid shared-pointer copies.
- Enables atomic runtime log-level updates.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/util/SpdlogTweaks.h |
Enables atomic logger levels. |
src/util/Logging.h |
Documents permanent loggers and updates getter APIs. |
src/util/Logging.cpp |
Implements permanent loggers and sink reconfiguration. |
Every CLOG_* invocation fetches its respective logger instance in order to check if we should log this line, and the getter used to be guarded by a recursive mutex lock. So we kept serializing every disabled log statement, which hindered parallelization for the code paths with frequent trace/debug logs. Now each partition has a single permanent logger, created on first use and never destroyed or replaced (logging rotations/reconfigurations now are done at the sink level, instead of the logger level). Additionally, we avoid a shared ptr copy on every logger access, as we now we can just safely return a non-owning pointer to it. The logging level checks per logger still have to be thread-safe, as the level may be changed in the middle of the app lifetime. We remove SPDLOG_NO_ATOMIC_LEVELS to achieve that, which simply uses very fast relaxed atomic checks.
dmkozh
enabled auto-merge
July 22, 2026 21:55
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Jul 23, 2026
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
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.
Description
Make log level checks lock-free via permanent loggers.
Every CLOG_* invocation fetches its respective logger instance in order to check if we should log this line, and the getter used to be guarded by a recursive mutex lock. So we kept serializing every disabled log statement, which hindered parallelization for the code paths with frequent trace/debug logs.
Now each partition has a single permanent logger, created on first use and never destroyed or replaced (logging rotations/reconfigurations now are done at the sink level, instead of the logger level). Additionally, we avoid a shared ptr copy on every logger access, as we now we can just safely return a non-owning pointer to it.
The logging level checks per logger still have to be thread-safe, as the level may be changed in the middle of the app lifetime. We remove SPDLOG_NO_ATOMIC_LEVELS to achieve that, which simply uses very fast relaxed atomic checks.
This doesn't have too large impact right now (~-3ms on the local benchmarks), but it will become more meaningful as we land more apply path parallelization.
Checklist
clang-formatv8.0.0 (viamake formator the Visual Studio extension)