Hi, Julius, On Dec 27, Julius Goryavsky wrote:
> commit 4d9f8a3c31e > Author: Julius Goryavsky <julius.goryav...@mariadb.com> > Date: Mon Dec 19 10:29:55 2022 +0100 > > diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl > index 12afab4d28c..8df8198fc99 100755 > --- a/mysql-test/mysql-test-run.pl > +++ b/mysql-test/mysql-test-run.pl > @@ -313,7 +313,7 @@ my %mysqld_logs; > my $opt_debug_sync_timeout= 300; # Default timeout for WAIT_FOR actions. > my $warn_seconds = 60; > > -my $rebootstrap_re= > '--innodb[-_](?:page[-_]size|checksum[-_]algorithm|undo[-_]tablespaces|log[-_]group[-_]home[-_]dir|data[-_]home[-_]dir)|data[-_]file[-_]path|force_rebootstrap'; > +my $rebootstrap_re= > '--(?:loose[-_])?(?:innodb[-_](?:page[-_]size|file(?:[-_]format|per[-_]table)|compression[-_](?:default|algorithm)|checksum(?:s|[-_]algorithm)|undo[-_](?:directory|tablespaces)|(?:data|log[-_]group)[-_]home[-_]dir|buffer[-_]pool[-_]filename|data[-_]file[-_]path|default[-_](?:encryption[-_]key[-_]id|page[-_]encryption)|sys[-_]|(?:index|table)[-_]stats)|aria[-_]log[-_](?:dir[-_]path|file[-_]size))|force_rebootstrap'; 1. drop force_rebootstrap please 2. this code was already not very readable, but got fairly ridiculous now. Let's change the approach. E.g. put them all in a hash, like my %rebootstrap_check = map { $_ => 1 } qw( innodb-page-size innodb-file-format ... ); you'll need to normalize an option before looking it up in the hash > > sub testcase_timeout ($) { return $opt_testcase_timeout * 60; } > sub check_timeout ($) { return testcase_timeout($_[0]); } > @@ -2762,7 +2762,29 @@ sub mysql_server_start($) { > return; > } > > - my $datadir= $mysqld->value('datadir'); > + my $extra_opts= get_extra_opts($mysqld, $tinfo); > + > + # The test options can contain the --datadir parameter, but > + # the bootstrap function can only read datadir location from > + # a .cnf file. So we need to parse the options to get the > + # actual location of the data directory, considering both > + # the options and the .cnf file, and then store the resulting > + # value in a "$mysqld" hash - to use later, when we need to > + # know the actual location of the data directory: > + my $datadir=""; > + foreach my $opt ( @$extra_opts ) > + { > + if ($opt =~ /^--datadir=/) { > + $datadir = substr($opt, 10); > + last; strictly speaking, `last` is incorrect, there can be more than one --datadir > + } > + } > + # If datadir is not in the options, then read it from .cnf: > + if (! $datadir) { > + $datadir = $mysqld->value('datadir'); > + } > + $mysqld->{'datadir'} = $datadir; > + > if (not $opt_start_dirty) > { > > @@ -2785,17 +2807,59 @@ sub mysql_server_start($) { > } > } > > + # Run <tname>-master.sh > + if ($mysqld->option('#!run-master-sh') and > + defined $tinfo->{master_sh} and > + run_system('/bin/sh ' . $tinfo->{master_sh}) ) > + { > + $tinfo->{'comment'}= "Failed to execute '$tinfo->{master_sh}'"; > + return 1; > + } > + > + # Run <tname>-slave.sh > + if ($mysqld->option('#!run-slave-sh') and > + defined $tinfo->{slave_sh} and > + run_system('/bin/sh ' . $tinfo->{slave_sh})) > + { > + $tinfo->{'comment'}= "Failed to execute '$tinfo->{slave_sh}'"; > + return 1; > + } > + > my $mysqld_basedir= $mysqld->value('basedir'); > - my $extra_opts= get_extra_opts($mysqld, $tinfo); > > if ( $basedir eq $mysqld_basedir ) > { > if (! $opt_start_dirty) # If dirty, keep possibly grown system db > { > + # Find the name of the current section in the configuration > + # file and its suffix: > + my $section = $mysqld->{name}; > + my $server_name; > + my $suffix = ""; > + if (index($section, '.') != -1) { > + ($server_name, $suffix) = $section =~ > /^\s*([^\s.]+)\s*\.\s*([^\s]+)/; > + } > + else { > + $server_name = $section =~ /^\s*([^\s]+)/; > + } Replace all 11 lines above with: my ($suffix) = ($mysqld->{name} =~ /\.(.*)/); > # Some InnoDB options are incompatible with the default bootstrap. > # If they are used, re-bootstrap > my @rebootstrap_opts; > @rebootstrap_opts = grep {/$rebootstrap_re/o} @$extra_opts if > $extra_opts; > + # Let's store the additional bootstrap options in > + # the environment variable - they may be used later > + # in the mtr tests - for re-bootstrap or run the > + # recovery, etc: > + my $extra_text = "--datadir=$datadir"; that's rather random. Why do you add only this one option, that doesn't make sense. Better handle any additional requirements in the test. > + if (@rebootstrap_opts) { why do you need an if() ? > + $extra_text .= ' '.join(' ', @rebootstrap_opts); > + } > + if ($suffix) { > + $ENV{'MTR_BOOTSTRAP_OPTS_'.$suffix} = $extra_text; > + } > + else { > + $ENV{'MTR_BOOTSTRAP_OPTS'} = $extra_text; Is that even possible? > + } > if (@rebootstrap_opts) > { > mtr_verbose("Re-bootstrap with @rebootstrap_opts"); > diff --git a/mysql-test/suite/galera/disabled.def > b/mysql-test/suite/galera/disabled.def > index 84babda2fa0..18e7c074059 100644 > --- a/mysql-test/suite/galera/disabled.def > +++ b/mysql-test/suite/galera/disabled.def > @@ -28,3 +28,4 @@ query_cache: MDEV-15805 Test failure on galera.query_cache > versioning_trx_id: MDEV-18590: galera.versioning_trx_id: Test failure: > mysqltest: Result content mismatch > galera_wsrep_provider_unset_set: wsrep_provider is read-only for security > reasons > pxc-421: wsrep_provider is read-only for security reasons > +galera_sst_rsync_innodb_nest : MDEV-29591 nesting innodb subdirectories in > datadir causes SST to fail Why do you disable it if the whole commit is about fixing the test? How can I see that the test works? > diff --git > a/mysql-test/suite/galera/r/galera_sst_rsync_innodb__innodb_dir,debug.rdiff > b/mysql-test/suite/galera/r/galera_sst_rsync_innodb__innodb_dir,debug.rdiff > new file mode 100644 > index 00000000000..027f4860a39 > --- /dev/null > +++ > b/mysql-test/suite/galera/r/galera_sst_rsync_innodb__innodb_dir,debug.rdiff > @@ -0,0 +1,114 @@ looks like a wrong file name? > +--- galera_sst_rsync_innodb_nest.result > ++++ galera_sst_rsync_innodb_nest.reject > +@@ -286,3 +286,111 @@ > + DROP TABLE t1; > + COMMIT; Regards, Sergei VP of MariaDB Server Engineering and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp