Skip to content

MDEV-39788: Fix master.info not upgrading master_use_gtid#5301

Open
ParadoxV5 wants to merge 3 commits into
12.3from
MDEV-39788
Open

MDEV-39788: Fix master.info not upgrading master_use_gtid#5301
ParadoxV5 wants to merge 3 commits into
12.3from
MDEV-39788

Conversation

@ParadoxV5

@ParadoxV5 ParadoxV5 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Note

This PR is based on #5147, which provides the regression test.

The line-count line in master.info and relay-log.info has been inconsistent (off by one) since its introduction.
MDEV-37530 corrected this by changing master.info to use relay-log.info’s line-count definition by chance, but at the cost of an upgrade incompatibility.
It now reads one more line beyond the pre-upgraded file’s line-based section, which for MariaDB 10.0–12.2 means an upgrade doesn’t carry over the first option stored in key=value, master_use_gtid.

Since older versions can read the new format in a downgrade, rather than encouraging the old inconsistency, this commit focuses on the upgrade problem by adding a shim entry to master.info’s list to emulate prior versions’ reading behaviour.
Although this implementation can only restore compatibility with versions 10.0+, versions before MariaDB 10 have long been EOL.

While here, this commit also fixes code and comments that contradict the actual effect.

MDEV-39788 found that the recent refactor on the `main` (now 12.3)
branch missed the (inconsistent) detail that, unlike `@@relay_log_info`,
`@@master_info`’s line count _includes_ the line-count line itself.

This commit extends and simplifies the test
`rpl.rpl_read_new_relay_log_info` to `main.rpl_read_new_info` so it
* Checks this detail to remind future changes of this type of mistake.
* Covers `@@master_info` as well.

While here, this commit also includes a new-format version
of MDEV-38020’s test to double as the value read check.
The line-count line in `master.info` and `relay-log.info`
has been inconsistent (off by one) since its introduction.
MDEV-37530 corrected this by changing `master.info`
to use `relay-log.info`’s line-count definition by
chance, but at the cost of an upgrade incompatibility.
It now reads one more line beyond the pre-upgraded file’s line-based
section, which for MariaDB 10.0–12.2 means an upgrade doesn’t carry
over the first option stored in `key=value`, `master_use_gtid`.

Since older versions can read the new format in a downgrade,
rather than encouraging the old inconsistency,
this commit focuses on the upgrade problem by adding a shim entry to
`master.info`’s list to emulate prior versions’ reading behaviour.
Although this implementation can only restore compatibility with
versions 10.0+, versions before MariaDB 10 have long been EOL.

While here, this commit also fixes code and
comments that contradict the actual effect.
@ParadoxV5 ParadoxV5 requested a review from bnestere June 30, 2026 00:25
@ParadoxV5 ParadoxV5 added MariaDB Corporation Replication Patches involved in replication labels Jun 30, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request addresses compatibility issues in parsing *.info files (specifically master.info and relay-log.info) from older MariaDB/MySQL versions, ensuring that line counts and position values greater than 2^31 are handled correctly. It introduces a LINE_COUNT_FIX pseudo-value in Master_info_file to restore compatibility with files generated by MariaDB 10.0+ and updates the padding and saving logic in Info_file. Additionally, new tests are added to verify these changes. The review feedback suggests naming the unnamed struct LINE_COUNT_FIX to Line_count_fix_t to improve readability and prevent potential compiler-specific issues, along with a minor typo correction in the documentation.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +543 to +559
protected:
/**
@ref Master_info_file has been inconsistent that its line count included
the line-count "value", while @ref Relay_log_info_file's excludes it.
Although the use common code now makes @ref Master_info_file match,
files from older versions are now off-by-one to Info_file::load_from_file().
The presence of this non-reading pseudo-value restores
compatibility with files generated by MariaDB 10.0+,
as the driver loop will increment the line counter for it,
thus emulating counting the line-count "value".
@deprecated This is not needed after all pre-12.3 versions go EoL.
*/
struct: Persistent
{
bool load_from(IO_CACHE *file) override { return false; } ///< No-op
void save_to(IO_CACHE *file) override {} ///< No-op
} LINE_COUNT_FIX;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Naming the unnamed struct Line_count_fix_t improves code readability and avoids potential compiler-specific issues with unnamed classes overriding virtual functions or being used in template-based pointer-to-member wrappers. Additionally, there is a minor typo in the comment: 'use common code' should be 'use of common code'.

protected:
  /**
    @ref Master_info_file has been inconsistent that its line count included
    the line-count "value", while @ref Relay_log_info_file's excludes it.
    Although the use of common code now makes @ref Master_info_file match,
    files from older versions are now off-by-one to Info_file::load_from_file().
    The presence of this non-reading pseudo-value restores
    compatibility with files generated by MariaDB 10.0+,
    as the driver loop will increment the line counter for it,
    thus emulating counting the line-count "value".
    @deprecated This is not needed after all pre-12.3 versions go EoL.
  */
  struct Line_count_fix_t : Persistent
  {
    bool load_from(IO_CACHE *file) override { return false; } ///< No-op
    void save_to(IO_CACHE *file) override {} ///< No-op
  } LINE_COUNT_FIX;

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

Labels

MariaDB Corporation Replication Patches involved in replication

Development

Successfully merging this pull request may close these issues.

1 participant