Repository navigation
PBS-42 feature: Implement basic support for COM_BINLOG_DUMP packet handling (part 5) - #201
Conversation
4fd6a04 to
ef990dd
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
EOF resume handling can omit startup events or advertise the previous file’s offset after switching binlogs.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds position-based resume support to the binlog replication listener for PBS-42.
Changes:
- Introduces sender state handling and checksum-aware startup events.
- Extends event-generation and timestamp helpers.
- Expands roundtrip tests with resumed dumps and direct MySQL comparisons.
| File | Description |
|---|---|
| src/util/flag_set.hpp | Adds flag clearing. |
| src/util/common_optional_types.hpp | Adds optional boolean alias. |
| src/operations/sender_context.hpp | Declares resume state and helpers. |
| src/operations/sender_context.cpp | Implements position-based streaming states. |
| src/operations/event_generation_helpers.hpp | Extends event-generation interfaces. |
| src/operations/event_generation_helpers.cpp | Supports explicit event positions and checksums. |
| src/operations/collector_context.cpp | Adapts event-generation calls. |
| src/minimysql/network_service.cpp | Passes requested position and checksum settings. |
| src/binsrv/storage_core.cpp | Corrects a comment typo. |
| src/binsrv/events/format_description_post_header_impl.hpp | Declares timestamp setters. |
| src/binsrv/events/format_description_post_header_impl.cpp | Implements timestamp setters. |
| src/binsrv/events/composite_binlog_name.cpp | Accepts empty binlog names. |
| src/binsrv/events/common_header.cpp | Handles artificial FORMAT_DESCRIPTION headers. |
| src/binsrv/basic_storage_backend.hpp | Corrects a comment typo. |
| README.md | Corrects a documentation typo. |
| mtr/binlog_streaming/t/rs_roundtrip.test | Sets checkpoint size. |
| mtr/binlog_streaming/t/rs_roundtrip.combinations | Adds checksum and server-ID combinations. |
| mtr/binlog_streaming/t/rs_roundtrip_encryption.combinations | Adds equivalent encrypted-test combinations. |
| mtr/binlog_streaming/t/auth_method_switch.test | Uses configured authentication credentials. |
| mtr/binlog_streaming/r/rs_roundtrip.result | Updates expected roundtrip output. |
| mtr/binlog_streaming/r/rs_roundtrip_encryption.result | Updates expected encrypted-roundtrip output. |
| mtr/binlog_streaming/include/rs_roundtrip_body.inc | Tests resumed dumps, comparisons, and restoration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const auto populate_result{populate_event_block(event)}; | ||
| if (populate_result.has_value()) { | ||
| return *populate_result; | ||
| } |
| logger_->log_format(binsrv::log_severity::info, | ||
| "sender : switched to a new binlog file {} -> {}", | ||
| saved_binlog_name.str(), binlog_name_.str()); | ||
| fsm_state_ = fsm_state_type::start_from_beginning; |
…ndling (part 5) https://perconadev.atlassian.net/browse/PBS-42 Implemented "resume from the specified position" logic for position- based replication in the listening mode. Network service extended with handling SET @source_binlog_checksum = 'CRC32', @master_binlog_checksum = 'CRC32'; and SET @source_binlog_checksum = 'NONE', @master_binlog_checksum = 'NONE' statements so that after switching to replication we would know whether the very first artificial ROTATE event generated by us needs to include checksum or not. By default this information should be taken from the last seen / sent FORMAT_DESCRIPTION event, but the very first artificial ROTATE event is a special case. Reworked 'operations::sender_context' class - it now operates as a state machine. In addition, instead of generating absolutely context unaware artificial ROTATE and FORMAT_DESCRIPTION events, we now always fetch them from the real data files and perform only necessary field changes before sending them back to the replication channel. Reworked event generation helpers: they now have a lower level generate_xxx_event_ex() counterpart functions that do not require an instance of a 'binsrv::events::reader_context' class. Added new 'clear_element()' method to the 'util::flag_set<>' class template. 'binlog_streaming.rs_roundtrip' and 'binlog_streaming.rs_roundtrip_encryption' MTR test cases extended with one more 'mysqlbinlog' run that extracts new set of events from the specified binlog file position after they were generated on the server. In addition, not only do we try to restore the dump files generated by the 'mysqlbinlog' and check that the restored data is identical to the one generated directly, we now also perform event-to-event comparison between dumps generated from the PBS and directly from the MySQL Server. These test cases are also run with 4 combinations now: * binlog event checksums enabled / disabled on the server; * server ID left to default / set to custom. Fixed some typos in the code comments.
7c9b356 to
ea380c9
Compare
kamil-holubicki
left a comment
There was a problem hiding this comment.
LGTM.
Two minor things
| set_create_timestamp_raw( | ||
| static_cast<std::uint32_t>(create_timestamp.get_value())); | ||
| } | ||
| void generic_post_header_impl<code_type::format_description>:: |
There was a problem hiding this comment.
It seems to be dead code (unused method).
As a consequence, set_create_timestamp is a dead code as well.
There was a problem hiding this comment.
Were added just for symmetry reasons with other event cstructures.
|
|
||
| --echo | ||
| --echo *** Inserting the third batch of rows. | ||
| INSERT INTO source_db.tbl VALUES (7, 'seven'), (8, 'eight'), (9, 'none'); |
There was a problem hiding this comment.
Multiple occurrences:
9, none - is that the intention? Shouldn't it be 9, nine?
…ndling (part 6) https://perconadev.atlassian.net/browse/PBS-42 Added new 'binlog_streaming.rs_position_based_consistency' MTR test case. It generates several binlog files (including an empty one), records (binlog name, position) checkpoints in a JSON session variable after each step, and also adds ["":4] and the end-of-file position of every binlog file. For each checkpoint 'mysqlbinlog --to-last-log' is run both against the PBS and directly against the MySQL Server and the generated event sequences are compared. The test case is run with the same 4 combinations as 'binlog_streaming.rs_roundtrip'. Extracted dump-and-compare logic into the new 'rs_compare_dumps.inc' include file and reused it in 'rs_roundtrip_body.inc' (shared by 'binlog_streaming.rs_roundtrip' and 'binlog_streaming.rs_roundtrip_encryption'). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ea380c9 to
fad1d58
Compare

