List:Commits« Previous MessageNext Message »
From:Vladislav Vaintroub Date:April 23 2009 11:35am
Subject:bzr commit into mysql-5.1-mtr branch (vvaintroub:2784) Bug#42804
View as plain text  
#At file:///G:/bzr/mysql-5.1-mtr/ based on revid:bjorn.munch@stripped

 2784 Vladislav Vaintroub	2009-04-23
      Bug #42804 --parallel option does not work for MTR under ActiveState 
      perl 
      
      The problem here was the method how MTR gets its unique thread ids.
      Prior to this patch, the method to do it was to maintain a global 
      table of pid,mtr_unique_id) pairs. The table was backed by a text 
      file. The table was cleaned up one in a while and dead processes leaking
      unique_ids were determined with with kill(0) or with scripting tasklist
      on Windows.
      
      This method is flawed specifically on native Windows Perl. fork() is 
      implemented with starting a new thread, give it a syntetic negative PID
      (threadID*(-1)), until this thread creates a new process with exec()
      However,  neither tasklist nor any other native Windows tool can cope with
      negative perl PIDs. This lead to incorrect determination of dead process 
      and reusing already used mtr_unique_id.
      
      The patch introduces alternative portable  method of solving unique-id 
      problem. When a process needs a unique id in range [min...max], it just 
      starts  to open files named min, min+1,...max in a loop . After file is 
      opened, we do non-blocking flock(). When flock() succeeds, process has 
      allocated the ID. When process dies, file is unlocked . Checks for zombies 
      are not necessary.
      
      Since the change would create a co-existence problems with older version
      of MTR, because of different way to calculate IDs, the default ID range
      is changed from 250-299 to 300-349.
      
      Another fix that was necessary enable --parallel option was to serialize 
      spawn() calls on Windows. specifically, IO redirects needed to be protected.
      
      This patch also fixes hanging CRTL-C (as described in Bug #38629) for the
      "new"  MTR. The fix was already in 6.0 and is now downported.

    modified:
      mysql-test/lib/My/SafeProcess/Base.pm
      mysql-test/lib/mtr_report.pm
      mysql-test/lib/mtr_unique.pm
      mysql-test/mysql-test-run.pl
=== modified file 'mysql-test/lib/My/SafeProcess/Base.pm'
--- a/mysql-test/lib/My/SafeProcess/Base.pm	2008-10-08 20:06:10 +0000
+++ b/mysql-test/lib/My/SafeProcess/Base.pm	2009-04-23 11:35:02 +0000
@@ -83,6 +83,13 @@ sub exit_status {
   };
 }
 
+# threads.pm may not exist everywhere, so use only on Windows.
+
+use if $^O eq "MSWin32", "threads";
+use if $^O eq "MSWin32", "threads::shared";
+
+my $win32_spawn_lock :shared;
+
 
 #
 # Create a new process
