MySQL Lists are EOL. Please join:

List:Internals« Previous MessageNext Message »
From:Petr Chardin Date:June 27 2005 11:48am
Subject:bk commit into 5.0 tree (petr:1.1982) BUG#10957
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of cps. When cps does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.1982 05/06/27 15:48:32 petr@stripped +3 -0
  Fix for BUG#10957 "stop instance, issued after flush instances causes IM to crash"

  server-tools/instance-manager/instance_options.cc
    1.21 05/06/27 15:48:21 petr@stripped +2 -0
    fix valgrind warning

  server-tools/instance-manager/instance.h
    1.11 05/06/27 15:48:21 petr@stripped +2 -0
    new functions declared

  server-tools/instance-manager/instance.cc
    1.19 05/06/27 15:48:21 petr@stripped +101 -16
    fix fork_and_monitor behaviour: it should not  rely on the fact that instance object which was used
    when the instances started exists at stop(). Instead it should save the name of the instance and look for it
    in the instance_map when needed.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	petr
# Host:	owlet.
# Root:	/home/cps/mysql/devel/mysql-5.0-10957

--- 1.18/server-tools/instance-manager/instance.cc	2005-06-07 15:46:57 +04:00
+++ 1.19/server-tools/instance-manager/instance.cc	2005-06-27 15:48:21 +04:00
@@ -31,6 +31,13 @@
 #include <m_string.h>
 #include <mysql.h>
 
+
+static void fork_and_monitor(const char *instance_name,
+                             uint instance_name_len,
+                             const char *mysqld_path,
+                             char **argv,
+                             Instance_map *instance_map);
+
 C_MODE_START
 
 /*
@@ -43,13 +50,23 @@
 pthread_handler_decl(proxy, arg)
 {
   Instance *instance= (Instance *) arg;
-  instance->fork_and_monitor();
+  fork_and_monitor(instance->options.instance_name,
+                   instance->options.instance_name_len,
+                   instance->options.mysqld_path,
+                   instance->options.argv,
+                   instance->get_map());
   return 0;
 }
 
 C_MODE_END
 
 
+Instance_map *Instance::get_map()
+{
+  return instance_map;
+}
+
+
 /*
   The method starts an instance.
 
@@ -108,19 +125,90 @@
 }
 
 
-void Instance::fork_and_monitor()
+/*
+  The method sets the crash flag and wakes all waiters on
+  COND_instance_stopped and COND_guardian
+
+  SYNOPSYS
+    set_crash_flag_n_wake_all()
+
+  DESCRIPTION
+    The method is called when an instance is crashed or terminated.
+    In the former case it might indicate that guardian probably should
+    restart it.
+
+  RETURN
+    Function returns no value
+*/
+
+void Instance::set_crash_flag_n_wake_all()
+{
+  /* set instance state to crashed */
+  pthread_mutex_lock(&LOCK_instance);
+  crashed= 1;
+  pthread_mutex_unlock(&LOCK_instance);
+
+  /*
+    Wake connection threads waiting for an instance to stop. This
+    is needed if a user issued command to stop an instance via
+    mysql connection. This is not the case if Guardian stop the thread.
+  */
+  pthread_cond_signal(&COND_instance_stopped);
+  /* wake guardian */
+  pthread_cond_signal(&instance_map->guardian->COND_guardian);
+}
+
+
+/*
+  Fork child, exec an instance and monitor it.
+
+  SYNOPSYS
+    fork_and_monitor()
+
+  DESCRIPTION
+    Fork a child, then exec and monitor it. When the child is dead,
+    find appropriate instance (for this purpose we save its name),
+    set appropriate flags and wake all threads waiting for instance
+    to stop.
+
+  RETURN
+    Function returns no value
+*/
+
+static void fork_and_monitor(const char *instance_name,
+                             uint instance_name_len,
+                             const char *mysqld_path,
+                             char **argv,
+                             Instance_map *instance_map)
 {
+  enum { MAX_INSTANCE_NAME_LEN= 512 };
+  char instance_name_buff[MAX_INSTANCE_NAME_LEN];
   pid_t pid;
-  log_info("starting instance %s", options.instance_name);
+
+  /*
+    Lock instance map to guarantee that no instances are deleted during
+    strmake() and execv() calls.
+  */
+  instance_map->lock();
+
+  /*
+    Save the instance name in the case if Instance object we
+    are using is destroyed. (E.g. by "FLUSH INSTANCES")
+  */
+  strmake(instance_name_buff, instance_name, MAX_INSTANCE_NAME_LEN - 1);
+
+  log_info("starting instance %s", instance_name_buff);
   switch (pid= fork()) {
   case 0:
-    execv(options.mysqld_path, options.argv);
+    execv(mysqld_path, argv);
     /* exec never returns */
     exit(1);
   case -1:
-    log_info("cannot fork() to start instance %s", options.instance_name);
+    log_info("cannot fork() to start instance %s", instance_name);
     return;
   default:
+    instance_map->unlock();
+
     /*
       Here we wait for the child created. This process differs for systems
       running LinuxThreads and POSIX Threads compliant systems. This is because
@@ -141,20 +229,17 @@
       wait(NULL);                               /* LinuxThreads were detected */
     else
       waitpid(pid, NULL, 0);
-    /* set instance state to crashed */
-    pthread_mutex_lock(&LOCK_instance);
-    crashed= 1;
-    pthread_mutex_unlock(&LOCK_instance);
+
+    Instance *instance;
+
+    instance= instance_map->find(instance_name, instance_name_len);
 
     /*
-      Wake connection threads waiting for an instance to stop. This
-      is needed if a user issued command to stop an instance via
-      mysql connection. This is not the case if Guardian stop the thread.
+      If we have such an instance loaded set it's state and wake all waiters.
+      If the instance is missing, this is also fine. Just stop then.
     */
-    pthread_cond_signal(&COND_instance_stopped);
-    /* wake guardian */
-    pthread_cond_signal(&instance_map->guardian->COND_guardian);
-    /* thread exits */
+    if (instance)
+      instance->set_crash_flag_n_wake_all();
     return;
   }
   /* we should never end up here */

--- 1.10/server-tools/instance-manager/instance.h	2005-06-07 15:46:57 +04:00
+++ 1.11/server-tools/instance-manager/instance.h	2005-06-27 15:48:21 +04:00
@@ -42,6 +42,8 @@
   void kill_instance(int signo);
   int is_crashed();
   void fork_and_monitor();
+  void set_crash_flag_n_wake_all();
+  Instance_map *Instance::get_map();
 
 public:
   enum { DEFAULT_SHUTDOWN_DELAY= 35 };

--- 1.20/server-tools/instance-manager/instance_options.cc	2005-06-07 17:57:15 +04:00
+++ 1.21/server-tools/instance-manager/instance_options.cc	2005-06-27 15:48:21 +04:00
@@ -130,6 +130,8 @@
                             version_option, sizeof(version_option)))
     goto err;
 
+  bzero(result, MAX_VERSION_STRING_LENGTH);
+
   rc= parse_output_and_get_value(cmd.buffer, mysqld_path,
                                  result, MAX_VERSION_STRING_LENGTH,
                                  GET_LINE);
Thread
bk commit into 5.0 tree (petr:1.1982) BUG#10957Petr Chardin27 Jun