MDEV-36841 Warn when SET slave_net_timeout drops below master_heartbeat_period

mysql-test/suite/binlog_in_engine/rpl_heartbeat.result

@@ -14,6 +14,8 @@ show status like 'Slave_heartbeat_period';;
 Variable_name	Slave_heartbeat_period
 Value	4.000
 set @@global.slave_net_timeout= 3 /* must be a warning */;
+Warnings:
+Warning	4276	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
 reset slave;
 connection master;
 drop table if exists t1;

mysql-test/suite/multi_source/slave_net_timeout_heartbeat.opt

@@ -0,0 +1 @@
+--master-heartbeat-period=30

mysql-test/suite/multi_source/slave_net_timeout_heartbeat.result

@@ -0,0 +1,17 @@
+connect  slave,127.0.0.1,root,,,$SERVER_MYPORT_3;
+connection slave;
+SET @restore_slave_net_timeout= @@global.slave_net_timeout;
+SET @@global.slave_net_timeout= 60;
+change master 'slave1' to master_port=MYPORT_1, master_host='127.0.0.1', master_user='root', master_ssl_verify_server_cert=0, master_heartbeat_period=40;
+change master 'slave2' to master_port=MYPORT_2, master_host='127.0.0.1', master_user='root', master_ssl_verify_server_cert=0, master_heartbeat_period=50;
+change master 'slave3' to master_port=MYPORT_1, master_host='localhost', master_user='root', master_ssl_verify_server_cert=0;
+SET @@global.slave_net_timeout= 20;
+SHOW WARNINGS;
+Level	Code	Message
+Warning	4276	This operation leaves replication connection 'slave1' with an invalid configuration: the heartbeat period (40.000 seconds) exceeds the requested slave_net_timeout (20 seconds); a sensible value for the period should be less than the timeout
+Warning	4276	This operation leaves replication connection 'slave2' with an invalid configuration: the heartbeat period (50.000 seconds) exceeds the requested slave_net_timeout (20 seconds); a sensible value for the period should be less than the timeout
+Warning	4276	This operation leaves replication connection 'slave3' with an invalid configuration: the heartbeat period (30.000 seconds) exceeds the requested slave_net_timeout (20 seconds); a sensible value for the period should be less than the timeout
+SET @@global.slave_net_timeout= @restore_slave_net_timeout;
+include/reset_master_slave.inc
+disconnect slave;
+connection default;

mysql-test/suite/multi_source/slave_net_timeout_heartbeat.test

@@ -0,0 +1,48 @@
+#
+# MDEV-36841: Lowering @@global.slave_net_timeout below an explicitly
+# configured master_heartbeat_period warns once per replication connection.
+#
+# With multi-source replication there can be more than one connection whose
+# heartbeat period exceeds the new timeout, so the warning names each affected
+# connection.  This test configures two named connections, each with an
+# explicit heartbeat period above the timeout we set next, and verifies that a
+# separate warning is emitted for both.
+#
+# The server is started with --master-heartbeat-period=30, so a third
+# connection left at the DEFAULT heartbeat period resolves to a fixed 30
+# seconds rather than slave_net_timeout/2, and must warn as well.
+#
+--source include/not_embedded.inc
+
+--connect (slave,127.0.0.1,root,,,$SERVER_MYPORT_3)
+
+--connection slave
+SET @restore_slave_net_timeout= @@global.slave_net_timeout;
+SET @@global.slave_net_timeout= 60;
+
+# Two sources, each with an explicit heartbeat period below the current
+# timeout (60), so configuring them does not warn.
+--replace_result $SERVER_MYPORT_1 MYPORT_1
+eval change master 'slave1' to master_port=$SERVER_MYPORT_1, master_host='127.0.0.1', master_user='root', master_ssl_verify_server_cert=0, master_heartbeat_period=40;
+--replace_result $SERVER_MYPORT_2 MYPORT_2
+eval change master 'slave2' to master_port=$SERVER_MYPORT_2, master_host='127.0.0.1', master_user='root', master_ssl_verify_server_cert=0, master_heartbeat_period=50;
+# DEFAULT heartbeat period, i.e. --master-heartbeat-period=30.  The
+# connection is never started; 'localhost' only avoids the host:port
+# conflict with 'slave1'.
+--replace_result $SERVER_MYPORT_1 MYPORT_1
+eval change master 'slave3' to master_port=$SERVER_MYPORT_1, master_host='localhost', master_user='root', master_ssl_verify_server_cert=0;
+
+# Lowering the timeout below all three heartbeat periods must warn once per
+# connection.  Warnings are pushed in master_info hash order, which is not
+# deterministic, so capture them with a sorted SHOW WARNINGS.
+--disable_warnings
+SET @@global.slave_net_timeout= 20;
+--enable_warnings
+--sorted_result
+SHOW WARNINGS;
+
+# Clean up
+SET @@global.slave_net_timeout= @restore_slave_net_timeout;
+--source include/reset_master_slave.inc
+--disconnect slave
+--connection default

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