@@ -104,6 +111,8 @@ sub create_process {
 
   if ($^O eq "MSWin32"){
 
+    lock($win32_spawn_lock);
+
     #printf STDERR "stdin %d, stdout %d, stderr %d\n",
     #    fileno STDIN, fileno STDOUT, fileno STDERR;
 

=== modified file 'mysql-test/lib/mtr_report.pm'
--- a/mysql-test/lib/mtr_report.pm	2009-04-01 11:58:30 +0000
+++ b/mysql-test/lib/mtr_report.pm	2009-04-23 11:35:02 +0000
@@ -30,6 +30,8 @@ our @EXPORT= qw(report_option mtr_print_
 		mtr_report_test);
 
 use mtr_match;
+use My::Platform;
+use POSIX qw[ _exit ];
 require "mtr_io.pl";
 
 my $tot_real_time= 0;
@@ -470,7 +472,14 @@ sub mtr_warning (@) {
 sub mtr_error (@) {
   print STDERR _name(), _timestamp(),
     "mysql-test-run: *** ERROR: ", join(" ", @_), "\n";
-  exit(1);
+  if (IS_WINDOWS)
+  {
+    POSIX::_exit(1);
+  }
+  else
+  {
+    exit(1);
+  }
 }
 
 

=== modified file 'mysql-test/lib/mtr_unique.pm'
--- a/mysql-test/lib/mtr_unique.pm	2009-03-04 10:34:47 +0000
+++ b/mysql-test/lib/mtr_unique.pm	2009-04-23 11:35:02 +0000
@@ -28,32 +28,36 @@ sub msg {
  # print "### unique($$) - ", join(" ", @_), "\n";
 }
 
-my $file;
+my $dir;
 
 if(!IS_WINDOWS)
 {
-  $file= "/tmp/mysql-test-ports";
+  $dir= "/tmp/mysql-unique-ids";
 }
 else
 {
-  $file= $ENV{'TEMP'}."/mysql-test-ports";
+  # Try to use machine-wide directory location for unique IDs,
+  # $ALLUSERSPROFILE . IF it is not available, fallback to $TEMP
+  # which is typically a per-user temporary directory
+  if (exists $ENV{'ALLUSERSPROFILE'} && -w $ENV{'ALLUSERSPROFILE'})
+  {
+    $dir= $ENV{'ALLUSERSPROFILE'}."/mysql-unique-ids";
+  }
+  else
+  {
+    $dir= $ENV{'TEMP'}."/mysql-unique-ids";
+  }
 }
-  
 
-my %mtr_unique_ids;
+my $mtr_unique_fh = undef;
 
-END {
-  my $allocated_id= $mtr_unique_ids{$$};
-  if (defined $allocated_id)
-  {
-    mtr_release_unique_id($allocated_id);
-  }
-  delete $mtr_unique_ids{$$};
+END
+{
+  mtr_release_unique_id();
 }
 
 #
-# Get a unique, numerical ID, given a file name (where all
-# requested IDs are stored), a minimum and a maximum value.
+# Get a unique, numerical ID in a specified range.
 #
 # If no unique ID within the specified parameters can be
 # obtained, return undef.
@@ -61,135 +65,63 @@ END {
 sub mtr_get_unique_id($$) {
   my ($min, $max)= @_;;
 
-  msg("get, '$file', $min-$max");
-
-  die "Can only get one unique id per process!" if $mtr_unique_ids{$$};
+  msg("get $min-$max, $$");
 
-  my $ret = undef;
-  my $changed = 0;
-
-  if(eval("readlink '$file'") || eval("readlink '$file.sem'")) {
-    die 'lock file is a symbolic link';
-  }
+  die "Can only get one unique id per process!" if defined $mtr_unique_fh;
 
-  chmod 0777, "$file.sem";
-  open SEM, ">", "$file.sem" or die "can't write to $file.sem";
-  flock SEM, LOCK_EX or die "can't lock $file.sem";
-  if(! -e $file) {
-    open FILE, ">", $file or die "can't create $file";
-    close FILE;
-  }
 
-  msg("HAVE THE LOCK");
+  # Make sure our ID directory exists
+  if (! -d $dir)
+  {
+    # If there is a file with the reserved
+    # directory name, just delete the file.
+    if (-e $dir)
+    {
+      unlink($dir);
+    }
 
-  if(eval("readlink '$file'") || eval("readlink '$file.sem'")) {
-    die 'lock file is a symbolic link';
-  }
+    mkdir $dir;
+    chmod 0777, $dir;
 
-  chmod 0777, $file;
-  open FILE, "+<", $file or die "can't open $file";
-  #select undef,undef,undef,0.2;
-  seek FILE, 0, 0;
-  my %taken = ();
-  while(<FILE>) {
-    chomp;
-    my ($id, $pid) = split / /;
-    $taken{$id} = $pid;
-    msg("taken: $id, $pid");
-    # Check if process with given pid is alive
-    if(!process_alive($pid)) {
-      print "Removing slot $id used by missing process $pid\n";
-      msg("Removing slot $id used by missing process $pid");
-      delete $taken{$id};
-      $changed++;
+    if(! -d $dir)
+    {
+      die "can't make directory $dir";
     }
   }
-  for(my $i=$min; $i<=$max; ++$i) {
-    if(! exists $taken{$i}) {
-      $ret = $i;
-      $taken{$i} = $$;
-      $changed++;
-      # Remember the id this process got
-      $mtr_unique_ids{$$}= $i;
-      msg(" got $i"); 
-      last;
+
+
+  my $fh;
+  for(my $id = $min; $id <= $max; $id++)
+  {
+    open( $fh, ">$dir/$id");
+    chmod 0666, "$dir/$id";
+    # Try to lock the file exclusively. If lock succeeds, we're done.
+    if (flock($fh, LOCK_EX|LOCK_NB))
+    {
+      # Store file handle - we would need it to release the ID (==unlock the file)
+      $mtr_unique_fh = $fh;
+      return $id;
     }
-  }
-  if($changed) {
-    seek FILE, 0, 0;
-    truncate FILE, 0 or die "can't truncate $file";
-    for my $k (keys %taken) {
-      print FILE $k . ' ' . $taken{$k} . "\n";
+    else
+    {
+      close $fh;
     }
   }
-  close FILE;
-
-  msg("RELEASING THE LOCK");
-  flock SEM, LOCK_UN or warn "can't unlock $file.sem";
-  close SEM;
-
-  return $ret;
+  return undef;
 }
 
 
 #
 # Release a unique ID.
 #
-sub mtr_release_unique_id($) {
-  my ($myid)= @_;
-
-  msg("release, $myid");
-
-
-  if(eval("readlink '$file'") || eval("readlink '$file.sem'")) {
-    die 'lock file is a symbolic link';
-  }
-
-  open SEM, ">", "$file.sem" or die "can't write to $file.sem";
-  flock SEM, LOCK_EX or die "can't lock $file.sem";
-
-  msg("HAVE THE LOCK");
-
-  if(eval("readlink '$file'") || eval("readlink '$file.sem'")) {
-    die 'lock file is a symbolic link';
-  }
-
-  if(! -e $file) {
-    open FILE, ">", $file or die "can't create $file";
-    close FILE;
-  }
-  open FILE, "+<", $file or die "can't open $file";
-  #select undef,undef,undef,0.2;
-  seek FILE, 0, 0;
-  my %taken = ();
-  while(<FILE>) {
-    chomp;
-    my ($id, $pid) = split / /;
-    msg(" taken, $id $pid");
-    $taken{$id} = $pid;
-  }
-
-  if ($taken{$myid} != $$)
+sub mtr_release_unique_id()
+{
+  msg("release $$");
+  if (defined $mtr_unique_fh)
   {
-    msg(" The unique id for this process does not match pid");
+    close $mtr_unique_fh;
+    $mtr_unique_fh = undef;
   }
-
-
-  msg(" removing $myid");
-  delete $taken{$myid};
-  seek FILE, 0, 0;
-  truncate FILE, 0 or die "can't truncate $file";
-  for my $k (keys %taken) {
-    print FILE $k . ' ' . $taken{$k} . "\n";
-  }
-  close FILE;
-
-  msg("RELEASE THE LOCK");
-
-  flock SEM, LOCK_UN or warn "can't unlock $file.sem";
-  close SEM;
-
-  delete $mtr_unique_ids{$$};
 }
 
 

=== modified file 'mysql-test/mysql-test-run.pl'
--- a/mysql-test/mysql-test-run.pl	2009-04-08 12:54:36 +0000
+++ b/mysql-test/mysql-test-run.pl	2009-04-23 11:35:02 +0000
@@ -1326,16 +1326,19 @@ sub set_build_thread_ports($) {
 
   if ( lc($opt_build_thread) eq 'auto' ) {
     my $found_free = 0;
-    $build_thread = 250;	# Start attempts from here
+    $build_thread = 300;	# Start attempts from here
     while (! $found_free)
     {
-      $build_thread= mtr_get_unique_id($build_thread, 299);
+      $build_thread= mtr_get_unique_id($build_thread, 349);
       if ( !defined $build_thread ) {
-	mtr_error("Could not get a unique build thread id");
+        mtr_error("Could not get a unique build thread id");
       }
       $found_free= check_ports_free($build_thread);
       # If not free, release and try from next number
-      mtr_release_unique_id($build_thread++) unless $found_free;
+      if (! $found_free) {
+        mtr_release_unique_id();
+        $build_thread++;
+      }
     }
   }
   else


Attachment: [text/bzr-bundle] bzr/vvaintroub@mysql.com-20090423113502-92tkqhhfpxft3rwy.bundle
Thread
bzr commit into mysql-5.1-mtr branch (vvaintroub:2784) Bug#42804Vladislav Vaintroub23 Apr