MDEV-35879 Slave_heartbeat_period is imprecise

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

@@ -16,7 +16,7 @@ show status like 'Slave_heartbeat_period';;
 Variable_name	Slave_heartbeat_period
 Value	5.000
 connection slave;
-change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 0.0009999;
+change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 0.0004999;
 Warnings:
 Warning	1703	The requested value for the heartbeat period is less than 1 millisecond. The value is reset to 0, meaning that heartbeating will effectively be disabled
 show status like 'Slave_heartbeat_period';;

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

@@ -144,24 +144,35 @@ CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='
 SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
 Variable_name	Value
 Slave_heartbeat_period	0.001
+Slave_heartbeat_period = '0.001'
 RESET SLAVE;
-CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=0.0009;
+CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=0.00049;
 Warnings:
 Warning	1703	The requested value for the heartbeat period is less than 1 millisecond. The value is reset to 0, meaning that heartbeating will effectively be disabled
 SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
 Variable_name	Value
 Slave_heartbeat_period	0.000
+Slave_heartbeat_period = '0.000'
 RESET SLAVE;
 
 *** Max slave_heartbeat_timeout ***
+CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294966.999;
+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
+SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
+Variable_name	Value
+Slave_heartbeat_period	4294966.999
+Slave_heartbeat_period = '4294966.999'
+RESET SLAVE;
 CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294967;
 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
 SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
 Variable_name	Value
 Slave_heartbeat_period	4294967.000
+Slave_heartbeat_period = '4294967.000'
 RESET SLAVE;
-CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294968;
+CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294967.001;
 ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967 seconds)
 RESET SLAVE;
 CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=8589935;

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

@@ -41,13 +41,13 @@ eval change master to master_host='127.0.0.1',master_port=$MASTER_MYPORT, master
 --query_vertical show status like 'Slave_heartbeat_period';
 
 #
-# the min value for the period is 1 millisecond an attempt to assign a
+# the min value for the period is 1 millisecond rounded; an attempt to assign a
 # lesser will be warned with treating the value as zero
 #
 connection slave;
 --replace_result $MASTER_MYPORT MASTER_PORT
 ### 5.1 mtr does not have --warning ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE
-eval change master to master_host='127.0.0.1',master_port=$MASTER_MYPORT, master_user='root', master_heartbeat_period= 0.0009999;
+eval change master to master_host='127.0.0.1',master_port=$MASTER_MYPORT, master_user='root', master_heartbeat_period= 0.0004999;
 --query_vertical show status like 'Slave_heartbeat_period';
 
 #

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

@@ -190,26 +190,36 @@ let $slave_heartbeat_timeout= query_get_value(SHOW GLOBAL STATUS LIKE 'slave_hea
 #
 # Check limits for slave_heartbeat_timeout
 #
+--let $status_items= Slave_heartbeat_period
+--let $all_slaves_status= 1
 
 --echo *** Min slave_heartbeat_timeout ***
 --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=0.001;
 SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
+--source include/show_slave_status.inc
 RESET SLAVE;
 --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=0.0009;
+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=0.00049;
 SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
+--source include/show_slave_status.inc
 RESET SLAVE;
 --echo
 
 --echo *** Max slave_heartbeat_timeout ***
 --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=4294966.999;
+SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
+--source include/show_slave_status.inc
+RESET SLAVE;
+--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=4294967;
 SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
+--source include/show_slave_status.inc
 RESET SLAVE;
 --replace_result $MASTER_MYPORT MASTER_PORT
 --error ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE 
-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=4294968;
+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=4294967.001;
 RESET SLAVE;
 # Check double size of max allowed value for master_heartbeat_period
 --replace_result $MASTER_MYPORT MASTER_PORT

sql/mysqld.cc

@@ -7010,7 +7010,7 @@ static int show_heartbeat_period(THD *thd, SHOW_VAR *var, void *buff,
       get_master_info(&thd->variables.default_master_connection,
                       Sql_condition::WARN_LEVEL_NOTE))
   {
-    sprintf(static_cast<char*>(buff), "%.3f", mi->heartbeat_period);
+    sprintf(static_cast<char*>(buff), "%.3lf", mi->heartbeat_period/1000.0);
     mi->release();
     var->type= SHOW_CHAR;
     var->value= buff;

sql/rpl_mi.cc

@@ -229,11 +229,8 @@ void init_master_log_pos(Master_info* mi)
     if CHANGE MASTER did not specify it.  (no data loss in conversion
     as hb period has a max)
   */
-  mi->heartbeat_period= (float) MY_MIN(SLAVE_MAX_HEARTBEAT_PERIOD,
-                                    (slave_net_timeout/2.0));
-  DBUG_ASSERT(mi->heartbeat_period > (float) 0.001
-              || mi->heartbeat_period == 0);
-
+  mi->heartbeat_period= MY_MIN(SLAVE_MAX_HEARTBEAT_PERIOD,
+                               slave_net_timeout*500ULL);
   DBUG_VOID_RETURN;
 }
 
@@ -450,7 +447,7 @@ file '%s')", fname);
     mi->fd = fd;
     int port, connect_retry, master_log_pos, lines;
     int ssl= 0, ssl_verify_server_cert= 0;