@@ -14,6 +14,8 @@ show status like 'Slave_heartbeat_period';;
 Variable_name	Slave_heartbeat_period
 Value	4.000
 set @@global.slave_net_timeout= 3 /* must be a warning */;
+Warnings:
+Warning	4276	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
 reset slave;
 connection master;
 drop table if exists t1;

mysql-test/suite/rpl/r/rpl_heartbeat_basic.result

@@ -54,11 +54,20 @@ Slave_heartbeat_period	25.000
 SET @@global.slave_net_timeout=@restore_slave_net_timeout;
 include/reset_slave.inc
 
-*** Warning if updated slave_net_timeout < slave_heartbeat_timeout ***
+*** No warning if slave_net_timeout lowered with default heartbeat_period ***
 SET @@global.slave_net_timeout=FLOOR(SLAVE_HEARTBEAT_TIMEOUT)-1;
 SET @@global.slave_net_timeout=@restore_slave_net_timeout;
 RESET SLAVE;
 
+*** Warning if slave_net_timeout set below explicit heartbeat_period ***
+SET @@global.slave_net_timeout=60;
+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	4276	This operation leaves replication connection '' with an invalid configuration: the heartbeat period (40.000 seconds) exceeds the requested slave_net_timeout (20 seconds); a sensible value for the period should be less than the timeout
+SET @@global.slave_net_timeout=@restore_slave_net_timeout;
+RESET SLAVE;
+
 *** Warning if updated slave_heartbeat_timeout > slave_net_timeout ***
 CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=SLAVE_NET_TIMEOUT;
 Warnings:

mysql-test/suite/rpl/t/rpl_heartbeat_basic.test

@@ -104,15 +104,25 @@ SET @@global.slave_net_timeout=@restore_slave_net_timeout;
 --source include/reset_slave.inc
 --echo
 
