bk commit into 5.1 tree (anozdrin:1.2351) BUG#17486

[Alexander Nozdrin]

Below is the list of changes that have just been committed into a local
5.1 repository of alik. When alik 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@stripped, 2006-10-19 13:12:45+04:00, anozdrin@stripped +10 -0
  Fix for BUG#17486: IM: race condition on exit.
  
  IM uses wait()/waitpid() system calls to wait for started mysqld to stop.
  The problem was that return values of these functions were ignored.
  So, IM assumed that if wait() returns, mysqld is down. However, this can
  be not so, if wait() was interrupted by SIGINT/SIGTERM.
  
  The fix is to call wait()/waitpid() again if the previous call was interrupted.

  server-tools/instance-manager/guardian.cc@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +6 -0
    Be more verbose.

  server-tools/instance-manager/instance.cc@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +30 -14
    1. Register mysqld-monitoring thread in Thread_registry;
    2. Call wait()/waitpid() again if the previous call was
       interrupted by signal.

  server-tools/instance-manager/instance.h@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +3 -1
    Add reference to Thread_registry.

  server-tools/instance-manager/instance_map.cc@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +6 -4
    Pass Thread_registry reference to Instance.

  server-tools/instance-manager/instance_map.h@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +5 -1
    Add reference to Thread_registry.

  server-tools/instance-manager/listener.cc@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +12 -8
    Be more verbose.

  server-tools/instance-manager/log.cc@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +10 -8
    Add pid/thread id to the each log record; split date time fields.

  server-tools/instance-manager/manager.cc@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +22 -1
    Be more verbose.

  server-tools/instance-manager/mysql_connection.cc@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +17 -10
    Eliminate type-conversion warnings.

  server-tools/instance-manager/thread_registry.cc@stripped, 2006-10-19 13:12:41+04:00,
anozdrin@stripped +11 -0
    Be more verbose.

# 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:	anozdrin
# Host:	booka.aliknet
# Root:	/home/alik/MySQL/devel/5.1-rt-bug17486