https://perconadev.atlassian.net/browse/PBS-42
Implemented "resume from the specified position" logic for position- based replication in the listening mode.
Network service extended with handling
SET @source_binlog_checksum = 'CRC32',
@master_binlog_checksum = 'CRC32';
and
SET @source_binlog_checksum = 'NONE',
@master_binlog_checksum = 'NONE'
statements so that after switching to replication we would know whether the very first artificial ROTATE event generated by us needs to include checksum or not. By default this information should be taken from the last seen / sent FORMAT_DESCRIPTION event, but the very first artificial ROTATE event is a special case.
Reworked 'operations::sender_context' class - it now operates as a state machine. In addition, instead of generating absolutely context unaware artificial ROTATE and FORMAT_DESCRIPTION events, we now always fetch them from the real data files and perform only necessary field changes before sending them back to the replication channel.
Reworked event generation helpers: they now have a lower level generate_xxx_event_ex() counterpart functions that do not require an instance of a 'binsrv::events::reader_context' class.
Added new 'clear_element()' method to the 'util::flag_set<>' class template.
'binlog_streaming.rs_roundtrip' and
'binlog_streaming.rs_roundtrip_encryption' MTR test cases extended with one more 'mysqlbinlog' run that extracts new set of events from the specified binlog file position after they were generated on the server. In addition, not only do we try to restore the dump files generated by the 'mysqlbinlog' and check that the restored data is identical to the one generated directly, we now also perform event-to-event comparison between dumps generated from the PBS and directly from the MySQL Server. These test cases are also run with 4 combinations now:
Fixed some typos in the code comments.