Skip to content

Commit fa9b7ae

Browse files
committed
Cherry-pick #4441 for merge conflict resolution
MDEV-35879 Slave_heartbeat_period is imprecise * Increase maximum to `std::numeric_limits<uint32_t>::max() ÷ 1000`, i.e., `4294967.295` *exactly* * Excludes its additional tests as they are exclusive to that issue fix. * But not the `0.00049` case, which was *changed*, not *added*. * Some changes are refactors and so are exclusive to the `main` branch. * Delete tests in `rpl.rpl_heartbeat` that duplicates `rpl.rpl_heartbeat_basic`
1 parent 0069e96 commit fa9b7ae

File tree

13 files changed

+104
-167
lines changed

13 files changed

+104
-167
lines changed

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

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,41 +6,6 @@ connection slave;
66
include/stop_slave.inc
77
set @restore_slave_net_timeout= @@global.slave_net_timeout;
88
set @@global.slave_net_timeout= 10;
9-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root';
10-
show status like 'Slave_heartbeat_period';;
11-
Variable_name Slave_heartbeat_period
12-
Value 5.000
13-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 4294968;
14-
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967 seconds)
15-
show status like 'Slave_heartbeat_period';;
16-
Variable_name Slave_heartbeat_period
17-
Value 5.000
18-
connection slave;
19-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 0.0009999;
20-
Warnings:
21-
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
22-
show status like 'Slave_heartbeat_period';;
23-
Variable_name Slave_heartbeat_period
24-
Value 0.000
25-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 4294967;
26-
Warnings:
27-
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
28-
show status like 'Slave_heartbeat_period';;
29-
Variable_name Slave_heartbeat_period
30-
Value 4294967.000
31-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 0.001;
32-
show status like 'Slave_heartbeat_period';;
33-
Variable_name Slave_heartbeat_period
34-
Value 0.001
35-
reset slave;
36-
set @@global.slave_net_timeout= 5;
37-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 5.001;
38-
Warnings:
39-
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
40-
show status like 'Slave_heartbeat_period';;
41-
Variable_name Slave_heartbeat_period
42-
Value 5.001
43-
reset slave;
449
set @@global.slave_net_timeout= 5;
4510
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 4;
4611
show status like 'Slave_heartbeat_period';;

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
145145
Variable_name Value
146146
Slave_heartbeat_period 0.001
147147
RESET SLAVE;
148-
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;
148+
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;
149149
Warnings:
150150
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
151151
SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
@@ -154,21 +154,21 @@ Slave_heartbeat_period 0.000
154154
RESET SLAVE;
155155

156156
*** Max slave_heartbeat_timeout ***
157-
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294967;
157+
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294967.295;
158158
Warnings:
159159
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
160160
SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
161161
Variable_name Value
162-
Slave_heartbeat_period 4294967.000
162+
Slave_heartbeat_period 4294967.295
163163
RESET SLAVE;
164164
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294968;
165-
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967 seconds)
165+
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967.295 seconds)
166166
RESET SLAVE;
167167
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=8589935;
168-
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967 seconds)
168+
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967.295 seconds)
169169
RESET SLAVE;
170170
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294967296;
171-
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967 seconds)
171+
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967.295 seconds)
172172
RESET SLAVE;
173173

174174
*** Misc incorrect values ***

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

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -21,58 +21,6 @@ set @@global.slave_net_timeout= 10;
2121
--enable_warnings
2222