-# Set slave_net_timeout less than current value of slave_heartbeat_period
---echo *** Warning if updated slave_net_timeout < slave_heartbeat_timeout ***
+# No warning when heartbeat_period is DEFAULT (automatically slave_net_timeout/2)
+--echo *** No warning if slave_net_timeout lowered with default heartbeat_period ***
 let $slave_heartbeat_timeout= query_get_value(SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period', Value, 1);
 --replace_result $slave_heartbeat_timeout SLAVE_HEARTBEAT_TIMEOUT
 eval SET @@global.slave_net_timeout=FLOOR($slave_heartbeat_timeout)-1;
 SET @@global.slave_net_timeout=@restore_slave_net_timeout;
 RESET SLAVE;
 --echo
 
+# Set slave_net_timeout less than explicitly set slave_heartbeat_period (MDEV-36841)
+--echo *** Warning if slave_net_timeout set below explicit heartbeat_period ***
+SET @@global.slave_net_timeout=60;
+--replace_result $MASTER_MYPORT MASTER_PORT
+eval CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=$MASTER_MYPORT, MASTER_USER='root', MASTER_CONNECT_RETRY=$connect_retry, MASTER_HEARTBEAT_PERIOD=40;
+SET @@global.slave_net_timeout=20;
+SET @@global.slave_net_timeout=@restore_slave_net_timeout;
+RESET SLAVE;
+--echo
+
 # Set value of slave_heartbeat_period greater than slave_net_timeout
 --echo *** Warning if updated slave_heartbeat_timeout > slave_net_timeout ***
 let $slave_net_timeout= query_get_value(SHOW VARIABLES LIKE 'slave_net_timeout', Value, 1);

sql/share/errmsg-utf8.txt

@@ -12427,3 +12427,5 @@ ER_INVALID_HOST_NETMASK
         eng "Invalid masked host '%s' for user '%s'"
 ER_ACL_ENTRY_IGNORED_BAD_HOST_MASK
         eng "'%s' entry '%s' ignored, the host is not a valid ip/netmask"
+ER_OPERATION_INVALIDATES_CONFIGURED_SLAVE
+        eng "This operation leaves replication connection '%s' with an invalid configuration: %s"

sql/sys_vars.cc

@@ -6151,13 +6151,57 @@ static Sys_var_uint Sys_opt_binlog_partial_rows_event_max_size(
       CMD_LINE(REQUIRED_ARG), VALID_RANGE(1024, MAX_MAX_ALLOWED_PACKET),
       DEFAULT(MAX_MAX_ALLOWED_PACKET), BLOCK_SIZE(1024));
 
+static bool check_slave_net_timeout(sys_var *self, THD *thd, set_var *var)
+{
+  uint timeout= (uint) var->save_result.ulonglong_value;
+  mysql_mutex_lock(&LOCK_active_mi);
+  if (master_info_index)
+  {
+    for (uint i= 0; i < master_info_index->master_info_hash.records; i++)
+    {
+      Master_info *mi= (Master_info *)
+        my_hash_element(&master_info_index->master_info_hash, i);
+      /*
+        Skip connections that were never configured (e.g. the implicit
+        default connection), as START SLAVE does.
+        A DEFAULTed heartbeat period without --master-heartbeat-period
+        follows slave_net_timeout/2 and so can never exceed the new timeout.
+        get_value() would still report half of the old timeout, so skip it.
+      */
+      if (!*mi->host ||
+          (mi->master_heartbeat_period.is_default() &&
+           master_heartbeat_period_is_auto))
+        continue;
+      uint32 period= mi->master_heartbeat_period.get_value();
+      if (period <= timeout * 1000ULL)
+        continue;
+
+      const char *name= mi->connection_name.str ? mi->connection_name.str : "";
+      char detail[256];
+      my_snprintf(detail, sizeof(detail),
+                  "the heartbeat period (%.3f seconds) exceeds the requested "
+                  "slave_net_timeout (%u seconds); a sensible value for the "
+                  "period should be less than the timeout",
+                  period / 1000.0,
+                  timeout);
+      push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
+                          ER_OPERATION_INVALIDATES_CONFIGURED_SLAVE,
+                          ER_THD(thd, ER_OPERATION_INVALIDATES_CONFIGURED_SLAVE),
+                          name, detail);
+    }
+  }
+  mysql_mutex_unlock(&LOCK_active_mi);
+  return false;
+}
+
 static Sys_var_on_access_global<Sys_var_uint,
                                 PRIV_SET_SYSTEM_GLOBAL_VAR_SLAVE_NET_TIMEOUT>
 Sys_slave_net_timeout(
        "slave_net_timeout", "Number of seconds to wait for more data "
        "from any master/slave connection before aborting the read",
        GLOBAL_VAR(slave_net_timeout), CMD_LINE(REQUIRED_ARG),
-       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(check_slave_net_timeout));
 
 
 static Sys_var_uint Sys_master_connect_retry(