Skip to content

PS-11080 fix: Spoiled logical clock information in rewritten binlog (part 4)#129

Open
percona-ysorokin wants to merge 3 commits into
Percona-Lab:0.2from
percona-ysorokin:sequence_number_rewrite_mtr
Open

PS-11080 fix: Spoiled logical clock information in rewritten binlog (part 4)#129
percona-ysorokin wants to merge 3 commits into
Percona-Lab:0.2from
percona-ysorokin:sequence_number_rewrite_mtr

Conversation

@percona-ysorokin
Copy link
Copy Markdown
Collaborator

https://perconadev.atlassian.net/browse/PS-11080

Added MTR test cases for the 'sequence_number' / 'last_committed' field
rewrite logic:

  • 'gtid_renumbering' - checks for renumbering when remote rotation occurs.
  • 'gtid_renumbering_local_rotation' - checks for renumbering when local rotation
    occurs.
  • 'gtid_renumbering_resume' - checks that 'last_sequence_number' is restored
    properly from the binlog metadata file upon PBS restart.
  • 'gtid_renumbering_resume_after_partial' - checks that 'last_sequence_number'
    in the binlog metadata file always corresponds to the actual data file (not the
    last seen value currently in the storage buffer). This helps with resuming PBS
    after the crash.

Currently 'gtid_renumbering_resume_after_partial' MTR test case requires
DEBUG_SYNC functionality to be available in the MySQL Server. Unfortunately,
at the moment we use release tarballs of the MySQL Server in GitHub Actions
workers and this test will always be skipped there.

kamil-holubicki and others added 3 commits May 21, 2026 23:48
…part 2)

https://perconadev.atlassian.net/browse/PS-11080

This is a prerequisite commit required to implement rewriting
'sequence_number' / 'last_committed' / 'transaction_length' in GTID events.

Along with last seen transaction GTID, 'binstv::events::reader_context' now also
keeps track of the last seen GTID event sequence_number.

At the same time 'binsrv::storage' expects one extra
'transaction_sequence_number' argument passed to its 'write_event()' method
(which is expected to be taken from the
'binstv::events::reader_context::get_transaction_sequence_number()'). It
implements the same logic of storing transaction's sequence_number for both
"ready to be flushed" buffered event data and for the "incomplete transaction"
buffered event data as for 'timestamps' and 'gtids'.

Binlog metadata file extended with one more value 'last_sequence_number' that
holds the 'sequence_number' value of the last GTID event (either GTID_LOG,
ANONYMOUS_GTID_LOG or GTID_TAGGED_LOG events) written to the binlog file.

Co-authored-by: Yura Sorokin <yura.sorokin@percona.com>
…part 3)

https://perconadev.atlassian.net/browse/PS-11080

Implemented generic event rewriting mechanism
('binsrv::events::rewriter::rewrite()' static method) that performs the following
operations.
* For GTID_LOG /ANONYMOUS_GTID_LOG it changes 'sequence_number' /
  'last_committed' fields in the post header and relocates the event (changes
  'next_event_position' in the common header).
* For GTID_TAGGED_LOG it changes 'sequence_number' / 'last_committed' in the
  serializer-encoded event body, which because of the variable length encoding
  of individual elements in the archive may change its size and may require
  updating 'transaction_length' in the event body and 'event_size' in the common
  header. In addition, it also performs event relocation.
* For every other event it simply performs event relocation.
All these changes are performed under a special guard
('binsrv::events::event_updatable_view::write_proxy') which guarantees that
the event checksum will be recalculated / added upon finalizing all field value
updates. We also make sure that all rewritten events will have a footer with
properly calculated checksum (event if it was not present in the original event).

The logic for updating 'sequence_number' /  'last_committed' in GTID events
is encapsulated inside ('fix_sequence_number_and_last_committed()' function).

Refactored the way how several event fields can be modified in a single run via
'event_updatable_view' with guarantee that event's checksum will be properly
recalculated. Introduced 'rewriter::generic_materialize()' template function that
accepts generic field modification functor. Also introduced two simplified helper
functions (which use the generic one underneath) for the most typical cases:
* 'rewriter::materialize()'
* 'rewriter::materialize_and_relocate()'

Co-authored-by: Kamil Holubicki <kamil.holubicki@percona.com>
…part 4)

https://perconadev.atlassian.net/browse/PS-11080

Added MTR test cases for the 'sequence_number' / 'last_committed' field
rewrite logic:
* 'gtid_renumbering' - checks for renumbering when remote rotation occurs.
* 'gtid_renumbering_local_rotation' - checks for renumbering when local rotation
  occurs.
* 'gtid_renumbering_resume' - checks that 'last_sequence_number' is restored
  properly from the binlog metadata file upon PBS restart.
* 'gtid_renumbering_resume_after_partial' - checks that 'last_sequence_number'
  in the binlog metadata file always corresponds to the actual data file (not the
  last seen value currently in the storage buffer). This helps with resuming PBS
  after the crash.

Currently 'gtid_renumbering_resume_after_partial' MTR test case requires
DEBUG_SYNC functionality to be available in the MySQL Server. Unfortunately,
at the moment we use release tarballs of the MySQL Server in GitHub Actions
workers and this test will always be skipped there.

Co-authored-by: Yura Sorokin <yura.sorokin@percona.com>
Comment thread src/binsrv/storage.hpp
}

[[nodiscard]] events::seq_no_t
get_current_transaction_sequence_number() const noexcept {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename it to get_last_transaction_sequence_number()?
Rationale: it is used as the parameter to rewriter (next phase commits), and it makes a bit confusion, because the parameter says about what we already seen, and the rewriter calculates the real CURRENT transaction seqno as parameter + 1 (`storage.get_current_transaction_sequence_number() + 1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants