[MDEV-27602] Automatic creation of working directories in mtr Created: 2022-01-24  Updated: 2023-11-28

Status: Stalled
Project: MariaDB Server
Component/s: Tests
Fix Version/s: 10.4, 10.5, 10.6, 10.11, 11.0, 11.1

Type: Task Priority: Minor
Reporter: Julius Goryavsky Assignee: Julius Goryavsky
Resolution: Unresolved Votes: 0
Labels: None

Issue Links:
Issue split
split from MDEV-27524 Incorrect binlogs after Galera SST us... Closed

 Description   

Now we have problems with automatically checking the correct operation of the Galera cluster when changing various paths to innodb files, to binary logs, and so on. Checking each such option requires the development of complex tests that stop the entire cluster, manually duplicate the bootstrap process with non-standard paths, then destroy everything and restart the cluster from a .test file with previous settings, which is very inconvenient and potentially masks failures that could be detected on the final stage of the check (if it was done inside mtr).

To solve this problem and create such tests without significant alteration of .test files, we need automatic creation of working directories in mtr - at least for testing a cluster with non-standard path settings. It can also be useful for testing mariabackup and even innodb and maybe other subsystems.



 Comments   
Comment by Julius Goryavsky [ 2022-01-25 ]

Proposed solution: https://github.com/MariaDB/server/commit/6777c260c22e16fb9a76a60f124c2d9337d04585

Comment by Sergei Golubchik [ 2022-02-17 ]

I don't see much of a point. It doesn't "greatly simplify" anything. The patch is

 mysql-test/lib/mtr_cases.pm  |   9 ++++
 mysql-test/mysql-test-run.pl | 126 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 129 insertions(+), 6 deletions(-)

And it allows to write

--- /dev/null
+++ b/mysql-test/suite/galera/t/galera_sst_mariabackup3.dirs
@@ -0,0 +1,2 @@
+${MYSQLTEST_VARDIR}/mysqld.1/node1
+${MYSQLTEST_VARDIR}/mysqld.2/node2

instead of

--- /dev/null
+++ b/mysql-test/suite/galera/t/galera_sst_mariabackup3.sh
@@ -0,0 +1,2 @@
+mkdir -p ${MYSQLTEST_VARDIR}/mysqld.1/node1 
+mkdir -p ${MYSQLTEST_VARDIR}/mysqld.2/node2

which doesn't strike me as a great simplification worth that much new code in the mtr.
You can even do

rm -rf ${MYSQLTEST_VARDIR}/mysqld.1/node1  ${MYSQLTEST_VARDIR}/mysqld.2/node2
mkdir -p ${MYSQLTEST_VARDIR}/mysqld.1/node1  ${MYSQLTEST_VARDIR}/mysqld.2/node2

or

rm -rf ${MYSQLTEST_VARDIR}/mysqld.1/node1  ${MYSQLTEST_VARDIR}/mysqld.2/node2
cp -rf ${MYSQLTEST_VARDIR}/mysqld.1/node1  ${MYSQLTEST_VARDIR}/mysqld.2/node2

whatever you need. There are tests that already do that. And there are only few of them, again, showing that this feature isn't needed in the mtr.

Comment by Julius Goryavsky [ 2022-02-17 ]

serg Thank you very much for the review, but let me disagree with the possibility of using shell scripts without writing dozens of lines of additional code in test files, for the following reasons:

1) Creating directories from shell scripts will do nothing to ensure that their paths would be substituted in the bootstrap process parameters (see changes around the mysql_install_db() call in the mysql_server_start code, especially in the second commit);

2) Currently we do not have the ability to pass parameters to the bootstrap stage from .cnf files;

3) Taking into account (1) and (2) if the base for the test should be created in such a way that some of the innodb files would be in non-standard directories, then shell scripts will not help us - they will be executed with standard parameters in the bootstrap procedure and the created database will not be located in non-standard directories, even if the directories themselves have been created in advance;

4) Because of (3) in the case of Galera, we don't just need to create directories, but we need to:

  • Sequentially stop all cluster nodes;
  • Erase working files like gcache & etc sequentially for each of them;
  • In the test code, create configuration files with a non-standard directory location and restart the server for each of the nodes by passing it the paths to these configuration files;
  • Run tests;
  • Stop the nodes sequentially again;
  • Restart them again on the standard configuration (which was at the time of the start of the test) - otherwise the verification process in mtr will fail due to a state mismatch at the beginning and at the end of the test;
  • Well, create / destroy non-standard directories using shell scripts;
    As a result, these are dozens of lines of code in a .test file.

5) Paragraph (4) actually cancels all mechanisms for checking state discrepancy in mtr (state difference after test completion), this masks errors and assigns this check to the test developer;

6) Using shell scripts immediately ties us to Linux, but we also officially support Windows (not for Galera, but, for example, this is true for mariabackup).

Comment by Julius Goryavsky [ 2022-02-17 ]

serg P.S. I also forgot to say:

(7) All manually made lines for restarting servers on nodes will need to be constantly supported as the system develops - if some new files are added to innodb, the names of old ones change, etc. - tests for Galera will be broken in a way that is not obvious to anyone (after a some time) - while with the proposed changes, the bootstrap process through mysql_install_db() will do everything correctly after innodb changes - and it will not be necessary to change a single line in the .test files for Galera.

Comment by Sergei Golubchik [ 2022-02-18 ]

You're right that shell scripts are invoked too late for bootstrap. This is easily fixed by moving their invocation few lines up.

After doing that and converting all dirs files to shell scripts all your tests from the commit 4e2da16802f (fix for MDEV-27524) pass.

$ git diff --shortstat @
 23 files changed, 50 insertions(+), 172 deletions(-)

So, all tests (that used to use .dirs) pass, mtr is ~120 lines smaller. Looks like a good thing to me.

Comment by Sergei Golubchik [ 2022-02-19 ]

It's good to improve the mtr! But every improvement or a bug fix must be accompanied by a test case. And then there's a question whether the goal can be achieved simpler.

The improvement related to .dirs files — I believe it's not needed, because -master.sh files achieve the same goal, but simpler.

The bug, that they're run too late — you are right, this is a bug, and it needs to be fixed. By moving the rebootstrap code down.

The bug that mysql_install_db() function in mtr does not notice that command line option overrides the value from the cnf file — you are right again, this is a bug, needs to be fixed too. For example, like this:

--- a/mysql-test/mysql-test-run.pl
+++ b/mysql-test/mysql-test-run.pl
@@ -3166,6 +3166,7 @@ sub mysql_install_db {
     {
       my $sql_dir= dirname($path_sql);
       # Use the mysql database for system tables
+      mtr_tofile($bootstrap_sql_file, "create database if not exists mysql;\n");
       mtr_tofile($bootstrap_sql_file, "use mysql;\n");
 
       # Add the offical mysql system tables

After fixing these two bugs all your new galera test cases pass.

If you know of any more mtr bugs, please, show test cases that trigger them and we can discuss how to fix them.

Generated at Thu Feb 08 09:54:10 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.