[#MDEV-27148] running multiple instances of mtr with different var directories can cause .reject and generated from .rdiff .result files overwriting

Comment by Sergei Golubchik [ 2022-01-12 ]

f1b35c7f3eb0 is ok to push, presuming you've tested it and verified that mtr can still find the reject file and prints diffs of failed tests just the same. And that tests with the same name in different suites do not overwrite each others' rejects.

Comment by Vladislav Lesin [ 2022-02-11 ]

I have updated the branch, pushed also the fix for tmp directory to avoid generated from .rdiff .result files overwriting.

The following is code research for the case with the same test names in different suites.

As we can see from

void check_result()

...

  case RESULT_CONTENT_MISMATCH:

/*

      Result mismatched, dump results to .reject file

      and then show the diff

*/

    char reject_file[FN_REFLEN];

    size_t reject_length;

    if (!mess)

      mess= "Result content mismatch\n";

    dirname_part(reject_file, result_file_name, &reject_length);

    /* Put reject file in opt_logdir */

    fn_format(reject_file, result_file_name, opt_logdir,

              ".reject", MY_REPLACE_DIR | MY_REPLACE_EXT);

    if (my_copy(log_file.file_name(), reject_file, MYF(0)) != 0)

      die("Failed to copy '%s' to '%s', errno: %d",

          log_file.file_name(), reject_file, errno);

...

.reject file name is formed from opt_logdir and result_file_name. Both are passed to mysqltest from mysql-test-run.pl.

The arguments for mysqltest are formed here:

sub start_mysqltest ($) {

  my ($tinfo)= @_;

  my $exe= $exe_mysqltest;

  my $args;

...

  mtr_add_arg($args, "--logdir=%s/log", $opt_vardir);

...

  if ( defined $tinfo->{'result_file'} ) {

    mtr_add_arg($args, "--result-file=%s", $tinfo->{'result_file'});

...

$tinfo->{'result_file'}

is formed here:

sub collect_one_test_case {

...

    if ($tinfo->{combinations}) {

      $re = '(?:' . join('|', @{$tinfo->{combinations}}) . ')';

    my $resdirglob = $suite->{rdir};

    $resdirglob.= ',' . $suite->{parent}->{rdir} if $suite->{parent};

    my %files;

    for (<{$resdirglob}/$tname*.{rdiff,result}>) {

      my ($path, $combs, $ext) =

                  m@^(.*)/$tname((?:,$re)*)\.(rdiff|result)$@ or next;

      my @combs = sort split /,/, $combs;

      $files{$_} = join '~', (                # sort files by

        99 - scalar(@combs),                  # number of combinations DESC

        join(',', sort @combs),               # combination names ASC

        $path eq $suite->{rdir} ? 1 : 2,      # overlay first

        $ext eq 'result' ? 1 : 2              # result before rdiff

);

    my @results = sort { $files{$a} cmp $files{$b} } keys %files;

    if (@results) {

      my $result_file = shift @results;

      $tinfo->{result_file} = $result_file;

      if ($result_file =~ /\.rdiff$/) {

        shift @results while $results[0] =~ /\.rdiff$/;

        mtr_error ("$result_file has no corresponding .result file")

          unless @results;

        $tinfo->{base_result} = $results[0];

        if (not $::exe_patch) {

          $tinfo->{skip} = 1;

          $tinfo->{comment} = "requires patch executable";

...

As we can see the "result_name" contains suite name. So the fix for .reject files is safe for the case of the same test names in different suites.

If .rdiff file extension is found, the .result file is generated to the following location:

sub do_before_run_mysqltest($)

  my $tinfo= shift;

  my $resfile= $tinfo->{result_file};

  return unless defined $resfile;

  # Remove old files produced by mysqltest

  die "unsupported result file name $resfile, stoping" unless

         $resfile =~ /^(.*?)((?:,\w+)*)\.(rdiff|result|result~)$/;

  my ($base_file, $suites, $ext)= ($1, $2, $3);

  # if the result file is a diff, make a proper result file

  if ($ext eq 'rdiff') {

    my $base_result = $tinfo->{base_result};

    my $resdir= dirname($resfile);

    # we'll use a separate extension for generated result files

    # to be able to distinguish them from manually created

    # version-controlled results, and to ignore them in git.

    my $dest = "$base_file$suites.result~";

    my @cmd = ($exe_patch);

    if ($^O eq "MSWin32") {

      push @cmd, '--binary';

    push @cmd, (qw/-r - -f -s -o/, $dest, $base_result, $resfile);

    $cmd[-3] = $dest = $opt_tmpdir . '/' . basename($dest);

    run_system(@cmd);

    $tinfo->{result_file} = $dest;

...

i.e. $dest contains suite name too: my $dest = "$base_file$suites.result~";

In the case of several workers, $opt_tmpdir is for formed as following:

sub main {

...

# Create child processes

  my %children;

  for my $child_num (1..$opt_parallel){

    my $child_pid= My::SafeProcess::Base::_safe_fork();

    if ($child_pid == 0){

      $server= undef; # Close the server port in child

      $tests= {}; # Don't need the tests list in child

      # Use subdir of var and tmp unless only one worker

      if ($opt_parallel > 1) {

        set_vardir("$opt_vardir/$child_num");

        $opt_tmpdir= "$opt_tmpdir/$child_num";

...

That is why tmp dirs of different workers must not intersect.

So the fix for .rdiff files as also safe for the case of the same test name in different suites.

[MDEV-27148] running multiple instances of mtr with different var directories can cause .reject and generated from .rdiff .result files overwriting Created: 2021-12-01 Updated: 2023-04-27
Status:	Stalled
Project:	MariaDB Server
Component/s:	Tests
Affects Version/s:	10.2, 10.3, 10.4, 10.5, 10.6, 10.7, 10.8
Fix Version/s:	10.4, 10.5, 10.6

[MDEV-27148] running multiple instances of mtr with different var directories can cause .reject and generated from .rdiff .result files overwriting Created: 2021-12-01 Updated: 2023-04-27