summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniele Sciascia <daniele.sciascia@galeracluster.com>2017-02-08 23:36:34 +0100
committerJan Lindström <jan.lindstrom@mariadb.com>2017-08-11 12:31:26 +0300
commit364b15c090e7337eb752eec4bea239052f73b2ed (patch)
tree2d886478eaa0a9ef662d3298b839da71131a8127
parent7af44d7aa07e1678bb48d4f4b9ce2c809b611079 (diff)
downloadmariadb-git-364b15c090e7337eb752eec4bea239052f73b2ed.tar.gz
MW-336 Avoid slave threads leaking
This patch fixes two problems that may arise when changing the value of wsrep_slave_threads: 1) Threads may be leaked if wsrep_slave_threads is changed repeatedly. Specifically, when changing the number of slave threads, we keep track of wsrep_slave_count_change, the number of slaves to start / stop. The problem arises when wsrep_slave_count_change is updated before slaves had a chance to exit (threads may take some time to exit, as they exit only after commiting one more replication event). The fix is to update wsrep_slave_count_change such that it reflects the number of threads that already exited or are scheduled to exit. 2) Attempting to set out of range value for wsrep_slave_threads (below 1 / above 512) results in wsrep_slave_count_change to be computed based on the out of range value, even though a warning is generated and wsrep_slave_threads is set to a truncated value. wsrep_slave_count_change is computed in wsrep_slave_threads_check(), which is called before mysql checks for valid range. Fix is to update wsrep_count_change whenever wsrep_slave_threads is updated with a valid value.
-rw-r--r--mysql-test/suite/galera/r/MW-336.result42
-rw-r--r--mysql-test/suite/galera/t/MW-336.test64
-rw-r--r--sql/sys_vars.cc2
-rw-r--r--sql/wsrep_var.cc13
4 files changed, 113 insertions, 8 deletions
diff --git a/mysql-test/suite/galera/r/MW-336.result b/mysql-test/suite/galera/r/MW-336.result
new file mode 100644
index 00000000000..9bdb61c1a9c
--- /dev/null
+++ b/mysql-test/suite/galera/r/MW-336.result
@@ -0,0 +1,42 @@
+CREATE TABLE t1 (f1 INTEGER) Engine=InnoDB;
+SET GLOBAL wsrep_slave_threads = 10;
+SET GLOBAL wsrep_slave_threads = 1;
+INSERT INTO t1 VALUES (1);
+SET GLOBAL wsrep_slave_threads = 10;
+SELECT COUNT(*) = 11 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user';
+COUNT(*) = 11
+1
+SET GLOBAL wsrep_slave_threads = 20;
+SELECT COUNT(*) = 21 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user';
+COUNT(*) = 21
+1
+SET GLOBAL wsrep_slave_threads = 1;
+INSERT INTO t1 VALUES (1);
+INSERT INTO t1 VALUES (2);
+INSERT INTO t1 VALUES (3);
+INSERT INTO t1 VALUES (4);
+INSERT INTO t1 VALUES (5);
+INSERT INTO t1 VALUES (6);
+INSERT INTO t1 VALUES (7);
+INSERT INTO t1 VALUES (8);
+INSERT INTO t1 VALUES (9);
+SET GLOBAL wsrep_slave_threads = 10;
+SET GLOBAL wsrep_slave_threads = 0;
+Warnings:
+Warning 1292 Truncated incorrect wsrep_slave_threads value: '0'
+INSERT INTO t1 VALUES (10);
+INSERT INTO t1 VALUES (11);
+INSERT INTO t1 VALUES (12);
+INSERT INTO t1 VALUES (13);
+INSERT INTO t1 VALUES (14);
+INSERT INTO t1 VALUES (15);
+INSERT INTO t1 VALUES (16);
+INSERT INTO t1 VALUES (17);
+INSERT INTO t1 VALUES (18);
+INSERT INTO t1 VALUES (19);
+INSERT INTO t1 VALUES (20);
+SELECT COUNT(*) = 2 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user';
+COUNT(*) = 2
+1
+SET GLOBAL wsrep_slave_threads = 1;
+DROP TABLE t1;
diff --git a/mysql-test/suite/galera/t/MW-336.test b/mysql-test/suite/galera/t/MW-336.test
new file mode 100644
index 00000000000..79d8951a822
--- /dev/null
+++ b/mysql-test/suite/galera/t/MW-336.test
@@ -0,0 +1,64 @@
+#
+# MW-336 Slave threads may leak if variable wsrep_slave_threads is set repeatedly
+#
+
+--source include/galera_cluster.inc
+--source include/have_innodb.inc
+
+CREATE TABLE t1 (f1 INTEGER) Engine=InnoDB;
+
+--connection node_1
+SET GLOBAL wsrep_slave_threads = 10;
+SET GLOBAL wsrep_slave_threads = 1;
+
+--connection node_2
+INSERT INTO t1 VALUES (1);
+
+--connection node_1
+--sleep 0.5
+SET GLOBAL wsrep_slave_threads = 10;
+--sleep 0.5
+SELECT COUNT(*) = 11 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user';
+
+SET GLOBAL wsrep_slave_threads = 20;
+--sleep 0.5
+SELECT COUNT(*) = 21 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user';
+
+
+SET GLOBAL wsrep_slave_threads = 1;
+
+--connection node_2
+INSERT INTO t1 VALUES (1);
+INSERT INTO t1 VALUES (2);
+INSERT INTO t1 VALUES (3);
+INSERT INTO t1 VALUES (4);
+INSERT INTO t1 VALUES (5);
+INSERT INTO t1 VALUES (6);
+INSERT INTO t1 VALUES (7);
+INSERT INTO t1 VALUES (8);
+INSERT INTO t1 VALUES (9);
+
+
+--connection node_1
+SET GLOBAL wsrep_slave_threads = 10;
+SET GLOBAL wsrep_slave_threads = 0;
+
+--connection node_2
+INSERT INTO t1 VALUES (10);
+INSERT INTO t1 VALUES (11);
+INSERT INTO t1 VALUES (12);
+INSERT INTO t1 VALUES (13);
+INSERT INTO t1 VALUES (14);
+INSERT INTO t1 VALUES (15);
+INSERT INTO t1 VALUES (16);
+INSERT INTO t1 VALUES (17);
+INSERT INTO t1 VALUES (18);
+INSERT INTO t1 VALUES (19);
+INSERT INTO t1 VALUES (20);
+
+--connection node_1
+--sleep 0.5
+SELECT COUNT(*) = 2 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user';
+
+SET GLOBAL wsrep_slave_threads = 1;
+DROP TABLE t1;
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
index 000a424faeb..e234d441b78 100644
--- a/sql/sys_vars.cc
+++ b/sql/sys_vars.cc
@@ -4568,7 +4568,7 @@ static Sys_var_ulong Sys_wsrep_slave_threads(
GLOBAL_VAR(wsrep_slave_threads), CMD_LINE(REQUIRED_ARG),
VALID_RANGE(1, 512), DEFAULT(1), BLOCK_SIZE(1),
&PLock_wsrep_slave_threads, NOT_IN_BINLOG,
- ON_CHECK(wsrep_slave_threads_check),
+ ON_CHECK(NULL),
ON_UPDATE(wsrep_slave_threads_update));
static Sys_var_charptr Sys_wsrep_dbug_option(
diff --git a/sql/wsrep_var.cc b/sql/wsrep_var.cc
index 8a507711daf..9a5c7b86068 100644
--- a/sql/wsrep_var.cc
+++ b/sql/wsrep_var.cc
@@ -36,6 +36,8 @@ const char* wsrep_node_address = 0;
const char* wsrep_node_incoming_address = 0;
const char* wsrep_start_position = 0;
+static long wsrep_prev_slave_threads = wsrep_slave_threads;
+
int wsrep_init_vars()
{
wsrep_provider = my_strdup(WSREP_NONE, MYF(MY_WME));
@@ -497,18 +499,15 @@ void wsrep_node_address_init (const char* value)
wsrep_node_address = (value) ? my_strdup(value, MYF(0)) : NULL;
}
-bool wsrep_slave_threads_check (sys_var *self, THD* thd, set_var* var)
+static void wsrep_slave_count_change_update ()
{
- mysql_mutex_lock(&LOCK_wsrep_slave_threads);
- wsrep_slave_count_change += (var->save_result.ulonglong_value -
- wsrep_slave_threads);
- mysql_mutex_unlock(&LOCK_wsrep_slave_threads);
-
- return 0;
+ wsrep_slave_count_change += (wsrep_slave_threads - wsrep_prev_slave_threads);
+ wsrep_prev_slave_threads = wsrep_slave_threads;
}
bool wsrep_slave_threads_update (sys_var *self, THD* thd, enum_var_type type)
{
+ wsrep_slave_count_change_update();
if (wsrep_slave_count_change > 0)
{
wsrep_create_appliers(wsrep_slave_count_change);