2323
--enable_prepare_warnings
24-
###
25-
### Checking the range
26-
###
27-
28-
#
29-
# default period slave_net_timeout/2
30-
#
31-
--replace_result $MASTER_MYPORT MASTER_PORT
32-
eval change master to master_host='127.0.0.1',master_port=$MASTER_MYPORT, master_user='root';
33-
--query_vertical show status like 'Slave_heartbeat_period';
34-
35-
#
36-
# the max for the period is ULONG_MAX/1000; an attempt to exceed it is denied
37-
#
38-
--replace_result $MASTER_MYPORT MASTER_PORT
39-
--error ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE
40-
eval change master to master_host='127.0.0.1',master_port=$MASTER_MYPORT, master_user='root', master_heartbeat_period= 4294968;
41-
--query_vertical show status like 'Slave_heartbeat_period';
42-
43-
#
44-
# the min value for the period is 1 millisecond an attempt to assign a
45-
# lesser will be warned with treating the value as zero
46-
#
47-
connection slave;
48-
--replace_result $MASTER_MYPORT MASTER_PORT
49-
### 5.1 mtr does not have --warning ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE
50-
eval change master to master_host='127.0.0.1',master_port=$MASTER_MYPORT, master_user='root', master_heartbeat_period= 0.0009999;
51-
--query_vertical show status like 'Slave_heartbeat_period';
52-
53-
#
54-
# the actual max and min must be accepted
55-
#
56-
--replace_result $MASTER_MYPORT MASTER_PORT
57-
eval change master to master_host='127.0.0.1',master_port=$MASTER_MYPORT, master_user='root', master_heartbeat_period= 4294967;
58-
--query_vertical show status like 'Slave_heartbeat_period';
59-
60-
--replace_result $MASTER_MYPORT MASTER_PORT
61-
eval change master to master_host='127.0.0.1',master_port=$MASTER_MYPORT, master_user='root', master_heartbeat_period= 0.001;
62-
--query_vertical show status like 'Slave_heartbeat_period';
63-
64-
reset slave;
65-
66-
#
67-
# A warning if period greater than slave_net_timeout
68-
#
69-
set @@global.slave_net_timeout= 5;
70-
--replace_result $MASTER_MYPORT MASTER_PORT
71-
eval change master to master_host='127.0.0.1',master_port=$MASTER_MYPORT, master_user='root', master_heartbeat_period= 5.001;
72-
--query_vertical show status like 'Slave_heartbeat_period';
73-
74-
reset slave;
75-
7624
#
7725
# A warning if slave_net_timeout is set to less than the current HB period
7826
#

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,14 +197,14 @@ eval CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=$MASTER_MYPORT, MASTE
197197
SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
198198
RESET SLAVE;
199199
--replace_result $MASTER_MYPORT MASTER_PORT
200-
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;
200+
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;
201201
SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
202202
RESET SLAVE;
203203
--echo
204204

205205
--echo *** Max slave_heartbeat_timeout ***
206206
--replace_result $MASTER_MYPORT MASTER_PORT
207-
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;
207+
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.295;
208208
SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
209209
RESET SLAVE;
210210
--replace_result $MASTER_MYPORT MASTER_PORT

sql/mysqld.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7112,8 +7112,8 @@ static int show_heartbeat_period(THD *thd, SHOW_VAR *var, void *buff,
71127112
get_master_info(&thd->variables.default_master_connection,
71137113
Sql_condition::WARN_LEVEL_NOTE))
71147114
{
7115-
sprintf(static_cast<char*>(buff), "%.3f",
7116-
mi->master_heartbeat_period / 1000.0);
7115+
sprintf(static_cast<char*>(buff), "%.3lf",
7116+
mi->master_heartbeat_period/1000.0);
71177117
mi->release();
71187118
var->type= SHOW_CHAR;
71197119
var->value= buff;

sql/rpl_info_file.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ namespace Int_IO_CACHE
5151
if (!length) // EOF
5252
return true;
5353
// SFINAE if `I` is not a numeric type
54-
std::from_chars_result result= std::from_chars(buf, &buf[length], value);
54+
std::from_chars_result result= std::from_chars(buf, &(buf[length]), value);
5555
// Return `true` if the conversion failed or if the number ended early
5656
return result.ec != ERRC_OK || *(result.ptr) != '\n';
5757
}

sql/rpl_master_info_file.h

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,13 @@
1818
#include "rpl_info_file.h"
1919
#include <unordered_map> // Type of @ref Master_info_file::FIELDS_MAP
2020
#include <string_view> // Key type of @ref Master_info_file::FIELDS_MAP
21-
#include <optional> // Storage type of @ref Optional_int_field
22-
#include <unordered_set> // Used by Master_info_file::load_from_file() to dedup
23-
#include "sql_const.h" // MAX_PASSWORD_LENGTH
21+
// Storage type of @ref Master_info_file::Optional_int_field
22+
#include <optional>
23+
#include <unordered_set> // Used by Master_info_file::load_from_file() to dedup
24+
#include <mysqld_error.h> // ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_X
25+
#include "sql_const.h" // MAX_PASSWORD_LENGTH
26+
// Interface type of @ref Master_info_file::master_heartbeat_period
27+
#include "my_decimal.h"
2428

