Skip to content
/ server Public

MDEV-35879 Slave_heartbeat_period is imprecise #4441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ParadoxV5 wants to merge 4 commits into
base: 10.11
Choose a base branch
from
Open

MDEV-35879 Slave_heartbeat_period is imprecise #4441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
676743a
MDEV-35879 Slave_heartbeat_period is imprecise
ParadoxV5 Nov 17, 2025
7061af8
Merge branch '10.11' into MDEV-35879
ParadoxV5 Nov 18, 2025
ce9e658
fix (i think)
ParadoxV5 Nov 18, 2025
bb6123d
oh wow, `__attribute__((unused))` does things
ParadoxV5 Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mysql-test/suite/rpl/r/rpl_heartbeat.result
View file
Open in desktop
Copy link
Contributor Author

@ParadoxV5 ParadoxV5 Nov 17, 2025

Choose a reason for hiding this comment

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

Range testing of master_heartbeat_period is duplicated between rpl_heartbeat_basic (runs with mix only) and rpl_heartbeat (tests all three binlog formats).

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor Author

@ParadoxV5 ParadoxV5 Nov 17, 2025

Choose a reason for hiding this comment

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

Should I keep the rounding method change out of 10.11?

Copy link
Contributor

@bnestere bnestere Jan 5, 2026

Choose a reason for hiding this comment

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

I think we should put the whole patch into main (or 12.3), it isn't an issue that any users/customers have complained about, and it doesn't have a broader impact. Better not to put those into older/more stable versions.

Copy link
Contributor Author

@ParadoxV5 ParadoxV5 Jan 23, 2026

Choose a reason for hiding this comment

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

the whole patch

Can you please clarify: Not just the rounding method change, but also the imprecision (main topic of MDEV-35879)?

Then, perhaps some or all of the points in MDEV-37529 should only be addressed on the main branch.

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';;
Expand Down
15 changes: 13 additions & 2 deletions mysql-test/suite/rpl/r/rpl_heartbeat_basic.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions mysql-test/suite/rpl/t/rpl_heartbeat.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

#
Expand Down
14 changes: 12 additions & 2 deletions mysql-test/suite/rpl/t/rpl_heartbeat_basic.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@bnestere bnestere Jan 5, 2026

Choose a reason for hiding this comment

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

I'd suggest adding a comment to why you show this twice from different sources.

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
Expand Down
2 changes: 1 addition & 1 deletion sql/mysqld.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 6 additions & 8 deletions sql/rpl_mi.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

@bnestere bnestere Jan 5, 2026

Choose a reason for hiding this comment

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

I assume this is an optimization of (x*1000)/2. I think the compiler should automatically do it though, and it would improve readability to have the full equation. Do you know how to disassemble code? I'd be curious if the generated instructions are the same or not. If not, then we can keep it (though I'd suggest a comment with the full equation).

DBUG_VOID_RETURN;
}

Expand Down Expand Up @@ -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];

Expand Down Expand Up @@ -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,
static_cast<uint32_t>(master_heartbeat_period*1000));
}
DBUG_PRINT("master_info",("log_file_name: %s position: %ld",
mi->master_log_name,
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion sql/rpl_mi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions sql/slave.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
10 changes: 4 additions & 6 deletions sql/slave.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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);

/*****************************************************************************
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion sql/sql_lex.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/*
Expand Down
6 changes: 3 additions & 3 deletions sql/sql_repl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3782,10 +3782,10 @@ bool change_master(THD* thd, Master_info* mi, bool *master_info_added)
if (lex_mi->connect_retry)
mi->connect_retry = lex_mi->connect_retry;
if (lex_mi->heartbeat_opt != LEX_MASTER_INFO::LEX_MI_UNCHANGED)
mi->heartbeat_period = lex_mi->heartbeat_period;
mi->heartbeat_period = static_cast<uint32_t>(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

/*
Expand Down
41 changes: 24 additions & 17 deletions sql/sql_yacc.yy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2267,30 +2267,37 @@ 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()
Copy link
Contributor

@bnestere bnestere Jan 5, 2026

Choose a reason for hiding this comment

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

Hmm, I wonder why my_decimal doesn't have this constructor innately? Would it make sense to add it? Though that'd also be a question for the runtime team

{
int unexpected_error __attribute__((unused))=
double2my_decimal(E_DEC_ERROR, value, this);
DBUG_ASSERT(!unexpected_error);
}
} MAX_PERIOD= SLAVE_MAX_HEARTBEAT_PERIOD/1000.0, THOUSAND= 1000;
Comment on lines +2272 to +2278
Copy link
Contributor Author

@ParadoxV5 ParadoxV5 Nov 19, 2025

Choose a reason for hiding this comment

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

Suggested change
Decimal_from_double(double value): my_decimal()
{
int unexpected_error __attribute__((unused))=
double2my_decimal(E_DEC_ERROR, value, this);
DBUG_ASSERT(!unexpected_error);
}
} MAX_PERIOD= SLAVE_MAX_HEARTBEAT_PERIOD/1000.0, THOUSAND= 1000;
Decimal_from_ll(longlong value): my_decimal()
{
int unexpected_error __attribute__((unused))=
int2my_decimal(E_DEC_ERROR, value, false, this);
DBUG_ASSERT(!unexpected_error);
}
} 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();
int unexpected_error __attribute__((unused))=
decimal_round(decimal, &rounded, 3, HALF_UP) |
Copy link
Contributor

@bnestere bnestere Jan 5, 2026

Choose a reason for hiding this comment

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

I don't think the compiler guarantees the order-of-operations with bitwise OR (i.e., the three functions involved here may run in any order, which is not what you intend, I think). I think this would be better served by &&s, also because the &&'s short-circuit if there's an error in an earlier stage:

!decimal_round && !decimal_mul && !decimal2ulonglong

or

!(decimal_round || decimal_mul || decimal2ulonglong)

I think that it reads better as well (either way), as it is clear you are working with a single boolean outcome.

Copy link
Contributor Author

@ParadoxV5 ParadoxV5 Jan 5, 2026

Choose a reason for hiding this comment

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

Good call; also affects #4430

decimal_mul(&rounded, &THOUSAND, &decimal_buffer) |
decimal2ulonglong(&decimal_buffer, &(Lex->mi.heartbeat_period));
Comment on lines +2290 to +2291
Copy link
Contributor Author

@ParadoxV5 ParadoxV5 Nov 19, 2025

Choose a reason for hiding this comment

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

Suggested change
decimal_mul(&rounded, &THOUSAND, &decimal_buffer) |
decimal2ulonglong(&decimal_buffer, &(Lex->mi.heartbeat_period));
/*
(The ideal order would export to a `double` and *then* multiply,
but disappointingly, decimal2double() is implemented
by printing into a string and parsing that char array.)
*/
decimal_mul(&rounded, &THOUSAND, &decimal_buffer) |
decimal2ulonglong(&decimal_buffer, &(Lex->mi.heartbeat_period));

Copy link
Contributor

@bnestere bnestere Jan 5, 2026

Choose a reason for hiding this comment

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

I'd suggest avoiding emotion in code comments, and stick to the facts. I.e., instead of

but disappointingly, decimal2double() is implemented by printing into a string and parsing that char array.)

to something like

but as of this patch, decimal2double() is implemented by printing into a string and parsing that char array, which is extra overhead that we can bypass by ...

DBUG_ASSERT(!unexpected_error);
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 ')'
Expand Down
2 changes: 1 addition & 1 deletion storage/perfschema/table_replication_connection_configuration.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down