MDEV-36841 Warn when SET slave_net_timeout drops below master_heartbeat_period by tonychen2001 · Pull Request #4845 · MariaDB/server

tonychen2001 · 2026-03-23T19:22:20Z

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

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

LGTM, but please fix the test failures.

gkodinov

LGTM. Please stand by for the final review.

bnestere

Thank you for your contribution, @tonychen2001 !

I've left a couple notes.

bnestere · 2026-04-21T18:40:45Z

-       VALID_RANGE(1, LONG_TIMEOUT), DEFAULT(SLAVE_NET_TIMEOUT), BLOCK_SIZE(1));
+       VALID_RANGE(1, LONG_TIMEOUT), DEFAULT(SLAVE_NET_TIMEOUT), BLOCK_SIZE(1),
+       NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
+       ON_UPDATE(fix_slave_net_timeout));


fix_slave_net_timeout() doesn't actually change any values does it? It looks like it only validates. I think in such case ON_UPDATE() isn't needed and it can just be done as part of ON_CHECK() and the function name should be changed to replace the fix_ prefix with something more appropriate (like check_).

The pattern for max_allowed_packet can be followed here as well as for my previous comment about changing the error to use.

Updated to ON_CHECK().

bnestere · 2026-04-21T18:49:10Z

+CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=40;
+SET @@global.slave_net_timeout=20;
+Warnings:
+Warning	1704	The requested value for the heartbeat period exceeds the value of `slave_net_timeout' seconds. A sensible value for the period should be less than the timeout


This reads like the wrong error message. "The requested value for the heartbeat period exceeds..." makes it seem like we are trying to changed the heartbeat period, but it is the slave_net_timeout variable being changed. Perhaps WARN_OPTION_BELOW_LIMIT would be more appropriate here, as it doesn't imply which value is being set (alternatively you can add WARN_OPTION_ABOVE_LIMIT 🤷).

Yes, I agree.

In regards to the warning, when using WARN_OPTION_BELOW_LIMIT (or even introducing WARN_OPTION_ABOVE_LIMIT) the error message would look like: The value of 'slave_net_timeout' should be no less than the value of 'master_heartbeat_period'.

A few thoughts related to this thread with Joro above is that when a user modifies the slave_net_timeout system variable, there could be multiple replication channels that have a master heartbeat period higher than the value of slave_net_timeout and it wouldn't be clear to the user which replication channels are problematic.

I think a clearer error message should contain the specific replication channels that have a problematic heartbeat period with respect to the new slave_net_timeout value.

What do you think about introducing a specific error message (e.g. ER_SLAVE_HEARTBEAT_PERIOD_ABOVE_TIMEOUT) that names the affected connection and have it emit a warning per replication connection with a problematic configuration:

Warning XXXX The heartbeat period for replication connection '' exceeds the value of 'slave_net_timeout'. A sensible value for the period should be less than the timeout. Warning XXXX The heartbeat period for replication connection 'connection2' exceeds the value of 'slave_net_timeout'. A sensible value for the period should be less than the timeout.

I agree that it would be good to call out the connection name. Though I think the message should be generalized to reporting on a parameterizable misconfiguration for a replication channel. Perhaps ER_OPERATION_INVALIDATES_CONFIGURED_SLAVE

With some message like
"This operation leaves replication connection '%s' with an invalid configuration: %s"

Thank you for the feedback, I have added a parameterized general warning message which will read like:

Warning 4257 This operation leaves replication connection '' with an invalid configuration: the heartbeat period (4.000 seconds) exceeds the requested slave_net_timeout (3 seconds); a sensible value for the period should be less than the timeout

bnestere

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 · 2026-10-16T23:26:45Z

All committers have signed the CLA.

tonychen2001 · 2026-10-16T23:28:21Z

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

gkodinov

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 added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Mar 24, 2026

gkodinov requested changes Mar 24, 2026

View reviewed changes

Comment thread mysql-test/suite/rpl/r/rpl_heartbeat.result

Comment thread sql/sys_vars.cc Outdated

tonychen2001 force-pushed the MDEV-36841 branch from 5a12245 to 84ea4fc Compare March 24, 2026 18:24

gkodinov approved these changes Mar 25, 2026

View reviewed changes

gkodinov requested a review from bnestere March 25, 2026 08:19

gkodinov assigned bnestere Mar 25, 2026

bnestere added the Replication Patches involved in replication label Apr 21, 2026

bnestere requested changes Apr 21, 2026

View reviewed changes

tonychen2001 force-pushed the MDEV-36841 branch 3 times, most recently from 8389993 to 1678744 Compare June 12, 2026 00:36

ParadoxV5 requested a review from bnestere June 12, 2026 03:53

bnestere approved these changes Oct 16, 2026

View reviewed changes

tonychen2001 force-pushed the MDEV-36841 branch from 1678744 to b354b77 Compare October 16, 2026 23:26

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

gkodinov requested changes Oct 20, 2026

View reviewed changes

tonychen2001 force-pushed the MDEV-36841 branch from b354b77 to a33f966 Compare October 20, 2026 16:43

Conversation

tonychen2001 commented Mar 23, 2026

Description

Release Notes

How can this PR be tested?

Basing the PR against the correct MariaDB version

Copyright

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

bnestere left a comment

Choose a reason for hiding this comment

Uh oh!

bnestere Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

tonychen2001 Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

bnestere Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

tonychen2001 Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bnestere Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

tonychen2001 Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bnestere left a comment

Choose a reason for hiding this comment

Uh oh!

CLAassistant commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonychen2001 commented Oct 16, 2026

Uh oh!

gkodinov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

tonychen2001 Apr 28, 2026 •

edited

Loading

tonychen2001 Jun 12, 2026 •

edited

Loading

CLAassistant commented Oct 16, 2026 •

edited

Loading

gkodinov left a comment •

edited

Loading