2529

2630
/**
@@ -396,19 +400,60 @@ struct Master_info_file: Info_file
396400
It has a `DEFAULT` value of @ref ::master_heartbeat_period,
397401
which in turn has a `DEFAULT` value of `@@slave_net_timeout / 2` seconds.
398402
*/
399-
struct: Optional_field<uint32_t>
403+
struct Heartbeat_period_field: Optional_field<uint32_t>
400404
{
405+
/// @return std::numeric_limits<uint32_t>::max() / 1000.0 as a C-string
406+
static constexpr std::string_view MAX= "4294967.295";
401407
using Optional_field::operator=;
402408
operator uint32_t() override
403409
{
404410
return is_default() ? ::master_heartbeat_period.value_or(
405-
MY_MIN(slave_net_timeout*500ULL, SLAVE_MAX_HEARTBEAT_PERIOD)
411+
MY_MIN(slave_net_timeout*500ULL, std::numeric_limits<uint32_t>::max())
406412
) : *(Optional_field<uint32_t>::optional);
407413
}
414+
/** Load from a `DECIMAL(10,3)`
415+
@param out_warning This is overwritten with a MariaDB error code -
416+
whose message has no additional parameters - on *and only on* warning.
417+
*/
418+
// TODO might have to be a static helper callable from the lexer, instead of error on parser
419+
bool load_from(const decimal_t &decimal, uint &out_warning)
420+
{
421+
/*
422+
(The ideal use would work with `double`s, including the `*1000` step,
423+
but disappointingly, the double interfaces of @ref decimal_t are
424+
implemented by printing into a string and parsing that char array.
425+
*/
426+
static const struct Decimal_from_str: my_decimal
427+
{
428+
Decimal_from_str(const std::string_view &string): my_decimal()
429+
{
430+
const char *end= string.end();
431+
[[maybe_unused]] int unexpected_error= str2my_decimal(
432+
E_DEC_ERROR, string.begin(), this, const_cast<char **>(&end));
433+
DBUG_ASSERT(!unexpected_error && !*end);
434+
}
435+
} MAX_PERIOD= MAX, THOUSAND= std::string_view(STRING_WITH_LEN("1000"));
436+
ulonglong decimal_out;
437+
if (decimal.sign || decimal_cmp(&MAX_PERIOD, &decimal) < 0)
438+
return (out_warning= ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE);
439+
bool overprecise= decimal.frac > 3;
440+
// decomposed from my_decimal2int() to reduce a bit of computations
441+
auto rounded= my_decimal(), product= my_decimal();
442+
[[maybe_unused]] int unexpected_error=
443+
decimal_round(&decimal, &rounded, 3, HALF_UP) |
444+
decimal_mul(&rounded, &THOUSAND, &product) |
445+
decimal2ulonglong(&product, &decimal_out);
446+
DBUG_ASSERT(!unexpected_error);
447+
operator=(static_cast<uint32_t>(decimal_out));
448+
if (unlikely(decimal_out > slave_net_timeout*1000ULL))
449+
out_warning= ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE_MAX;
450+
else if (unlikely(!decimal_out && overprecise))
451+
out_warning= ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE_MIN;
452+
return false;
453+
}
408454
bool load_from(IO_CACHE *file) override
409455
{
410-
/// Read in floating point first to validate the range
411-
double seconds;
456+
[[maybe_unused]] uint ignored_warning;
412457
/**
413458
Number of chars Optional_int_field::load_from() uses plus
414459
1 for the decimal point; truncate the excess precision,
@@ -418,20 +463,10 @@ struct Master_info_file: Info_file
418463
size_t length= my_b_gets(file, buf, sizeof(buf));
419464
if (!length)
420465
return true;
421-
#if defined(__clang__) ? _LIBCPP_VERSION < 200000 :\
422-
defined(__GNUC__) && __GNUC__ < 11
423-
// FIXME: floating-point variants of `std::from_chars()` not supported
424-
char end;
425-
if (sscanf(buf, "%lf%c", &seconds, &end) < 2 || end
426-
#else
427-
std::from_chars_result result=
428-
std::from_chars(buf, &buf[length], seconds, std::chars_format::fixed);
429-
if (result.ec != Int_IO_CACHE::ERRC_OK || *(result.ptr)
430-
#endif
431-
!= '\n' || seconds < 0 || seconds > SLAVE_MAX_HEARTBEAT_PERIOD)
432-
return true;
433-
operator=(static_cast<uint32_t>(seconds * 1000));
434-
return false;
466+
char *end= &(buf[length]);
467+
auto decimal= my_decimal();
468+
return str2my_decimal(E_DEC_ERROR, buf, &decimal, &end) || *end != '\n' ||
469+
load_from(decimal, ignored_warning);
435470
}
436471
/**
437472
This method is engineered (that is, hard-coded) to take

sql/share/errmsg-utf8.txt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8482,11 +8482,11 @@ ER_SLAVE_HEARTBEAT_FAILURE
84828482
spa "Datos inesperados de latido (heartbeat) de maestro (master): %s"
84838483
sw "Data isiyotarajiwa ya mapigo ya moyo ya bwana: %s"
84848484
ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE
8485-
chi "心跳周期的请求值是负的或超过允许的最大值(%u秒)"
8486-
eng "The requested value for the heartbeat period is either negative or exceeds the maximum allowed (%u seconds)"
8487-
geo "გულისცემის პერიოდის მოთოხვნილი მნიშვნელობა ან უარყოფითა, ან მაქსიმალურ დაშვებულ მნიშვნელობას (%u წამი) აჭარბებს"
8488-
spa "El valor requerido para el período de latido o es negativo o excede al máximo permitido (%u segundos)"
8489-
sw "Thamani iliyoombwa kwa kipindi cha mpigo wa moyo ni hasi au inazidi kiwango cha juu kinachoruhusiwa (sekunde %u)"
8485+
chi "心跳周期的请求值是负的或超过允许的最大值(%s秒)"
8486+
eng "The requested value for the heartbeat period is either negative or exceeds the maximum allowed (%s seconds)"
8487+
geo "გულისცემის პერიოდის მოთოხვნილი მნიშვნელობა ან უარყოფითა, ან მაქსიმალურ დაშვებულ მნიშვნელობას (%s წამი) აჭარბებს"
8488+
spa "El valor requerido para el período de latido o es negativo o excede al máximo permitido (%s segundos)"
8489+
sw "Thamani iliyoombwa kwa kipindi cha mpigo wa moyo ni hasi au inazidi kiwango cha juu kinachoruhusiwa (sekunde %s)"
84908490
ER_UNUSED_14
84918491
eng "You should never see it"
84928492
geo "ამას ვერ უნდა ხედავდეთ"

sql/slave.cc

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,13 +1905,11 @@ when it try to get the value of TIME_ZONE global variable from master.";
19051905

19061906
if (heartbeat_period)
19071907
{
1908-
const char query_format[]= "SET @master_heartbeat_period= %llu";
1909-
char query[sizeof(query_format) + 32];
1910-
/*
1911-
the period is an ulonglong of nano-secs.
1912-
*/
1913-
my_snprintf(query, sizeof(query), query_format,
1914-
heartbeat_period*1'000'000ULL);
1908+
// The user variable is in nanoseconds
1909+
static const char query_format[]=
1910+
"SET @master_heartbeat_period= %" PRIu32 "000""000";
1911+
char query[sizeof(query_format) + Int_IO_CACHE::BUF_SIZE<uint32_t>];
1912+
my_snprintf(query, sizeof(query), query_format, heartbeat_period);
19151913

19161914
DBUG_EXECUTE_IF("simulate_slave_heartbeat_network_error",
19171915
{ static ulong dbug_count= 0;

sql/sql_const.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@
3333
#define MAX_SYS_VAR_LENGTH 32
3434
#define MAX_KEY MAX_INDEXES /* Max used keys */
3535
#define MAX_REF_PARTS 32 /* Max parts used as ref */
36-
/// @return std::numeric_limits<uint32_t>::max() / 1000
37-
#define SLAVE_MAX_HEARTBEAT_PERIOD 4294967
3836

3937
/*
4038
Maximum length of the data part of an index lookup key.

0 commit comments

Comments
 (0)