Repository navigation
PBS-37 feature: Combine binlog_server with minimysql_server into a single executable (part 6) - #184
Merged
percona-ysorokin merged 1 commit intoSep 17, 2026
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The implementation is coherent, with only minor log-message and documentation corrections remaining.
Pull request overview
Adds thread-safe centralized logging across storage, replication, and MiniMySQL networking.
Changes:
- Serializes logger output and adds
std::format-based logging. - Routes MiniMySQL diagnostics through
basic_logger. - Adds
null_loggerfor non-fetch storage operations.
File summaries
| File | Description |
|---|---|
CMakeLists.txt |
Registers null_logger. |
src/binsrv/basic_logger.cpp |
Serializes output calls. |
src/binsrv/basic_logger.hpp |
Adds atomic levels and formatted logging. |
src/binsrv/exception_handling_helpers.cpp |
Uses formatted exception logs. |
src/binsrv/null_logger.hpp |
Adds no-op logger. |
src/binsrv/storage.cpp |
Requires and directly uses a logger. |
src/binsrv/storage.hpp |
Removes obsolete logging helper. |
src/minimysql/connection_context.cpp |
Renames auth-switch parser. |
src/minimysql/connection_context.hpp |
Exposes last sequence number. |
src/minimysql/network_service.cpp |
Routes network diagnostics through logging. |
src/minimysql/network_service.hpp |
Injects and retains a logger. |
src/operations/collector_context.cpp |
Converts messages to formatted logging. |
src/operations/list_operation.cpp |
Uses null_logger. |
src/operations/logger_helpers.cpp |
Simplifies message formatting. |
src/operations/pull_operation.cpp |
Shares logger with network service. |
src/operations/purge_binlogs_operation.cpp |
Uses null_logger. |
src/operations/search_by_gtid_set_operation.cpp |
Uses null_logger. |
src/operations/search_by_timestamp_operation.cpp |
Uses null_logger. |
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| class [[nodiscard]] null_logger final : public basic_logger { | ||
| public: | ||
| // 'delimiter' is the highest severity, so even message formatting is skipped |
| << " max_packet_size : " << context.get_client_max_packet_size() | ||
| << '\n'; | ||
| logger.log_format(binsrv::log_severity::info, | ||
| "net : parsed client auth method switch {}", |
percona-ysorokin
force-pushed
the
combined_binary_logger
branch
2 times, most recently
from
September 16, 2026 13:35
a6787f0 to
adde08a
Compare
…ngle executable (part 6) https://perconadev.atlassian.net/browse/PBS-37 Reworked 'binsrv::basic_logger' class to support multi-threading. 'binsrv::basic_logger' class now includes a mutex that serializes calls to 'do_log()' method. Also, added new 'log_format()' method which accepts variadic parameters identical to 'std::format()' for easier message composition. A number of calls to 'binsrv::basic_logger::log()' which were preceded by manual log message composition changed to 'log_format()'. 'minimysql::network_service' class reworked to use 'binsrv::basic_logger' instead of direct prints to 'std::cout' / 'std::cerr'. Added new no-op implementation of the 'binsrv::basic_logger' called 'binsrv::null_logger' that simply does nothing in 'do_print()' method. Operations that were not involved in fetching events ('list', 'search_by_timestamp', 'search_by_gtid_set', and 'purge_binlogs') now use an instance of this 'binsrv::null_logger' as a logger. Fixed 'binlog_streaming.auth_method_switch' MTR test case as network events are now logged into the same log file as the rest of PBS events. Improved 'log_span_dump()' logger helper function - now it generates hex dump strings only when the log level is set to 'trace'. Improved 'generate_binsrv_config.inc' MTR include file - it is now possible to specify the log level which will be set in the '<logger.level>' configuration parameter. If not specified, the default value 'trace' will be used. As 'binlog_streaming.checkpointing' MTR test case operates with a big number of binlog events, the log file generated by the binlog server utility can become quite large and can cause really long execution time under Valgrind. Fixed by reducing log level for this test case to 'info'.
percona-ysorokin
force-pushed
the
combined_binary_logger
branch
from
September 17, 2026 00:39
225bb41 to
a640f85
Compare
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.
https://perconadev.atlassian.net/browse/PBS-37
Reworked 'binsrv::basic_logger' class to support multi-threading.
'binsrv::basic_logger' class now includes a mutex that serializes calls
to 'do_log()' method. Also, added new 'log_format()' method which
accepts variadic parameters identical to 'std::format()' for easier
message composition.
A number of calls to 'binsrv::basic_logger::log()' which were preceded
by manual log message composition changed to 'log_format()'.
'minimysql::network_service' class reworked to use
'binsrv::basic_logger' instead of direct prints to 'std::cout' /
'std::cerr'.
Added new no-op implementation of the 'binsrv::basic_logger' called
'binsrv::null_logger' that simply does nothing in 'do_print()' method.
Operations that were not involved in fetching events ('list',
'search_by_timestamp', 'search_by_gtid_set', and 'purge_binlogs') now
use an instance of this 'binsrv::null_logger' as a logger.
Fixed 'binlog_streaming.auth_method_switch' MTR test case as network
events are now logged into the same log file as the rest of PBS events.
Improved 'log_span_dump()' logger helper function - now it generates hex
dump strings only when the log level is set to 'trace'.
Improved 'generate_binsrv_config.inc' MTR include file - it is now
possible to specify the log level which will be set in the
'<logger.level>' configuration parameter. If not specified, the default
value 'trace' will be used.
As 'binlog_streaming.checkpointing' MTR test case operates with a big
number of binlog events, the log file generated by the binlog server
utility can become quite large and can cause really long execution time
under Valgrind. Fixed by reducing log level for this test case to
'info'.