server icon indicating copy to clipboard operation
server copied to clipboard

[MDEV-34614] mysqlbinlog warn on EOF before GTID in --stop-position

Open ParadoxV5 opened this issue 1 year ago • 1 comments
trafficstars

  • [x] The Jira issue number for this PR is: MDEV-34614
  • Follow-up of MDEV-27037
    • Resolves https://github.com/MariaDB/server/pull/3400#discussion_r1681030120

Description

This PR adds mariadb-binlog warnings for --stop-position GTIDs that were not reached at EOF, mainly as a follow-up to MDEV-27037 which added warnings for unreached --stop-position file indices and --stop-datetimes.

What problem is the patch trying to solve?

--stop-position warnings inform possible mistakes in the input; they’re helpful for both DBAs on CLI and scripts/wrappers that check/log the output status.

MDEV-34614 enchances MDEV-27037 with per-domain GTID validation. MDEV-27037 was considered a bug fix and so built upon the oldest supported branch (at the time) for forwardporting, but GTID range selection wasn’t a feature of its ancient base version.

Unlike #3400, the warning implementation is in the Gtid_event_filter family through a new public polymorphic (virtual&override) method verify_completed_state, which provides (sub)filter statuses to stderr without exposing internals such as Domain_gtid_event_filter#m_stop_filters. While it’s uncommon to commit a new public API targeting a non-latest version, this method is hierarchical and is extensible to say --start-position and even --ignore-server-ids (MDEV-20119).

This choice is superior to the previous Binlog_gtid_state_validator design. Note that in --gtid-strict-mode (errors) or -vvv mode (warnings) (only), Binlog_gtid_state_validator is already prompting that all (rather than any) --start-positions are unreached. This may rather be an erroneous side effect (not certain; I’m new 👶).

https://github.com/MariaDB/server/blob/0e8fb977b00983d98c4c35e39bc1f36463095938/client/mysqlbinlog.cc#L1130-L1151

Ideally, the Separation of Concerns would have:

  • Filters preselect which entries (row or transaction depending on the format) are processed.
  • The GTID validator doesn’t care about ranges; it only validates entries (e.g., confirming GITD order) within the range given by the filters.
  • For parity, the program warns out-of-range --start-position/datetimes regardless of --gtid-strict-mode/-vvv, not just --stop-position/datetimes.

Do you think this patch might introduce side-effects in other parts of the server?

Performance: The task of validating --stop-position (and --start-position too, if those’re to be implemented) GTIDs is O(n).

Release Notes

What does the Release Notes say for MDEV-27037?… Nothing? 🤷

Here’s a summary that covers both this and the previous PR:

mariadb-binlog now warns about any --stop-positions and --stop-datetimes that didn’t apply after reading the entire log.

How can this PR be tested?

The new test binlog_mysqlbinlog_warn_stop_gtid covers positive expectations ~~(but not negative testing yet)~~, though QA should run the entire binlog suite to also confirm no regressions.

PR quality check

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • [x] This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.
  • [x] I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • [x] For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

ParadoxV5 avatar Oct 29 '24 21:10 ParadoxV5