Skip to content

Commit 42c58b8

Browse files
committed
MDEV-18096 The server would crash when has configs rpl_semi_sync_master_enabled = OFF rpl_semi_sync_master_wait_no_slave = OFF
The patch fixes a fired assert in the semisync master module. The assert caught attempt to switch semisync off (per rpl_semi_sync_master_wait_no_slave = OFF) when it was not even initialized (per rpl_semi_sync_master_enabled = OFF). The switching-off execution branch is relocated under one that executes enable_master() first. A minor cleaup is done to remove the int return from two functions that did not return anything but an error which could not happen in the functions.
1 parent 323e6cd commit 42c58b8

File tree

5 files changed

+38
-19
lines changed

5 files changed

+38
-19
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
include/master-slave.inc
2+
[connection master]
3+
connection master;
4+
CREATE TABLE t1 (a INT);
5+
INSERT INTO t1 SET a=1;
6+
DROP TABLE t1;
7+
connection slave;
8+
include/rpl_end.inc
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
--rpl_semi_sync_master_enabled=0 --rpl_semi_sync_master_wait_no_slave=0
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# The test verifies master crash of MDEV-18096 when the server starts with
2+
# rpl_semi_sync_master_enabled = OFF rpl_semi_sync_master_wait_no_slave = OFF
3+
4+
--source include/master-slave.inc
5+
--source include/have_binlog_format_mixed.inc
6+
7+
--connection master
8+
CREATE TABLE t1 (a INT);
9+
INSERT INTO t1 SET a=1;
10+
DROP TABLE t1;
11+
12+
--sync_slave_with_master
13+
14+
--source include/rpl_end.inc

sql/semisync_master.cc

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ bool Active_tranx::is_tranx_end_pos(const char *log_file_name,
228228
DBUG_RETURN(entry != NULL);
229229
}
230230

231-
int Active_tranx::clear_active_tranx_nodes(const char *log_file_name,
231+
void Active_tranx::clear_active_tranx_nodes(const char *log_file_name,
232232
my_off_t log_file_pos)
233233
{
234234
Tranx_node *new_front;
@@ -307,7 +307,7 @@ int Active_tranx::clear_active_tranx_nodes(const char *log_file_name,
307307
m_trx_front->log_name, (ulong)m_trx_front->log_pos));
308308
}
309309

310-
DBUG_RETURN(0);
310+
DBUG_VOID_RETURN;
311311
}
312312

313313

@@ -371,20 +371,21 @@ int Repl_semi_sync_master::init_object()
371371
{
372372
result = enable_master();
373373
if (!result)
374+
{
374375
result= ack_receiver.start(); /* Start the ACK thread. */
376+
/*
377+
If rpl_semi_sync_master_wait_no_slave is disabled, let's temporarily
378+
switch off semisync to avoid hang if there's none active slave.
379+
*/
380+
if (!rpl_semi_sync_master_wait_no_slave)
381+
switch_off();
382+
}
375383
}
376384
else
377385
{
378386
result = disable_master();
379387
}
380388

381-
/*
382-
If rpl_semi_sync_master_wait_no_slave is disabled, let's temporarily
383-
switch off semisync to avoid hang if there's none active slave.
384-
*/
385-
if (!rpl_semi_sync_master_wait_no_slave)
386-
switch_off();
387-
388389
return result;
389390
}
390391

@@ -961,25 +962,23 @@ int Repl_semi_sync_master::commit_trx(const char* trx_wait_binlog_name,
961962
* the current sending event catches up with last wait position. If it
962963
* does match, semi-sync will be switched on again.
963964
*/
964-
int Repl_semi_sync_master::switch_off()
965+
void Repl_semi_sync_master::switch_off()
965966
{
966-
int result;
967-
968967
DBUG_ENTER("Repl_semi_sync_master::switch_off");
969968

970969
m_state = false;
971970

972971
/* Clear the active transaction list. */
973972
assert(m_active_tranxs != NULL);
974-
result = m_active_tranxs->clear_active_tranx_nodes(NULL, 0);
973+
m_active_tranxs->clear_active_tranx_nodes(NULL, 0);
975974

976975
rpl_semi_sync_master_off_times++;
977976
m_wait_file_name_inited = false;
978977
m_reply_file_name_inited = false;
979978
sql_print_information("Semi-sync replication switched OFF.");
980979
cond_broadcast(); /* wake up all waiting threads */
981980

982-
DBUG_RETURN(result);
981+
DBUG_VOID_RETURN;
983982
}
984983

985984
int Repl_semi_sync_master::try_switch_on(int server_id,

sql/semisync_master.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -343,11 +343,8 @@ class Active_tranx
343343
* position.
344344
* If log_file_name is NULL, everything will be cleared: the sorted
345345
* list and the hash table will be reset to empty.
346-
*
347-
* Return:
348-
* 0: success; non-zero: error
349346
*/
350-
int clear_active_tranx_nodes(const char *log_file_name,
347+
void clear_active_tranx_nodes(const char *log_file_name,
351348
my_off_t log_file_pos);
352349

353350
/* Given a position, check to see whether the position is an active
@@ -449,7 +446,7 @@ class Repl_semi_sync_master
449446
}
450447

451448
/* Switch semi-sync off because of timeout in transaction waiting. */
452-
int switch_off();
449+
void switch_off();
453450

454451
/* Switch semi-sync on when slaves catch up. */
455452
int try_switch_on(int server_id,

0 commit comments

Comments
 (0)