Skip to content
/ server Public

MDEV-36841 Warn when SET slave_net_timeout drops below master_heartbeat_period#4845

Open
tonychen2001 wants to merge 1 commit intoMariaDB:12.3from
tonychen2001:MDEV-36841
Open

MDEV-36841 Warn when SET slave_net_timeout drops below master_heartbeat_period#4845
tonychen2001 wants to merge 1 commit intoMariaDB:12.3from
tonychen2001:MDEV-36841

Conversation

@tonychen2001
Copy link
Copy Markdown
Contributor

Description

When CHANGE MASTER sets master_heartbeat_period to a value greater than
@@GLOBAL.slave_net_timeout, a ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE_MAX
warning is emitted. However, the reverse scenario, lowering
slave_net_timeout below an existing master_heartbeat_period, produced
no warning.

Add an on_update callback for slave_net_timeout that iterates all
Master_info entries and warns if any resolved heartbeat period exceeds
the new timeout value.

Release Notes

N/A

How can this PR be tested?

The MTRs rpl.rpl_heartbeat_basic and rpl.rpl_heartbeat have been updated.

Basing the PR against the correct MariaDB version

  • This is a bug fix, and the PR is based against the branch 12.3.

Copyright

All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.

@gkodinov gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Mar 24, 2026
Copy link
Copy Markdown
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! This is a preliminary review.

LGTM, but please fix the test failures.

my_hash_element(&master_info_index->master_info_hash, i);
if ((uint32_t) mi->master_heartbeat_period > timeout * 1000ULL)
{
push_warning(thd, Sql_condition::WARN_LEVEL_WARN,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd put a connection name in the message.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the feedback. I agree that the current warning message is not very user friendly so I wanted to gather some thoughts on the warning we want to push.

Option 1 (consolidate all replication connection names into 1 warning message):

Warnings:
Warning	1704	The heartbeat period for replication connection(s) '', 'connection1', 'connection2' exceeds the requested value of `slave_net_timeout' seconds. A sensible value for heartbeat periods should be less than the timeout.

Option 2 (separate warning for each connection):

Warnings:
Warning	1704	The heartbeat period for replication connection '' exceeds the requested value of `slave_net_timeout' seconds. A sensible value for the period should be less than the timeout.
Warning	1704	The heartbeat period for replication connection 'connection2' exceeds the requested value of `slave_net_timeout' seconds. A sensible value for the period should be less than the timeout.

I assume we can re-use the 1704 error code here.

Please let me know your thoughts.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd go with the latter.

…at_period

When CHANGE MASTER sets master_heartbeat_period to a value greater than
@@GLOBAL.slave_net_timeout, a ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE_MAX
warning is emitted. However, the reverse scenario, lowering
slave_net_timeout below an existing master_heartbeat_period,  produced
no warning.

Add an on_update callback for slave_net_timeout that iterates all
Master_info entries and warns if any resolved heartbeat period exceeds
the new timeout value.

All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.
Copy link
Copy Markdown
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

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

LGTM. Please stand by for the final review.

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

Labels

External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.

Development

Successfully merging this pull request may close these issues.

3 participants