Skip to content

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

Merged
gkodinov merged 1 commit into
MariaDB:mainfrom
tonychen2001:MDEV-36841
Oct 21, 2026
Merged

gkodinov merged 1 commit into
MariaDB:mainfrom
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

@gkodinov gkodinov left a comment

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.

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

LGTM, but please fix the test failures.

Comment thread mysql-test/suite/rpl/r/rpl_heartbeat.result
Comment thread sql/sys_vars.cc Outdated

@gkodinov gkodinov left a comment

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.

LGTM. Please stand by for the final review.

@gkodinov
gkodinov requested a review from bnestere March 25, 2026 08:19
@bnestere bnestere added the Replication Patches involved in replication label Apr 21, 2026

@bnestere bnestere 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.

Thank you for your contribution, @tonychen2001 !

I've left a couple notes.

Comment thread sql/sys_vars.cc Outdated
Comment thread mysql-test/suite/rpl/r/rpl_heartbeat_basic.result Outdated
@tonychen2001
tonychen2001 force-pushed the MDEV-36841 branch 3 times, most recently from 8389993 to 1678744 Compare June 12, 2026 00:36
@ParadoxV5
ParadoxV5 requested a review from bnestere June 12, 2026 03:53

@bnestere bnestere 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.

Thanks @tonychen2001 , this looks good.

Though please rebase to the main branch, rather than 12.3, as it adds a new warning.

Sorry for such a slow turn-around on the review :(

@CLAassistant

CLAassistant commented Oct 16, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@tonychen2001
tonychen2001 changed the base branch from 12.3 to main October 16, 2026 23:26
@tonychen2001

Copy link
Copy Markdown
Contributor Author

Thank you @bnestere , I've updated the target to main.

@gkodinov gkodinov left a comment •

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.

There seems to be compilation failure (see below). Can you please attend to the buildbot failures?

/home/buildbot/aarch64-debian-11/build/sql/sys_vars.cc: In function ‘bool check_slave_net_timeout(sys_var*, THD*, set_var*)’:
/home/buildbot/aarch64-debian-11/build/sql/sys_vars.cc:6169:11: error: invalid cast from type ‘Master_info_file::Heartbeat_period_value’ to type ‘uint32_t’ {aka ‘unsigned int’}
 6169 |           (uint32_t) mi->master_heartbeat_period <= timeout * 1000ULL)
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/buildbot/aarch64-debian-11/build/sql/sys_vars.cc:6178:28: error: invalid cast from type ‘Master_info_file::Heartbeat_period_value’ to type ‘uint32_t’ {aka ‘unsigned int’}
 6178 |                   (double) (uint32_t) mi->master_heartbeat_period / 1000,
      |                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [sql/CMakeFiles/sql.dir/build.make:2166: sql/CMakeFiles/sql.dir/sys_vars.cc.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:11692: sql/CMakeFiles/sql.dir/all] Error 2
make: *** [Makefile:182: all] Error 2

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_check callback for slave_net_timeout that iterates all
Master_info entries and warns if any heartbeat period exceeds the
requested timeout. Connections that are not configured are skipped. A
DEFAULT heartbeat period is skipped unless --master-heartbeat-period was
given at startup: without it the period follows slave_net_timeout/2 and
can never exceed the timeout, but with it the period is a fixed value
and is checked like an explicit one.

The warning is reported with a new, generalized error
ER_OPERATION_INVALIDATES_CONFIGURED_SLAVE, which names the affected
replication connection and carries a parameterized description of the
misconfiguration. Naming the connection matters for multi-source: when
several connections exceed the new timeout, the user can see exactly
which ones are problematic. One warning is emitted per affected
connection.

Add a multi_source test, run with --master-heartbeat-period=30, that
configures two named connections with explicit heartbeat periods above
the timeout and a third left at DEFAULT, and verifies a distinct warning
is emitted for each.

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 merged commit bcdb4c4 into MariaDB:main Oct 21, 2026
19 checks passed
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. Replication Patches involved in replication

Development

Successfully merging this pull request may close these issues.

4 participants