← Back to team overview

maria-developers team mailing list archive

Re: 1eb364f8b3d: MDEV-17421: mtr does not restart the server whose parameters were changed

 

Hi, Julius!

Looks ok, I'll merge it. But without mtr_restart_t1/mtr_restart_t2
tests. You cannot rely on any specific order of test execution, mtr
reorders them as it sees fit.

On Jan 23, Julius Goryavsky wrote:
> revision-id: 1eb364f8b3d (mariadb-10.1.35-84-g1eb364f8b3d)
> parent(s): 3c3c4ae2254
> author: Julius Goryavsky <sysprg@xxxxxxxxx>
> committer: Julius Goryavsky <sysprg@xxxxxxxxx>
> timestamp: 2018-10-10 17:16:34 +0200
> message:
> 
> MDEV-17421: mtr does not restart the server whose parameters were changed
> 
> If a mtr test runs multiple servers and only some of them get
> restarted on whatever reason with new command-line parameters,
> then subsequent mtr test may fail, because no cleanup is performed.
> Replication and Galera test suites are affected.
> 
> In the mtr script, there is a server_need_restart function
> that decides whether we need to start a new mysqld process before
> the new (next) test. If the mysqld parameters were changed in the
> previous test - not necessarily the parameters of the primary mysqld
> server, maybe even the secondary server parameters - this function
> decides to start a new mysqld process. But since it does not remove
> the old (changed) parameters, the new process starts with the
> parameters changed by the *previous* test.
> 
> To correct this error, we must delete the modified process
> parameters after checking that they have been changed during
> the previous test.
> 
> This patch also simplifies and makes more stable the
> galera_drop_database test, during debugging of which this
> problem was detected.
> 
> https://jira.mariadb.org/browse/MDEV-17421
> 
> ---
>  mysql-test/mysql-test-run.pl                       |  1 +
>  mysql-test/suite/galera/disabled.def               |  1 -
>  .../suite/galera/r/galera_mtr_restart_t1.result    |  1 +
>  .../suite/galera/r/galera_mtr_restart_t2.result    |  1 +
>  .../suite/galera/t/galera_drop_database.test       | 26 +++-------------
>  .../suite/galera/t/galera_mtr_restart_t1.test      | 35 ++++++++++++++++++++++
>  .../suite/galera/t/galera_mtr_restart_t2.test      | 13 ++++++++
>  mysql-test/suite/rpl/r/mtr_restart_t1.result       |  5 ++++
>  mysql-test/suite/rpl/r/mtr_restart_t2.result       |  4 +++
>  mysql-test/suite/rpl/t/mtr_restart_t1.test         | 31 +++++++++++++++++++
>  mysql-test/suite/rpl/t/mtr_restart_t2.test         | 18 +++++++++++
>  11 files changed, 113 insertions(+), 23 deletions(-)
> 
> diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl
> index d969d7bf9f6..d3ffd16987d 100755
> --- a/mysql-test/mysql-test-run.pl
> +++ b/mysql-test/mysql-test-run.pl
> @@ -5282,6 +5282,7 @@ sub server_need_restart {
>          exists $server->{'restart_opts'})
>      {
>        my $use_dynamic_option_switch= 0;
> +      delete $server->{'restart_opts'};
>        if (!$use_dynamic_option_switch)
>        {
>  	mtr_verbose_restart($server, "running with different options '" .
> diff --git a/mysql-test/suite/galera/disabled.def b/mysql-test/suite/galera/disabled.def
> index 603031f52b7..464ed6444f9 100644
> --- a/mysql-test/suite/galera/disabled.def
> +++ b/mysql-test/suite/galera/disabled.def
> @@ -34,4 +34,3 @@ partition : MDEV-13881 galera.partition failed in buildbot with wrong result
>  galera_as_slave_replication_budle : MDEV-15785 Test case galera_as_slave_replication_bundle caused debug assertion
>  galera_wan : MDEV-17259: Test failure on galera.galera_wan
>  galera_pc_ignore_sb : MDEV-17357 Test failure on galera.galera_pc_ignore_sb
> -galera_drop_database : test
> diff --git a/mysql-test/suite/galera/r/galera_mtr_restart_t1.result b/mysql-test/suite/galera/r/galera_mtr_restart_t1.result
> new file mode 100644
> index 00000000000..c628a99f1e8
> --- /dev/null
> +++ b/mysql-test/suite/galera/r/galera_mtr_restart_t1.result
> @@ -0,0 +1 @@
> +weight=111
> diff --git a/mysql-test/suite/galera/r/galera_mtr_restart_t2.result b/mysql-test/suite/galera/r/galera_mtr_restart_t2.result
> new file mode 100644
> index 00000000000..0d488f3d174
> --- /dev/null
> +++ b/mysql-test/suite/galera/r/galera_mtr_restart_t2.result
> @@ -0,0 +1 @@
> +weight=1
> diff --git a/mysql-test/suite/galera/t/galera_drop_database.test b/mysql-test/suite/galera/t/galera_drop_database.test
> index 47fe8315198..12d9efea2f9 100644
> --- a/mysql-test/suite/galera/t/galera_drop_database.test
> +++ b/mysql-test/suite/galera/t/galera_drop_database.test
> @@ -9,6 +9,7 @@
>  --let $node_2=node_2
>  --source include/auto_increment_offset_save.inc
>  
> +# Create test database with two sets of the FTS indexes:
>  CREATE DATABASE fts;
>  USE fts;
>  CREATE TABLE fts_t1 (f1 INT PRIMARY KEY AUTO_INCREMENT, f2 VARCHAR(100), FULLTEXT (f2)) ENGINE=InnoDB;
> @@ -23,34 +24,19 @@ DROP TABLE ten;
>  UPDATE fts_t1 SET f2 = 'abcd';
>  UPDATE fts_t2 SET f2 = 'efjh';
>  
> +# Restart the second node:
>  --connection node_2
> -let $wsrep_cluster_address = `SELECT @@global.wsrep_node_incoming_address`;
>  --source include/restart_mysqld.inc
>  
>  --connection node_1
>  --let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size';
>  --source include/wait_condition.inc
>  
> ---let $galera_connection_name = node_2a
> ---let $galera_server_number = 2
> ---source include/galera_connect.inc
> ---connection node_2a
> +--connection node_2
>  --source include/wait_until_ready.inc
>  
> +# Drop the tables and database after nodes restarted:
>  --connection node_1
> ---let $restart_parameters = --wsrep-cluster-address=gcomm://$wsrep_cluster_address
> ---source include/restart_mysqld.inc
> -
> ---connection node_2a
> ---let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size';
> ---source include/wait_condition.inc
> -
> ---let $galera_connection_name = node_1a
> ---let $galera_server_number = 1
> ---source include/galera_connect.inc
> ---connection node_1a
> ---source include/wait_until_ready.inc
> -
>  USE fts;
>  DROP TABLE fts_t1;
>  DROP TABLE fts_t2;
> @@ -58,8 +44,4 @@ SHOW TABLES;
>  DROP DATABASE fts;
>  
>  # Restore original auto_increment_offset values.
> ---let $node_1=node_1a
> ---let $node_2=node_2a
>  --source include/auto_increment_offset_restore.inc
> -
> ---source include/galera_end.inc
> diff --git a/mysql-test/suite/galera/t/galera_mtr_restart_t1.test b/mysql-test/suite/galera/t/galera_mtr_restart_t1.test
> new file mode 100644
> index 00000000000..563d199625a
> --- /dev/null
> +++ b/mysql-test/suite/galera/t/galera_mtr_restart_t1.test
> @@ -0,0 +1,35 @@
> +# This test verifies that mtr will restart the mysqld process,
> +# whose parameters were changed during the test. The verification
> +# itself is carried out in the following galera_mtr_restart_t2
> +# test. If mtr restart the mysqld process, then the pc.weight
> +# value will be reset to the default ("1"), but if there is no
> +# restart, then we will see the changed value ("111") during
> +# the next test.
> +#
> +--source include/galera_cluster.inc
> +--source include/have_innodb.inc
> +
> +# Save original auto_increment_offset values.
> +--let $node_1=node_1
> +--let $node_2=node_2
> +--source include/auto_increment_offset_save.inc
> +
> +--connection node_2
> +--let $restart_parameters = --wsrep_provider_options=pc.weight=111;repl.causal_read_timeout=PT90S;evs.suspect_timeout=PT10S;evs.inactive_timeout=PT30S;evs.install_timeout=PT15S
> +--source include/restart_mysqld.inc
> +
> +--connection node_1
> +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size';
> +--source include/wait_condition.inc
> +
> +--connection node_2
> +--source include/wait_until_ready.inc
> +
> +# Check that the parameter value really changed:
> +--let $gp = `SELECT SUBSTR(@@wsrep_provider_options, LOCATE('pc.weight =', @@wsrep_provider_options) + LENGTH('pc.weight = '))`
> +--let $weight = `SELECT SUBSTR('$gp', 1, LOCATE(';', '$gp') - 1)`
> +--echo weight=$weight
> +
> +# Restore original auto_increment_offset values.
> +--connection node_1
> +--source include/auto_increment_offset_restore.inc
> diff --git a/mysql-test/suite/galera/t/galera_mtr_restart_t2.test b/mysql-test/suite/galera/t/galera_mtr_restart_t2.test
> new file mode 100644
> index 00000000000..fcc1d0515a6
> --- /dev/null
> +++ b/mysql-test/suite/galera/t/galera_mtr_restart_t2.test
> @@ -0,0 +1,13 @@
> +# This test verifies that mtr will restart the mysqld process,
> +# whose parameters were changed during the previous test. If mtr
> +# restart the mysqld process, then the pc.weight value will be
> +# reset to the default ("1"), but if there is no restart, then
> +# we will see the changed value ("111") in this test.
> +#
> +--source include/galera_cluster.inc
> +--source include/have_innodb.inc
> +
> +--connection node_2
> +--let $gp = `SELECT SUBSTR(@@wsrep_provider_options, LOCATE('pc.weight =', @@wsrep_provider_options) + LENGTH('pc.weight = '))`
> +--let $weight = `SELECT SUBSTR('$gp', 1, LOCATE(';', '$gp') - 1)`
> +--echo weight=$weight
> diff --git a/mysql-test/suite/rpl/r/mtr_restart_t1.result b/mysql-test/suite/rpl/r/mtr_restart_t1.result
> new file mode 100644
> index 00000000000..56b64a2fc70
> --- /dev/null
> +++ b/mysql-test/suite/rpl/r/mtr_restart_t1.result
> @@ -0,0 +1,5 @@
> +include/master-slave.inc
> +[connection master]
> +include/rpl_stop_server.inc [server_number=1]
> +new auto_increment_offset=111
> +include/rpl_end.inc
> diff --git a/mysql-test/suite/rpl/r/mtr_restart_t2.result b/mysql-test/suite/rpl/r/mtr_restart_t2.result
> new file mode 100644
> index 00000000000..3c8fe59d607
> --- /dev/null
> +++ b/mysql-test/suite/rpl/r/mtr_restart_t2.result
> @@ -0,0 +1,4 @@
> +include/master-slave.inc
> +[connection master]
> +auto_increment_offset=1
> +include/rpl_end.inc
> diff --git a/mysql-test/suite/rpl/t/mtr_restart_t1.test b/mysql-test/suite/rpl/t/mtr_restart_t1.test
> new file mode 100644
> index 00000000000..3003a49c424
> --- /dev/null
> +++ b/mysql-test/suite/rpl/t/mtr_restart_t1.test
> @@ -0,0 +1,31 @@
> +# This test verifies that mtr will restart the mysqld process,
> +# whose parameters were changed during the test. The verification
> +# itself is carried out in the following mtr_restart_t2 test.
> +# If mtr restart the mysqld process, then the auto_increment_offset
> +# parameter value will be reset to the default ("1"), but if there
> +# is no restart, then we will see the changed value ("111") during
> +# the next test.
> +#
> +--source include/have_binlog_format_row.inc
> +--source include/master-slave.inc
> +
> +connection master;
> +
> +let $auto_increment_offset = `SELECT @@global.auto_increment_offset`;
> +
> +--let $rpl_server_number=1
> +source include/rpl_stop_server.inc;
> +--let $rpl_server_parameters=--auto-increment-offset=111
> +--let $keep_include_silent=1
> +source include/rpl_start_server.inc;
> +--let $keep_include_silent=0
> +
> +let $auto_increment_offset_new = `SELECT @@global.auto_increment_offset`;
> +--echo new auto_increment_offset=$auto_increment_offset_new
> +
> +--disable_query_log
> +--eval SET @@global.auto_increment_offset = $auto_increment_offset;
> +--enable_query_log
> +
> +--connection master
> +--source include/rpl_end.inc
> diff --git a/mysql-test/suite/rpl/t/mtr_restart_t2.test b/mysql-test/suite/rpl/t/mtr_restart_t2.test
> new file mode 100644
> index 00000000000..ab246af6b44
> --- /dev/null
> +++ b/mysql-test/suite/rpl/t/mtr_restart_t2.test
> @@ -0,0 +1,18 @@
> +# This test verifies that mtr will restart the mysqld process,
> +# whose parameters were changed during the previous test. If mtr
> +# restart the mysqld process, then the auto_increment_offsert
> +# parameter value will be reset to the default ("1"), but if there
> +# is no restart, then we will see the changed value ("111") in
> +# this test.
> +#
> +--source include/have_binlog_format_row.inc
> +--source include/master-slave.inc
> +
> +connection master;
> +
> +let $auto_increment_offset = `SELECT @@global.auto_increment_offset`;
> +
> +--echo auto_increment_offset=$auto_increment_offset
> +
> +--connection master
> +--source include/rpl_end.inc
> 
Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx