Skip to content

MDEV-20122: Remove Master_info::is_demotion#5115

Open
ParadoxV5 wants to merge 1 commit into
mainfrom
px5.is_demotion
Open

MDEV-20122: Remove Master_info::is_demotion#5115
ParadoxV5 wants to merge 1 commit into
mainfrom
px5.is_demotion

Conversation

@ParadoxV5
Copy link
Copy Markdown
Contributor

MDEV-20122 introduced this field, but it was never actually used.
CHANGE MASTER TO master_demote_to_slave=… only uses the parser field LEX_MASTER_INFO::is_demotion_opt and is not recorded in either @@master_info_file or @@relay_log_info_file, as it takes effect during CHANGE MASTER rather than during replication.

MDEV-20122 introduced this field, but it was never actually used.
`CHANGE MASTER TO master_demote_to_slave=…` only uses the parser
field `LEX_MASTER_INFO::is_demotion_opt` and is not recorded in
either `@@master_info_file` or `@@relay_log_info_file`,
as it takes effect during CHANGE MASTER rather than during replication.
@ParadoxV5 ParadoxV5 requested a review from bnestere May 24, 2026 01:00
@ParadoxV5 ParadoxV5 added MariaDB Corporation Replication Patches involved in replication labels May 24, 2026
Copy link
Copy Markdown

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

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 relocates the demotion flag from the Master_info class to the LEX_MASTER_INFO struct. Feedback was provided to improve the accuracy of the documentation by using 'server' instead of 'connection' and to align the comment style with existing codebase conventions regarding capitalization and formatting.

Comment thread sql/sql_lex.h
Comment on lines +374 to +377
When `true`, transition this connection from an active master to a slave.
This updates the replication state to account for any transactions which
were committed into the binary log. In particular, it merges
`gtid_binlog_pos` into `gtid_slave_pos`.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The term "connection" is slightly inaccurate in this context. While the demotion is initiated through a specific replication connection (channel), the operation transitions the server's role and affects the global GTID state. The original wording "transition this server" is more precise. Additionally, using standard capitalization for TRUE and avoiding backticks in the comment text maintains better consistency with the existing codebase style.

    When TRUE, transition this server from being an active master to a slave.
    This updates the replication state to account for any transactions which
    were committed into the binary log. In particular, it merges
    gtid_binlog_pos into gtid_slave_pos.
References
  1. Use standard capitalization for TRUE and avoid backticks in comments for consistency with the codebase style. (link)

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