--- 1.29/server-tools/instance-manager/listener.cc	2006-10-19 13:12:50 +04:00
+++ 1.30/server-tools/instance-manager/listener.cc	2006-10-19 13:12:50 +04:00
@@ -112,6 +112,8 @@ void Listener_thread::run()
 {
   int i, n= 0;
 
+  log_info("Listener_thread: started.");
+
 #ifndef __WIN__
   /* we use this var to check whether we are running on LinuxThreads */
   pid_t thread_pid;
@@ -164,7 +166,7 @@ void Listener_thread::run()
     if (rc == 0 || rc == -1)
     {
       if (rc == -1 && errno != EINTR)
-        log_error("Listener_thread::run(): select() failed, %s",
+        log_error("Listener_thread: select() failed, %s",
                   strerror(errno));
       continue;
     }
@@ -198,7 +200,7 @@ void Listener_thread::run()
 
   /* III. Release all resources and exit */
 
-  log_info("Listener_thread::run(): shutdown requested, exiting...");
+  log_info("Listener_thread: shutdown requested, exiting...");
 
   for (i= 0; i < num_sockets; i++)
     close(sockets[i]);
@@ -209,6 +211,8 @@ void Listener_thread::run()
 
   thread_registry.unregister_thread(&thread_info);
   my_thread_end();
+
+  log_info("Listener_thread: finished.");
   return;
 
 err:
@@ -230,7 +234,7 @@ int Listener_thread::create_tcp_socket()
   int ip_socket= socket(AF_INET, SOCK_STREAM, 0);
   if (ip_socket == INVALID_SOCKET)
   {
-    log_error("Listener_thead::run(): socket(AF_INET) failed, %s",
+    log_error("Listener_thead: socket(AF_INET) failed, %s",
               strerror(errno));
     return -1;
   }
@@ -261,7 +265,7 @@ int Listener_thread::create_tcp_socket()
   if (bind(ip_socket, (struct sockaddr *) &ip_socket_address,
            sizeof(ip_socket_address)))
   {
-    log_error("Listener_thread::run(): bind(ip socket) failed, '%s'",
+    log_error("Listener_thread: bind(ip socket) failed, '%s'",
               strerror(errno));
     close(ip_socket);
     return -1;
@@ -269,7 +273,7 @@ int Listener_thread::create_tcp_socket()
 
   if (listen(ip_socket, LISTEN_BACK_LOG_SIZE))
   {
-    log_error("Listener_thread::run(): listen(ip socket) failed, %s",
+    log_error("Listener_thread: listen(ip socket) failed, %s",
               strerror(errno));
     close(ip_socket);
     return -1;
@@ -294,7 +298,7 @@ create_unix_socket(struct sockaddr_un &u
   int unix_socket= socket(AF_UNIX, SOCK_STREAM, 0);
   if (unix_socket == INVALID_SOCKET)
   {
-    log_error("Listener_thead::run(): socket(AF_UNIX) failed, %s",
+    log_error("Listener_thead: socket(AF_UNIX) failed, %s",
               strerror(errno));
     return -1;
   }
@@ -314,7 +318,7 @@ create_unix_socket(struct sockaddr_un &u
   if (bind(unix_socket, (struct sockaddr *) &unix_socket_address,
            sizeof(unix_socket_address)))
   {
-    log_error("Listener_thread::run(): bind(unix socket) failed, "
+    log_error("Listener_thread: bind(unix socket) failed, "
               "socket file name is '%s', error '%s'",
               unix_socket_address.sun_path, strerror(errno));
     close(unix_socket);
@@ -325,7 +329,7 @@ create_unix_socket(struct sockaddr_un &u
 
   if (listen(unix_socket, LISTEN_BACK_LOG_SIZE))
   {
-    log_error("Listener_thread::run(): listen(unix socket) failed, %s",
+    log_error("Listener_thread: listen(unix socket) failed, %s",
               strerror(errno));
     close(unix_socket);
     return -1;

--- 1.10/server-tools/instance-manager/log.cc	2006-10-19 13:12:50 +04:00
+++ 1.11/server-tools/instance-manager/log.cc	2006-10-19 13:12:50 +04:00
@@ -52,14 +52,16 @@ static inline void log(FILE *file,  cons
   struct tm bd_time;                            // broken-down time
   localtime_r(&now, &bd_time);
 
-  char buff_date[32];
-  sprintf(buff_date, "%02d%02d%02d %2d:%02d:%02d\t",
-                            bd_time.tm_year % 100,
-                            bd_time.tm_mon + 1,
-                            bd_time.tm_mday,
-                            bd_time.tm_hour,
-                            bd_time.tm_min,
-                            bd_time.tm_sec);
+  char buff_date[128];
+  sprintf(buff_date, "[%d/%d] [%02d/%02d/%02d %02d:%02d:%02d] ",
+          (int) getpid(),
+          (int) pthread_self(),
+          bd_time.tm_year % 100,
+          bd_time.tm_mon + 1,
+          bd_time.tm_mday,
+          bd_time.tm_hour,
+          bd_time.tm_min,
+          bd_time.tm_sec);
   /* Format the message */
   char buff_stack[256];
 

--- 1.36/server-tools/instance-manager/manager.cc	2006-10-19 13:12:50 +04:00
+++ 1.37/server-tools/instance-manager/manager.cc	2006-10-19 13:12:50 +04:00
@@ -143,7 +143,8 @@ void manager()
   */
 
   User_map user_map;
-  Instance_map instance_map(Options::Main::default_mysqld_path);
+  Instance_map instance_map(Options::Main::default_mysqld_path,
+                            thread_registry);
   Guardian_thread guardian_thread(thread_registry,
                                   &instance_map,
                                   Options::Main::monitoring_interval);
@@ -294,6 +295,8 @@ void manager()
   */
   pthread_cond_signal(&guardian_thread.COND_guardian);
 
+  log_info("Main loop: started.");
+
   while (!shutdown_complete)
   {
     int signo;
@@ -305,6 +308,20 @@ void manager()
       goto err;
     }
 
+    /*
+      The general idea in this loop is the following:
+        - we are waiting for SIGINT, SIGTERM -- signals that mean we should
+          shutdown;
+        - as shutdown signal is caught, we stop Guardian thread (by calling
+          Guardian_thread::request_shutdown());
+        - as Guardian_thread is stopped, it sends SIGTERM to this thread
+          (by calling Thread_registry::request_shutdown()), so that the
+          my_sigwait() above returns;
+        - as we catch the second SIGTERM, we send signals to all threads
+          registered in Thread_registry (by calling
+          Thread_registry::deliver_shutdown()) and waiting for threads to stop;
+    */
+
 #ifndef __WIN__
 /*
   On some Darwin kernels SIGHUP is delivered along with most
@@ -321,6 +338,8 @@ void manager()
     else
 #endif
     {
+      log_info("Main loop: got shutdown signal.");
+
       if (!guardian_thread.is_stopped())
       {
         guardian_thread.request_shutdown();
@@ -333,6 +352,8 @@ void manager()
       }
     }
   }
+
+  log_info("Main loop: finished.");
 
 err:
   /* delete the pid file */

--- 1.26/server-tools/instance-manager/guardian.cc	2006-10-19 13:12:50 +04:00
+++ 1.27/server-tools/instance-manager/guardian.cc	2006-10-19 13:12:50 +04:00
@@ -251,6 +251,8 @@ void Guardian_thread::run()
   LIST *node;
   struct timespec timeout;
 
+  log_info("Guardian: started.");
+
   thread_registry.register_thread(&thread_info);
 
   my_thread_init();
@@ -278,12 +280,16 @@ void Guardian_thread::run()
                                      &LOCK_guardian, &timeout);
   }
 
+  log_info("Guardian: stopped.");
+
   stopped= TRUE;
   pthread_mutex_unlock(&LOCK_guardian);
   /* now, when the Guardian is stopped we can stop the IM */
   thread_registry.unregister_thread(&thread_info);
   thread_registry.request_shutdown();
   my_thread_end();
+
+  log_info("Guardian: finished.");
 }
 
 

--- 1.39/server-tools/instance-manager/instance.cc	2006-10-19 13:12:50 +04:00
+++ 1.40/server-tools/instance-manager/instance.cc	2006-10-19 13:12:50 +04:00
@@ -34,6 +34,7 @@
 #include "mysql_manager_error.h"
 #include "portability.h"
 #include "priv.h"
+#include "thread_registry.h"
 
 
 const LEX_STRING
@@ -44,7 +45,8 @@ static const int INSTANCE_NAME_PREFIX_LE
 
 
 static void start_and_monitor_instance(Instance_options *old_instance_options,
-                                       Instance_map *instance_map);
+                                       Instance_map *instance_map,
+                                       Thread_registry *thread_registry);
 
 #ifndef __WIN__
 typedef pid_t My_process_info;
@@ -63,7 +65,8 @@ pthread_handler_t proxy(void *arg)
 {
   Instance *instance= (Instance *) arg;
   start_and_monitor_instance(&instance->options,
-                             instance->get_map());
+                             instance->get_map(),
+                             &instance->thread_registry);
   return 0;
 }
 
@@ -99,12 +102,16 @@ static int wait_process(My_process_info 
     thread, but we don't know this one). Or we could use waitpid(), but
     couldn't use wait(), because it could return in any wait() in the program.
   */
-  if (linuxthreads)
-    wait(NULL);                               /* LinuxThreads were detected */
-  else
-    waitpid(*pi, NULL, 0);
 
-  return 0;
+  while (true)
+  {
+    int err_status= linuxthreads ? wait(NULL) : waitpid(*pi, NULL, 0);
+
+    if (err_status < 0 && errno == EINTR)
+      continue;
+
+    return 0;
+  }
 }
 #else
 static int wait_process(My_process_info *pi)
@@ -239,11 +246,19 @@ static int start_process(Instance_option
 */
 
 static void start_and_monitor_instance(Instance_options *old_instance_options,
-                                       Instance_map *instance_map)
+                                       Instance_map *instance_map,
+                                       Thread_registry *thread_registry)
 {
   Instance_name instance_name(&old_instance_options->instance_name);
   Instance *current_instance;
   My_process_info process_info;
+  Thread_info thread_info(pthread_self());
+
+  log_info("Monitoring thread (instance: '%s'): started.",
+           (const char *) instance_name.get_c_str());
+
+  thread_registry->register_thread(&thread_info);
+  my_thread_init();
 
   /*
     Lock instance map to guarantee that no instances are deleted during
@@ -279,7 +294,11 @@ static void start_and_monitor_instance(I
 
   instance_map->unlock();
 
-  return;
+  thread_registry->unregister_thread(&thread_info);
+  my_thread_end();
+
+  log_info("Monitoring thread (instance: '%s'): finished.",
+           (const char *) instance_name.get_c_str());
 }
 
 
@@ -342,10 +361,6 @@ int Instance::start()
   {
     remove_pid();
 
-    /*
-      No need to monitor this thread in the Thread_registry, as all
-      instances are to be stopped during shutdown.
-    */
     pthread_t proxy_thd_id;
     pthread_attr_t proxy_thd_attr;
     int rc;
@@ -403,7 +418,8 @@ void Instance::set_crash_flag_n_wake_all
 
 
 
-Instance::Instance(): crashed(FALSE), configured(FALSE)
+Instance::Instance(Thread_registry &thread_registry_arg):
+  crashed(FALSE), configured(FALSE), thread_registry(thread_registry_arg)
 {
   pthread_mutex_init(&LOCK_instance, 0);
   pthread_cond_init(&COND_instance_stopped, 0);

--- 1.16/server-tools/instance-manager/instance.h	2006-10-19 13:12:50 +04:00
+++ 1.17/server-tools/instance-manager/instance.h	2006-10-19 13:12:50 +04:00
@@ -27,6 +27,7 @@
 #endif
 
 class Instance_map;
+class Thread_registry;
 
 
 /*
@@ -87,7 +88,7 @@ public:
   static bool is_mysqld_compatible_name(const LEX_STRING *name);
 
 public:
-  Instance();
+  Instance(Thread_registry &thread_registry_arg);
 
   ~Instance();
   int init(const LEX_STRING *name_arg);
@@ -120,6 +121,7 @@ public:
 public:
   enum { DEFAULT_SHUTDOWN_DELAY= 35 };
   Instance_options options;
+  Thread_registry &thread_registry;
 
 private:
   /* This attributes is a flag, specifies if the instance has been crashed. */

--- 1.29/server-tools/instance-manager/instance_map.cc	2006-10-19 13:12:50 +04:00
+++ 1.30/server-tools/instance-manager/instance_map.cc	2006-10-19 13:12:50 +04:00
@@ -169,7 +169,7 @@ int Instance_map::process_one_option(con
   if (!(instance= (Instance *) hash_search(&hash, (byte *) group->str,
                                            group->length)))
   {
-    if (!(instance= new Instance()))
+    if (!(instance= new Instance(thread_registry)))
       return 1;
 
     if (instance->init(group) || add_instance(instance))
@@ -213,8 +213,10 @@ int Instance_map::process_one_option(con
 }
 
 
-Instance_map::Instance_map(const char *default_mysqld_path_arg):
-mysqld_path(default_mysqld_path_arg)
+Instance_map::Instance_map(const char *default_mysqld_path_arg,
+                           Thread_registry &thread_registry_arg):
+  mysqld_path(default_mysqld_path_arg),
+  thread_registry(thread_registry_arg)
 {
   pthread_mutex_init(&LOCK_instance_map, 0);
 }
@@ -331,7 +333,7 @@ int Instance_map::remove_instance(Instan
 int Instance_map::create_instance(const LEX_STRING *instance_name,
                                   const Named_value_arr *options)
 {
-  Instance *instance= new Instance();
+  Instance *instance= new Instance(thread_registry);
 
   if (!instance)
   {

--- 1.20/server-tools/instance-manager/instance_map.h	2006-10-19 13:12:50 +04:00
+++ 1.21/server-tools/instance-manager/instance_map.h	2006-10-19 13:12:50 +04:00
@@ -28,6 +28,7 @@
 class Guardian_thread;
 class Instance;
 class Named_value_arr;
+class Thread_registry;
 
 extern int load_all_groups(char ***groups, const char *filename);
 extern void free_groups(char **groups);
@@ -104,7 +105,8 @@ public:
   int create_instance(const LEX_STRING *instance_name,
                       const Named_value_arr *options);
 
-  Instance_map(const char *default_mysqld_path_arg);
+  Instance_map(const char *default_mysqld_path_arg,
+               Thread_registry &thread_registry_arg);
   ~Instance_map();
 
   /*
@@ -130,6 +132,8 @@ private:
   enum { START_HASH_SIZE = 16 };
   pthread_mutex_t LOCK_instance_map;
   HASH hash;
+
+  Thread_registry &thread_registry;
 };
 
 #endif /* INCLUDES_MYSQL_INSTANCE_MANAGER_INSTANCE_MAP_H */

--- 1.16/server-tools/instance-manager/mysql_connection.cc	2006-10-19 13:12:50 +04:00
+++ 1.17/server-tools/instance-manager/mysql_connection.cc	2006-10-19 13:12:50 +04:00
@@ -165,7 +165,7 @@ Mysql_connection_thread::~Mysql_connecti
 
 void Mysql_connection_thread::run()
 {
-  log_info("accepted mysql connection %d", connection_id);
+  log_info("accepted mysql connection %d", (int) connection_id);
 
   my_thread_init();
 
@@ -175,7 +175,7 @@ void Mysql_connection_thread::run()
     return;
   }
 
-  log_info("connection %d is checked successfully", connection_id);
+  log_info("connection %d is checked successfully", (int) connection_id);
 
   vio_keepalive(vio, TRUE);
 
@@ -315,7 +315,7 @@ int Mysql_connection_thread::do_command(
     enum enum_server_command command= (enum enum_server_command)
                                       (uchar) *packet;
     log_info("connection %d: packet_length=%d, command=%d",
-             connection_id, packet_length, command);
+             (int) connection_id, (int) packet_length, (int) command);
     return dispatch_command(command, packet + 1, packet_length - 1);
   }
 }
@@ -325,27 +325,33 @@ int Mysql_connection_thread::dispatch_co
 {
   switch (command) {
   case COM_QUIT:                                // client exit
-    log_info("query for connection %d received quit command", connection_id);
+    log_info("query for connection %d received quit command",
+             (int) connection_id);
     return 1;
   case COM_PING:
-    log_info("query for connection %d received ping command", connection_id);
+    log_info("query for connection %d received ping command",
+             (int) connection_id);
     net_send_ok(&net, connection_id, NULL);
     break;
   case COM_QUERY:
   {
     log_info("query for connection %d : ----\n%s\n-------------------------",
-	     connection_id,packet);
+	     (int) connection_id,
+             (const char *) packet);
     if (Command *command= parse_command(&instance_map, packet))
     {
       int res= 0;
-      log_info("query for connection %d successfully parsed",connection_id);
+      log_info("query for connection %d successfully parsed",
+               (int) connection_id);
       res= command->execute(&net, connection_id);
       delete command;
       if (!res)
-        log_info("query for connection %d executed ok",connection_id);
+        log_info("query for connection %d executed ok",
+                 (int) connection_id);
       else
       {
-        log_info("query for connection %d executed err=%d",connection_id,res);
+        log_info("query for connection %d executed err=%d",
+                 (int) connection_id, (int) res);
         net_send_error(&net, res);
         return 0;
       }
@@ -358,7 +364,8 @@ int Mysql_connection_thread::dispatch_co
     break;
   }
   default:
-    log_info("query for connection %d received unknown command",connection_id);
+    log_info("query for connection %d received unknown command",
+             (int) connection_id);
     net_send_error(&net, ER_UNKNOWN_COM_ERROR);
     break;
   }

--- 1.11/server-tools/instance-manager/thread_registry.cc	2006-10-19 13:12:50 +04:00
+++ 1.12/server-tools/instance-manager/thread_registry.cc	2006-10-19 13:12:50 +04:00
@@ -86,6 +86,9 @@ Thread_registry::~Thread_registry()
 
 void Thread_registry::register_thread(Thread_info *info)
 {
+  log_info("Thread_registry: registering thread %d...",
+           (int) info->thread_id);
+
 #ifndef __WIN__
   struct sigaction sa;
   sa.sa_handler= handle_signal;
@@ -112,11 +115,19 @@ void Thread_registry::register_thread(Th
 
 void Thread_registry::unregister_thread(Thread_info *info)
 {
+  log_info("Thread_registry: unregistering thread %d...",
+           (int) info->thread_id);
+
   pthread_mutex_lock(&LOCK_thread_registry);
   info->prev->next= info->next;
   info->next->prev= info->prev;
+
   if (head.next == &head)
+  {
+    log_info("Thread_registry: thread registry is empty!");
     pthread_cond_signal(&COND_thread_registry_is_empty);
+  }
+
   pthread_mutex_unlock(&LOCK_thread_registry);
 }