-    float master_heartbeat_period= 0.0;
+    double master_heartbeat_period= 0.0;
     char *first_non_digit;
     char buf[HOSTNAME_LENGTH+1];
 
@@ -674,7 +671,8 @@ file '%s')", fname);
     mi->connect_retry= (uint) connect_retry;
     mi->ssl= (my_bool) ssl;
     mi->ssl_verify_server_cert= ssl_verify_server_cert;
-    mi->heartbeat_period= MY_MIN(SLAVE_MAX_HEARTBEAT_PERIOD, master_heartbeat_period);
+    mi->heartbeat_period=
+     MY_MIN(SLAVE_MAX_HEARTBEAT_PERIOD, master_heartbeat_period*1000);
   }
   DBUG_PRINT("master_info",("log_file_name: %s  position: %ld",
                             mi->master_log_name,
@@ -811,7 +809,7 @@ int flush_master_info(Master_info* mi,
      of file we don't care about this garbage.
   */
   char heartbeat_buf[FLOATING_POINT_BUFFER];
-  my_fcvt(mi->heartbeat_period, 3, heartbeat_buf, NULL);
+  my_fcvt(mi->heartbeat_period/1000.0, 3, heartbeat_buf, NULL);
   my_b_seek(file, 0L);
   my_b_printf(file,
               "%u\n%s\n%s\n%s\n%s\n%s\n%d\n%d\n%d\n%s\n%s\n%s\n%s\n%s\n%d\n%s\n%s\n%s\n%s\n%d\n%s\n%s\n"

sql/rpl_mi.h

@@ -282,7 +282,7 @@ class Master_info : public Slave_reporting_capability
     events should happen before fsyncing.
   */
   uint sync_counter;
-  float heartbeat_period;         // interface with CHANGE MASTER or master.info
+  uint32_t heartbeat_period; ///< integer in milliseconds
   ulonglong received_heartbeats;  // counter of received heartbeat events
   DYNAMIC_ARRAY ignore_server_ids;
   ulong master_id;

sql/slave.cc

@@ -1593,15 +1593,15 @@ int init_intvar_from_file(int* var, IO_CACHE* f, int default_val)
   DBUG_RETURN(1);
 }
 
-int init_floatvar_from_file(float* var, IO_CACHE* f, float default_val)
+int init_floatvar_from_file(double* var, IO_CACHE* f, float default_val)
 {
   char buf[16];
   DBUG_ENTER("init_floatvar_from_file");
 
 
   if (my_b_gets(f, buf, sizeof(buf)))
   {
-    if (sscanf(buf, "%f", var) != 1)
+    if (sscanf(buf, "%lf", var) != 1)
       DBUG_RETURN(1);
     else
       DBUG_RETURN(0);
@@ -2146,15 +2146,15 @@ when it try to get the value of TIME_ZONE global variable from master.";
     }
   }
 
-  if (mi->heartbeat_period != 0.0)
+  if (mi->heartbeat_period)
   {
     const char query_format[]= "SET @master_heartbeat_period= %llu";
     char query[sizeof(query_format) + 32];
     /* 
        the period is an ulonglong of nano-secs. 
     */
     my_snprintf(query, sizeof(query), query_format,
-                (ulonglong) (mi->heartbeat_period*1000000000UL));
+                mi->heartbeat_period*1000000ULL);
 
     DBUG_EXECUTE_IF("simulate_slave_heartbeat_network_error",
                     { static ulong dbug_count= 0;
@@ -3413,7 +3413,7 @@ static bool send_show_master_info_data(THD *thd, Master_info *mi, bool full,
       protocol->store((ulonglong) mi->rli.max_relay_log_size);
       protocol->store(mi->rli.executed_entries);
       protocol->store((uint32)    mi->received_heartbeats);
-      protocol->store_double(mi->heartbeat_period, 3);
+      protocol->store_double(mi->heartbeat_period/1000.0, 3);
       protocol->store(gtid_pos->ptr(), gtid_pos->length(), &my_charset_bin);
     }
 

sql/slave.h

@@ -37,10 +37,8 @@
    compiled in (embedded).
 */
 
-/**
-   The maximum is defined as (ULONG_MAX/1000) with 4 bytes ulong
-*/
-#define SLAVE_MAX_HEARTBEAT_PERIOD 4294967
+/// @return UINT32_MAX / 1000 * 1000
+#define SLAVE_MAX_HEARTBEAT_PERIOD 4294967000
 
 #ifdef HAVE_REPLICATION
 
@@ -67,7 +65,7 @@ struct rpl_parallel_thread;
 int init_intvar_from_file(int* var, IO_CACHE* f, int default_val);
 int init_strvar_from_file(char *var, int max_size, IO_CACHE *f,
                           const char *default_val);
-int init_floatvar_from_file(float* var, IO_CACHE* f, float default_val);
+int init_floatvar_from_file(double* var, IO_CACHE* f, float default_val);
 int init_dynarray_intvar_from_file(DYNAMIC_ARRAY* arr, IO_CACHE* f);
 
 /*****************************************************************************
@@ -266,7 +264,7 @@ int apply_event_and_update_pos_for_parallel(Log_event* ev, THD* thd,
                                             struct rpl_group_info *rgi);
 
 int init_intvar_from_file(int* var, IO_CACHE* f, int default_val);
-int init_floatvar_from_file(float* var, IO_CACHE* f, float default_val);
+int init_floatvar_from_file(double* var, IO_CACHE* f, float default_val);
 int init_strvar_from_file(char *var, int max_size, IO_CACHE *f,
                           const char *default_val);
 int init_dynarray_intvar_from_file(DYNAMIC_ARRAY* arr, IO_CACHE* f);

sql/sql_lex.h

@@ -506,7 +506,7 @@ struct LEX_MASTER_INFO
   ulong relay_log_pos;
   ulong server_id;
   uint port, connect_retry;
-  float heartbeat_period;
+  ulonglong heartbeat_period;
   int sql_delay;
   bool is_demotion_opt;
   /*

sql/sql_repl.cc

@@ -3784,8 +3784,8 @@ bool change_master(THD* thd, Master_info* mi, bool *master_info_added)
   if (lex_mi->heartbeat_opt != LEX_MASTER_INFO::LEX_MI_UNCHANGED)
     mi->heartbeat_period = lex_mi->heartbeat_period;
   else
-    mi->heartbeat_period= (float) MY_MIN(SLAVE_MAX_HEARTBEAT_PERIOD,
-                                      (slave_net_timeout/2.0));
+    mi->heartbeat_period= MY_MIN(SLAVE_MAX_HEARTBEAT_PERIOD,
+                                 slave_net_timeout*500ULL);
   mi->received_heartbeats= 0; // counter lives until master is CHANGEd
 
   /*

sql/sql_yacc.yy

@@ -2267,30 +2267,33 @@ master_def:
 
         | MASTER_HEARTBEAT_PERIOD_SYM '=' NUM_literal
           {
-            Lex->mi.heartbeat_period= (float) $3->val_real();
-            if (unlikely(Lex->mi.heartbeat_period >
-                         SLAVE_MAX_HEARTBEAT_PERIOD) ||
-                unlikely(Lex->mi.heartbeat_period < 0.0))
-               my_yyabort_error((ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE, MYF(0),
-                                 SLAVE_MAX_HEARTBEAT_PERIOD));
-
-            if (unlikely(Lex->mi.heartbeat_period > slave_net_timeout))
+            static const struct Decimal_from_double: my_decimal
             {
+              Decimal_from_double(double value): my_decimal()
+              { DBUG_ASSERT(!double2my_decimal(E_DEC_ERROR, value, this)); }
+            } MAX_PERIOD= SLAVE_MAX_HEARTBEAT_PERIOD/1000, THOUSAND= 1000;
+            auto decimal_buffer= my_decimal();
+            my_decimal *decimal= $3->val_decimal(&decimal_buffer);
+            if (!decimal ||
+                decimal->sign() || decimal_cmp(&MAX_PERIOD, decimal) < 0)
+               my_yyabort_error((ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE, MYF(0),
+                                 SLAVE_MAX_HEARTBEAT_PERIOD/1000));
+            bool overprecise= decimal->frac > 3;
+            // decomposed from my_decimal2int() to reduce a bit of computations
+            auto rounded= my_decimal();
+            DBUG_ASSERT(
+              !decimal_round(decimal, &rounded, 3, HALF_UP) &&
+              !decimal_mul(&rounded, &THOUSAND, &decimal_buffer) &&
+              !decimal2ulonglong(&decimal_buffer, &(Lex->mi.heartbeat_period))
+            );
+            if (unlikely(Lex->mi.heartbeat_period > slave_net_timeout*1000ULL))
               push_warning(thd, Sql_condition::WARN_LEVEL_WARN,
                            ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE_MAX,
                            ER_THD(thd, ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE_MAX));
-            }
-            if (unlikely(Lex->mi.heartbeat_period < 0.001))
-            {
-              if (unlikely(Lex->mi.heartbeat_period != 0.0))
-              {
+            else if (unlikely(!Lex->mi.heartbeat_period && overprecise))
                 push_warning(thd, Sql_condition::WARN_LEVEL_WARN,
                              ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE_MIN,
                              ER_THD(thd, ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE_MIN));
-                Lex->mi.heartbeat_period= 0.0;
-              }
-              Lex->mi.heartbeat_opt=  LEX_MASTER_INFO::LEX_MI_DISABLE;
-            }
             Lex->mi.heartbeat_opt=  LEX_MASTER_INFO::LEX_MI_ENABLE;
           }
         | IGNORE_SERVER_IDS_SYM '=' '(' ignore_server_id_list ')'

storage/perfschema/table_replication_connection_configuration.cc

@@ -257,7 +257,7 @@ void table_replication_connection_configuration::make_row(Master_info *mi)
 
   m_row.connection_retry_count= master_retry_count; //(ulong) mi->retry_count;
 
-  m_row.heartbeat_interval= (double)mi->heartbeat_period;
+  m_row.heartbeat_interval= mi->heartbeat_period / 1000.0;
 
   m_row.ignore_server_ids= convert_array_to_str(&mi->ignore_server_ids);
   if (m_row.ignore_server_ids == NULL)