Patch for IM in scope of working on BUG#24415: Instance manager test

im_daemon_life_cycle fails randomly.

1. Move IM-angel functionality into a separate file, create Angel class.
2. Be more verbose;
3. Fix typo in FLUSH INSTANCES implementation;
4. Polishing.


mysql-test/r/im_options.result:
  Updated result file.
mysql-test/t/im_cmd_line.imtest:
  Updated test.
server-tools/instance-manager/IMService.cpp:
  Move HandleServiceOptions() into IMService::main().
server-tools/instance-manager/IMService.h:
  Move HandleServiceOptions() into IMService::main().
server-tools/instance-manager/Makefile.am:
  Added angel.cc and angel.h.
server-tools/instance-manager/WindowsService.cpp:
  Initialize class-members in constructor.
server-tools/instance-manager/WindowsService.h:
  Initialize class-members in constructor.
server-tools/instance-manager/commands.cc:
  Return actual error code (ER_OUT_OF_RESOURCES or ER_THERE_IS_ACTIVE_INSTANCE)
  from FLUSH INSTANCES.
server-tools/instance-manager/manager.cc:
  1. Return actual error code from Manager::flush_instances().
  2. Be more verbose.
server-tools/instance-manager/manager.h:
  Return actual error code from Manager::flush_instances().
server-tools/instance-manager/mysqlmanager.cc:
  Move IM-angel functionality into separate file (angel.cc).
server-tools/instance-manager/priv.cc:
  Use return bool datatype instead int{ 0, 1 }.
server-tools/instance-manager/priv.h:
  Use return bool datatype instead int{ 0, 1 }.
server-tools/instance-manager/angel.cc:
  IM-angel functionality.
server-tools/instance-manager/angel.h:
  IM-angel functionality.

mysql-test/r/im_options.result

---------------------------------------------------------------------
-server_id           = 1
-server_id           = 2
---------------------------------------------------------------------
 SHOW VARIABLES LIKE 'server_id';
 Variable_name	Value
 server_id	1
 SHOW INSTANCES;
 instance_name	state
-mysqld1	starting
+mysqld1	XXXXX
 mysqld2	offline
 UNSET mysqld1.server_id;
 ERROR HY000: The instance is active. Stop the instance first
@@ -86,7 +82,7 @@ UNSET mysqld2.aaa, mysqld3.bbb, mysqld2.ccc, mysqld3.ddd;
 --------------------------------------------------------------------
 --------------------------------------------------------------------
 SET mysqld2.aaa, mysqld3.bbb, mysqld.ccc = 0010;
-ERROR HY000: Bad instance name. Check that the instance with such a name exists
+ERROR HY000: Unknown instance name
 --------------------------------------------------------------------
 --------------------------------------------------------------------
 --------------------------------------------------------------------
@@ -98,7 +94,7 @@ ERROR HY000: The instance is active. Stop the instance first
 --------------------------------------------------------------------
 --------------------------------------------------------------------
 UNSET mysqld2.server_id, mysqld3.server_id, mysqld.ccc;
-ERROR HY000: Bad instance name. Check that the instance with such a name exists
+ERROR HY000: Unknown instance name
 --------------------------------------------------------------------
 server_id           = 1
 server_id=2

mysql-test/t/im_cmd_line.imtest

@@ -26,7 +26,7 @@
 --echo
 
 --echo --> Printing out line for 'testuser'...
---exec $IM_EXE --defaults-file="$IM_DEFAULTS_PATH" --print-password-line --username=testuser --password=abc | tail -1
+--exec $IM_EXE --defaults-file="$IM_DEFAULTS_PATH" --print-password-line --username=testuser --password=abc | tail -2 | head -1
 --echo
 
 --echo --> Listing users...
@@ -45,7 +45,7 @@
 --echo
 
 --echo --> Printing out line for 'testuser'...
---exec $IM_EXE --defaults-file="$IM_DEFAULTS_PATH" --print-password-line --username=testuser --password=xyz | tail -1
+--exec $IM_EXE --defaults-file="$IM_DEFAULTS_PATH" --print-password-line --username=testuser --password=xyz | tail -2 | head -1
 --echo
 
 --echo --> Listing users...

server-tools/instance-manager/IMService.cpp

@@ -15,17 +15,19 @@
 
 #include <windows.h>
 #include <signal.h>
-#include "log.h"
-#include "options.h"
+
 #include "IMService.h"
+
+#include "log.h"
 #include "manager.h"
+#include "options.h"
+
+static const char * const IM_SVC_USERNAME= NULL;
+static const char * const IM_SVC_PASSWORD= NULL;
 
 IMService::IMService(void)
+  :WindowsService("MySqlManager", "MySQL Manager")
 {
-  serviceName= "MySqlManager";
-  displayName= "MySQL Manager";
-  username= NULL;
-  password= NULL;
 }
 
 IMService::~IMService(void)
@@ -60,50 +62,63 @@ void IMService::Log(const char *msg)
   log_info(msg);
 }
 
-int HandleServiceOptions()
+int IMService::main()
 {
-  int ret_val= 0;
-
   IMService winService;
 
   if (Options::Service::install_as_service)
   {
     if (winService.IsInstalled())
+    {
       log_info("Service is already installed.");
-    else if (winService.Install())
+      return 1;
+    }
+
+    if (winService.Install(IM_SVC_USERNAME, IM_SVC_PASSWORD))
+    {
       log_info("Service installed successfully.");
+      return 0;
+    }
     else
     {
       log_error("Service failed to install.");
-      ret_val= 1;
+      return 1;
     }
   }
-  else if (Options::Service::remove_service)
+
+  if (Options::Service::remove_service)
   {
-    if (! winService.IsInstalled())
+    if (!winService.IsInstalled())
+    {
       log_info("Service is not installed.");
-    else if (winService.Remove())
+      return 1;
+    }
+
+    if (winService.Remove())
+    {
       log_info("Service removed successfully.");
+      return 0;
+    }
     else
     {
       log_error("Service failed to remove.");
-      ret_val= 1;
+      return 1;
     }
   }
-  else
+
+  log_info("Initializing Instance Manager service...");
+
+  if (!winService.Init())
   {
-    log_info("Initializing Instance Manager service...");
+    log_error("Service failed to initialize.");
 
-    if (!winService.Init())
-    {
-      log_error("Service failed to initialize.");
-      fprintf(stderr,
-              "The service should be started by Windows Service Manager.\n"
-              "The MySQL Manager should be started with '--standalone'\n"
-              "to run from command line.");
-      ret_val= 1;
-    }
+    fprintf(stderr,
+      "The service should be started by Windows Service Manager.\n"
+      "The MySQL Manager should be started with '--standalone'\n"
+      "to run from command line.");
+
+    return 1;
   }
 
-  return ret_val;
+  return 0;
 }

server-tools/instance-manager/IMService.h

@@ -14,11 +14,14 @@
    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA */
 
 #pragma once
-#include "windowsservice.h"
+#include "WindowsService.h"
 
 class IMService: public WindowsService
 {
 public:
+  static int main();
+
+private:
   IMService(void);
   ~IMService(void);
 
@@ -27,5 +30,3 @@ protected:
   void Stop();
   void Run(DWORD argc, LPTSTR *argv);
 };
-
-extern int HandleServiceOptions();

server-tools/instance-manager/Makefile.am

@@ -80,7 +80,9 @@ mysqlmanager_SOURCES=	command.cc command.h mysqlmanager.cc \
                         portability.h \
 			exit_codes.h \
 			user_management_commands.h \
-			user_management_commands.cc
+			user_management_commands.cc \
+			angel.h \
+			angel.cc
 
 mysqlmanager_LDADD=	@CLIENT_EXTRA_LDFLAGS@ \
 			liboptions.la \

server-tools/instance-manager/WindowsService.cpp

@@ -14,19 +14,29 @@
    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA */
 
 #include <windows.h>
-#include <assert.h>
-#include ".\windowsservice.h"
+
+#include "WindowsService.h"
 
 static WindowsService *gService;
 
-WindowsService::WindowsService(void) :
+WindowsService::WindowsService(const char *p_serviceName,
+                               const char *p_displayName) :
   statusCheckpoint(0),
-  serviceName(NULL),
+  serviceName(p_serviceName),
+  displayName(p_displayName),
   inited(FALSE),
   dwAcceptedControls(SERVICE_ACCEPT_STOP),
   debugging(FALSE)
 {
+  DBUG_ASSERT(serviceName != NULL);
+
+  /* TODO: shouldn't we check displayName too (can it really be NULL)? */
+
+  /* WindowsService is assumed to be singleton. Let's assure this. */
+  DBUG_ASSERT(gService == NULL);
+
   gService= this;
+
   status.dwServiceType= SERVICE_WIN32_OWN_PROCESS;
   status.dwServiceSpecificExitCode= 0;
 }
@@ -35,7 +45,7 @@ WindowsService::~WindowsService(void)
 {
 }
 
-BOOL WindowsService::Install()
+BOOL WindowsService::Install(const char *username, const char *password)
 {
   bool ret_val= FALSE;
   SC_HANDLE newService;
@@ -70,7 +80,7 @@ BOOL WindowsService::Install()
 
 BOOL WindowsService::Init()
 {
-  assert(serviceName != NULL);
+  DBUG_ASSERT(serviceName != NULL);
 
   if (inited)
     return TRUE;
@@ -207,7 +217,7 @@ void WindowsService::HandleControlCode(DWORD opcode)
 
 void WINAPI WindowsService::ServiceMain(DWORD argc, LPTSTR *argv)
 {
-  assert(gService != NULL);
+  DBUG_ASSERT(gService != NULL);
 
   // register our service control handler:
   gService->RegisterAndRun(argc, argv);
@@ -215,7 +225,7 @@ void WINAPI WindowsService::ServiceMain(DWORD argc, LPTSTR *argv)
 
 void WINAPI WindowsService::ControlHandler(DWORD opcode)
 {
-  assert(gService != NULL);
+  DBUG_ASSERT(gService != NULL);
 
   return gService->HandleControlCode(opcode);
 }

server-tools/instance-manager/WindowsService.h

@@ -21,8 +21,6 @@ protected:
   bool                  inited;
   const char            *serviceName;
   const char            *displayName;
-  const char            *username;
-  const char            *password;
   SERVICE_STATUS_HANDLE statusHandle;
   DWORD                 statusCheckpoint;
   SERVICE_STATUS        status;
@@ -30,7 +28,7 @@ protected:
   bool                  debugging;
 
 public:
-  WindowsService(void);
+  WindowsService(const char *p_serviceName, const char *p_displayName);
   ~WindowsService(void);
 
   BOOL  Install();

server-tools/instance-manager/angel.cc

+/* Copyright (C) 2003-2006 MySQL AB
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+
+#ifndef __WIN__
+
+#include "angel.h"
+
+#include <signal.h>
+#include <string.h>
+
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+/*
+  Include my_global.h right after system includes so that we can change
+  system defines if needed.
+*/
+
+#include "my_global.h"
+
+/* Include other IM files. */
+
+#include "log.h"
+#include "manager.h"
+#include "options.h"
+#include "priv.h"
+
+/************************************************************************/
+
+enum { CHILD_OK= 0, CHILD_NEED_RESPAWN, CHILD_EXIT_ANGEL };
+
+static int log_fd;
+
+static volatile sig_atomic_t child_status= CHILD_OK;
+static volatile sig_atomic_t shutdown_request_signo= 0;
+
+
+/************************************************************************/
+/**
+  Open log file.
+
+  @return
+    TRUE  on error;
+    FALSE on success.
+*************************************************************************/
+
+static bool open_log_file()
+{
+  log_info("Angel: opening log file '%s'...",
+           (const char *) Options::Daemon::log_file_name);
+
+  log_fd= open(Options::Daemon::log_file_name,
+               O_WRONLY | O_CREAT | O_APPEND | O_NOCTTY,
+               S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+
+  if (log_fd < 0)
+  {
+    log_error("Can not open log file '%s': %s.",
+              (const char *) Options::Daemon::log_file_name,
+              (const char *) strerror(errno));
+
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+
+/************************************************************************/
+/**
+  Detach the process from controlling tty.
+
+  @return
+    TRUE on error;
+    FALSE on success.
+*************************************************************************/
+
+static bool detach_process()
+{
+  /*
+    Become a session leader (the goal is not to have a controlling tty).
+
+    setsid() must succeed because child is guaranteed not to be a process
+    group leader (it belongs to the process group of the parent).
+
+    NOTE: if we now don't have a controlling tty we will not receive
+    tty-related signals - no need to ignore them.
+  */
+
+  if (setsid() < 0)
+  {
+    log_error("setsid() failed: %s.", (const char *) strerror(errno));
+    return -1;
+  }
+
+  /* Close STDIN. */
+
+  log_info("Angel: preparing standard streams.");
+
+  if (close(STDIN_FILENO) < 0)
+  {
+    log_error("Warning: can not close stdin (%s)."
+              "Trying to continue...",
+              (const char *) strerror(errno));
+  }
+
+  /* Dup STDOUT and STDERR to the log file. */
+
+  if (dup2(log_fd, STDOUT_FILENO) < 0 ||
+      dup2(log_fd, STDERR_FILENO) < 0)
+  {
+    log_error("Can not redirect stdout and stderr to the log file: %s.",
+              (const char *) strerror(errno));
+
+    return TRUE;
+  }
+
+  if (log_fd != STDOUT_FILENO && log_fd != STDERR_FILENO)
+  {
+    if (close(log_fd) < 0)
+    {
+      log_error("Can not close original log file handler (%d): %s. "
+                "Trying to continue...",
+                (int) log_fd,
+                (const char *) strerror(errno));
+    }
+  }
+
+  return FALSE;
+}
+
+
+/************************************************************************/
+/**
+  Create PID file.
+
+  @return
+    TRUE  on error;
+    FALSE on success.
+*************************************************************************/
+
+static bool create_pid_file()
+{
+  if (create_pid_file(Options::Daemon::angel_pid_file_name, getpid()))
+  {
+    log_error("Angel: can not create pid file (%s).",
+              (const char *) Options::Daemon::angel_pid_file_name);
+
+    return TRUE;
+  }
+
+  log_info("Angel: pid file (%s) created.",
+           (const char *) Options::Daemon::angel_pid_file_name);
+
+  return FALSE;
+}
+
+
+/************************************************************************/
+/**
+  SIGCHLD handler.
+
+  Reap child, analyze child exit status, and set child_status
+  appropriately.
+*************************************************************************/
+
+void reap_child(int __attribute__((unused)) signo)
+{
+  /* NOTE: As we have only one child, no need to cycle waitpid(). */
+
+  int child_exit_status;
+
+  if (waitpid(0, &child_exit_status, WNOHANG) > 0)
+  {
+    child_status= WIFSIGNALED(child_exit_status) ?
+                  CHILD_NEED_RESPAWN :
+                  CHILD_EXIT_ANGEL;
+  }
+}
+
+
+/************************************************************************/
+/**
+  SIGTERM, SIGHUP, SIGINT handler.
+
+  Set termination status and return.
+*************************************************************************/
+
+void terminate(int signo)
+{
+  shutdown_request_signo= signo;
+}
+
+
+/************************************************************************/
+/**
+  Angel main loop.
+
+  @return
+    The function returns exit status for global main():
+      0  -- program completed successfully;
+      !0 -- error occurred.
+*************************************************************************/
+
+static int angel_main_loop()
+{
+  /*
+    Install signal handlers.
+
+    NOTE: Although signal handlers are needed only for parent process
+    (IM-angel), we should install them before fork() in order to avoid race
+    condition (i.e. to be sure, that IM-angel will receive SIGCHLD in any
+    case).
+  */
+
+  sigset_t wait_for_signals_mask;
+
+  struct sigaction sa_chld;
+  struct sigaction sa_term;
+  struct sigaction sa_chld_orig;
+  struct sigaction sa_term_orig;
+  struct sigaction sa_int_orig;
+  struct sigaction sa_hup_orig;
+
+  log_info("Angel: setting necessary signal actions...");
+
+  sigemptyset(&wait_for_signals_mask);
+
+  sigemptyset(&sa_chld.sa_mask);
+  sa_chld.sa_handler= reap_child;
+  sa_chld.sa_flags= SA_NOCLDSTOP;
+
+  sigemptyset(&sa_term.sa_mask);
+  sa_term.sa_handler= terminate;
+  sa_term.sa_flags= 0;
+
+  /* NOTE: sigaction() fails only if arguments are wrong. */
+
+  DBUG_ASSERT(!sigaction(SIGCHLD, &sa_chld, &sa_chld_orig));
+  DBUG_ASSERT(!sigaction(SIGTERM, &sa_term, &sa_term_orig));
+  DBUG_ASSERT(!sigaction(SIGINT, &sa_term, &sa_int_orig));
+  DBUG_ASSERT(!sigaction(SIGHUP, &sa_term, &sa_hup_orig));
+
+  /* The main Angel loop. */
+
+  while (true)
+  {
+    /* Spawn a new Manager. */
+
+    log_info("Angel: forking Manager process...");
+
+    switch (fork()) {
+    case -1:
+      log_error("Angel: can not fork IM-main: %s.",
+                (const char *) strerror(errno));
+
+      return -1;
+
+    case 0:
+      /*
+        We are in child process, which will be IM-main:
+          - Restore default signal actions to let the IM-main work with
+            signals as he wishes;
+          - Call Manager::main();
+      */
+
+      log_info("Angel: Manager process created successfully.");
+
+      /* NOTE: sigaction() fails only if arguments are wrong. */
+
+      DBUG_ASSERT(!sigaction(SIGCHLD, &sa_chld_orig, NULL));
+      DBUG_ASSERT(!sigaction(SIGTERM, &sa_term_orig, NULL));
+      DBUG_ASSERT(!sigaction(SIGINT, &sa_int_orig, NULL));
+      DBUG_ASSERT(!sigaction(SIGHUP, &sa_hup_orig, NULL));
+
+      log_info("Angel: executing Manager...");
+
+      return Manager::main();
+    }
+
+    /* Wait for signals. */
+
+    log_info("Angel: waiting for signals...");
+
+    while (child_status == CHILD_OK && shutdown_request_signo == 0)
+      sigsuspend(&wait_for_signals_mask);
+
+    /* Exit if one of shutdown signals has been caught. */
+
+    if (shutdown_request_signo)
+    {
+      log_info("Angel: received shutdown signal (%d). Exiting...",
+               (int) shutdown_request_signo);
+
+      return 0;
+    }
+
+    /* Manager process died. Respawn it if it was a failure. */
+
+    if (child_status == CHILD_NEED_RESPAWN)
+    {
+      child_status= CHILD_OK;
+
+      log_error("Angel: Manager exited abnormally.");
+
+      log_info("Angel: sleeping 1 second...");
+
+      sleep(1); /* don't respawn too fast */
+
+      log_info("Angel: respawning Manager...");
+
+      continue;
+    }
+
+    log_info("Angel: Manager exited normally. Exiting...");
+
+    return 0;
+  }
+}
+
+
+/************************************************************************/
+/**
+  Angel main function.
+
+  @return
+    The function returns exit status for global main():
+      0  -- program completed successfully;
+      !0 -- error occurred.
+*************************************************************************/
+
+int Angel::main()
+{
+  int ret_status;
+
+  log_info("Angel: started.");
+
+  /* Open log file. */
+
+  if (open_log_file())
+    return -1;
+
+  /* Fork a new process. */
+
+  log_info("Angel: daemonizing...");
+
+  switch (fork()) {
+  case -1:
+    /*
+      This is the main Instance Manager process, fork() failed.
+      Log an error and bail out with error code.
+    */
+
+    log_error("fork() failed: %s.", (const char *) strerror(errno));
+    return -1;
+
+  case 0:
+    /* We are in child process. Continue Angel::main() execution. */
+
+    break;
+
+  default:
+    /*
+      We are in the parent process. Return 0 so that parent exits
+      successfully.
+    */
+
+    log_info("Angel: exiting from the original process...");
+
+    return 0;
+  }
+
+  /* Detach child from controlling tty. */
+
+  if (detach_process())
+    return -1;
+
+  /* Create PID file. */
+
+  if (create_pid_file())
+    return -1;
+
+  /* Start Angel main loop. */
+
+  return angel_main_loop();
+}
+
+#endif // __WIN__

server-tools/instance-manager/angel.h

+/* Copyright (C) 2003-2006 MySQL AB
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+
+#ifndef INCLUDES_MYSQL_ANGEL_H
+#define INCLUDES_MYSQL_ANGEL_H
+
+#ifndef __WIN__
+
+#if defined(__GNUC__) && defined(USE_PRAGMA_INTERFACE)
+#pragma interface
+#endif
+
+#include <my_global.h>
+
+class Angel
+{
+public:
+  static int main();
+};
+
+#endif // INCLUDES_MYSQL_ANGEL_H
+#endif // __WIN__

server-tools/instance-manager/commands.cc

@@ -210,8 +210,10 @@ int Show_instances::write_data(st_net *net)
 
 int Flush_instances::execute(st_net *net, ulong connection_id)
 {
-  if (Manager::flush_instances())
-    return ER_OUT_OF_RESOURCES;
+  int err_status= Manager::flush_instances();
+
+  if (err_status)
+    return err_status;
 
   return net_send_ok(net, connection_id, NULL) ? ER_OUT_OF_RESOURCES : 0;
 }

server-tools/instance-manager/manager.cc

@@ -29,6 +29,8 @@
 #include "guardian.h"
 #include "instance_map.h"
 #include "listener.h"
+#include "mysql_manager_error.h"
+#include "mysqld_error.h"
 #include "log.h"
 #include "options.h"
 #include "priv.h"
@@ -205,14 +207,16 @@ int Manager::main()
   bool shutdown_complete= FALSE;
   pid_t manager_pid= getpid();
 
+  log_info("Manager: initializing...");
+
 #ifndef __WIN__
   if (check_if_linux_threads(&linux_threads))
   {
-    log_error("Can not determine thread model.");
+    log_error("Manager: can not determine thread model.");
     return 1;
   }
 
-  log_info("Detected threads model: %s.",
+  log_info("Manager: detected threads model: %s.",
            (const char *) (linux_threads ? "LINUX threads" : "POSIX threads"));
 #endif // __WIN__
 
@@ -250,7 +254,7 @@ int Manager::main()
 
   if (instance_map.init())
   {
-    log_error("Can not initialize instance list: out of memory.");
+    log_error("Manager: can not initialize instance list: out of memory.");
     return 1;
   }
 
@@ -258,7 +262,7 @@ int Manager::main()
 
   if (user_map.init())
   {
-    log_error("Can not initialize user list: out of memory.");
+    log_error("Manager: can not initialize user list: out of memory.");
     return 1;
   }
 
@@ -277,20 +281,19 @@ int Manager::main()
     }
     else
     {
-      log_error("%s.", (const char *) err_msg);
+      log_error("Manager: %s.", (const char *) err_msg);
       return 1;
     }
   }
 
   /* Write Instance Manager pid file. */
 
-  log_info("IM pid file: '%s'; PID: %d.",
-           (const char *) Options::Main::pid_file_name,
-           (int) manager_pid);
-
   if (create_pid_file(Options::Main::pid_file_name, manager_pid))
     return 1; /* necessary logging has been already done. */
 
+  log_info("Manager: pid file (%s) created.",
+           (const char *) Options::Main::pid_file_name);
+
   /*
     Initialize signals and alarm-infrastructure.
 
@@ -326,7 +329,7 @@ int Manager::main()
 
   if (guardian.start(Thread::DETACHED))
   {
-    log_error("Can not start Guardian thread.");
+    log_error("Manager: can not start Guardian thread.");
     goto err;
   }
 
@@ -334,7 +337,7 @@ int Manager::main()
 
   if (Manager::flush_instances())
   {
-    log_error("Can not init instances repository.");
+    log_error("Manager: can not init instances repository.");
     stop_all_threads();
     goto err;
   }
@@ -343,7 +346,7 @@ int Manager::main()
 
   if (listener.start(Thread::DETACHED))
   {
-    log_error("Can not start Listener thread.");
+    log_error("Manager: can not start Listener thread.");
     stop_all_threads();
     goto err;
   }
@@ -366,7 +369,7 @@ int Manager::main()
 
     if ((status= my_sigwait(&mask, &signo)) != 0)
     {
-      log_error("sigwait() failed");
+      log_error("Manager: sigwait() failed");
       stop_all_threads();
       goto err;
     }
@@ -426,7 +429,6 @@ err:
 #ifndef __WIN__
   /* free alarm structures */
   end_thr_alarm(1);
-  /* don't pthread_exit to kill all threads who did not shut down in time */
 #endif
   return rc;
 }
@@ -460,34 +462,41 @@ err:
 
     In order to avoid such side effects one should never call
     FLUSH INSTANCES without prior stop of all running instances.
+
+  RETURN
+    0                           On success
+    ER_OUT_OF_RESOURCES         Not enough resources to complete the operation
+    ER_THERE_IS_ACTIVE_INSTACE  If there is an active instance
 */
 
-bool Manager::flush_instances()
+int Manager::flush_instances()
 {
   p_instance_map->lock();
 
   if (p_instance_map->is_there_active_instance())
   {
     p_instance_map->unlock();
-    return TRUE;
+    return ER_THERE_IS_ACTIVE_INSTACE;
   }
 
   if (p_instance_map->reset())
   {
     p_instance_map->unlock();
-    return TRUE;
+    return ER_OUT_OF_RESOURCES;
   }
 
   if (p_instance_map->load())
   {
     p_instance_map->unlock();
-    return TRUE; /* Don't init guardian if we failed to load instances. */
+
+    /* Don't init guardian if we failed to load instances. */
+    return ER_OUT_OF_RESOURCES;
   }
 
-  get_guardian()->init(); /* TODO: check error status. */
+  get_guardian()->init();
   get_guardian()->ping();
 
   p_instance_map->unlock();
 
-  return FALSE;
+  return 0;
 }

server-tools/instance-manager/manager.h

@@ -32,7 +32,7 @@ class Manager
 public:
   static int main();
 
-  static bool flush_instances();
+  static int flush_instances();
 
 public:
   /**

server-tools/instance-manager/mysqlmanager.cc

@@ -17,140 +17,138 @@
 #include <my_sys.h>
 
 #include <string.h>
-#include <signal.h>
-#include <sys/types.h>
-#include <sys/stat.h>
 
 #ifndef __WIN__
 #include <pwd.h>
 #include <grp.h>
-#include <sys/wait.h>
 #endif
 
+#include "angel.h"
 #include "log.h"
 #include "manager.h"
 #include "options.h"
-#include "priv.h"
 #include "user_management_commands.h"
 
 #ifdef __WIN__
 #include "IMService.h"
-#include "WindowsService.h"
 #endif
 
 
 /*
-  Few notes about Instance Manager architecture:
-  Instance Manager consisits of two processes: the angel process, and the
-  instance manager process. Responsibilities of the angel process is to
-  monitor the instance manager process, and restart it in case of
-  failure/shutdown. The angel process is started only if startup option
-  '--run-as-service' is provided.
-  The Instance Manager process consists of several
-  subsystems (thread sets):
-  - the signal handling thread: it's responsibilities are to handle
-    user signals and propogate them to the other threads. All other threads
-    are accounted in the signal handler thread Thread Registry.
-  - the listener: listens all sockets. There is a listening
-    socket for each (mysql, http, snmp, rendezvous (?)) subsystem.
-  - mysql subsystem: Instance Manager acts like an ordinary MySQL Server,
-    but with very restricted command set. Each MySQL client connection is
-    handled in a separate thread. All MySQL client connections threads
-    constitute mysql subsystem.
-  - http subsystem: it is also possible to talk with Instance Manager via
-    http. One thread per http connection is used. Threads are pooled.
-  - 'snmp' connections (FIXME: I know nothing about it yet)
-  - rendezvous threads
+  Instance Manager consists of two processes: the angel process (IM-angel),
+  and the manager process (IM-main). Responsibilities of IM-angel is to
+  monitor IM-main, and restart it in case of failure/shutdown. IM-angel is
+  started only if startup option '--run-as-service' is provided.
+
+  IM-main consists of several subsystems (thread sets):
+
+    - the signal handling thread
+
+      The signal thread handles user signals and propagates them to the
+      other threads. All other threads are accounted in the signal handler
+      thread Thread Registry.
+
+    - the listener
+
+      The listener listens to all sockets. There is a listening socket for
+      each subsystem (TCP/IP, UNIX socket).
+
+    - mysql subsystem
+
+      Instance Manager acts like an ordinary MySQL Server, but with very
+      restricted command set. Each MySQL client connection is handled in a
+      separate thread. All MySQL client connections threads constitute
+      mysql subsystem.
 */
 
-static void init_environment(char *progname);
+static int im_main(int argc, char *argv[]);
 
 #ifndef __WIN__
-static void daemonize(const char *log_file_name);
-static void angel();
-static struct passwd *check_user(const char *user);
-static int set_user(const char *user, struct passwd *user_info);
+static struct passwd *check_user();
+static bool switch_user();
 #endif
 
 
-/*
-  main, entry point
-  - init environment
-  - handle options
-  - daemonize and run angel process (if necessary)
-  - run manager process
-*/
+/************************************************************************/
+/**
+  The entry point.
+*************************************************************************/
 
 int main(int argc, char *argv[])
 {
-  int return_value= 1;
-  init_environment(argv[0]);
+  int return_value;
 
-  if ((return_value= Options::load(argc, argv)))
-    goto main_end;
+  /* Initialize. */
 
-  if (Options::User_management::cmd)
-  {
-    return_value= Options::User_management::cmd->execute();
+  MY_INIT(argv[0]);
+  log_init();
+  umask(0117);
+  srand((unsigned int) time(0));
 
-    goto main_end;
-  }
+  /* Main function. */
 
-#ifndef __WIN__
+  log_info("IM: started.");
 
-  struct passwd *user_info;
+  return_value= im_main(argc, argv);
 
-  if ((user_info= check_user(Options::Daemon::user)))
-  {
-      if (set_user(Options::Daemon::user, user_info))
-      {
-        return_value= 1;
-        goto main_end;
-      }
-  }
+  log_info("IM: finished.");
 
-  if (Options::Daemon::run_as_service)
-  {
-    /* forks, and returns only in child */
-    daemonize(Options::Daemon::log_file_name);
-    /* forks again, and returns only in child: parent becomes angel */
-    angel();
-  }
+  /* Cleanup. */
 
-  (void) Manager::main(); /* ignore the return value for now */
+  Options::cleanup();
+  my_end(0);
 
-#else
+  return return_value;
+}
 
-  if (!Options::Service::stand_alone)
-  {
-    if (HandleServiceOptions())
-    {
-      return_value= 1;
-      goto main_end;
-    }
-  }
-  else
-  {
-    (void) Manager::main(); /* ignore the return value for now */
-  }
 
-#endif
+/************************************************************************/
+/**
+  Instance Manager main functionality.
+*************************************************************************/
 
-  return_value= 0;
+int im_main(int argc, char *argv[])
+{
+  int rc;
 
-main_end:
-  Options::cleanup();
-  my_end(0);
-  return return_value;
+  if ((rc= Options::load(argc, argv)))
+    return rc;
+
+  if (Options::User_management::cmd)
+    return Options::User_management::cmd->execute();
+
+#ifndef __WIN__
+
+  if (switch_user())
+    return 1;
+
+  return Options::Daemon::run_as_service ?
+         Angel::main() :
+         Manager::main();
+
+#else
+
+  return Options::Service::stand_alone ?
+         Manager::main() :
+         IMService::main();
+
+#endif
 }
 
-/******************* Auxilary functions implementation **********************/
+/**************************************************************************
+ OS-specific functions implementation.
+**************************************************************************/
 
 #if !defined(__WIN__) && !defined(OS2) && !defined(__NETWARE__)
-/* Change to run as another user if started with --user */
 
-static struct passwd *check_user(const char *user)
+/************************************************************************/
+/**
+  Change to run as another user if started with --user.
+*************************************************************************/
+
+static struct passwd *check_user()
 {
+  const char *user= Options::Daemon::user;
   struct passwd *user_info;
   uid_t user_id= geteuid();
 
@@ -195,200 +193,36 @@ err:
   return NULL;
 }
 
-static int set_user(const char *user, struct passwd *user_info)
+
+/************************************************************************/
+/**
+  Switch user.
+*************************************************************************/
+
+static bool switch_user()
 {
-  DBUG_ASSERT(user_info);
+  struct passwd *user_info= check_user();
+
+  if (!user_info)
+    return FALSE;
+
 #ifdef HAVE_INITGROUPS
-  initgroups((char*) user,user_info->pw_gid);
+  initgroups(Options::Daemon::user, user_info->pw_gid);
 #endif
+
   if (setgid(user_info->pw_gid) == -1)
   {
     log_error("setgid() failed");
-    return 1;
+    return TRUE;
   }
+
   if (setuid(user_info->pw_uid) == -1)
   {
     log_error("setuid() failed");
-    return 1;
+    return TRUE;
   }
-  return 0;
-}
-#endif
-
 
-/*
-  Init environment, common for daemon and non-daemon
-*/
-
-static void init_environment(char *progname)
-{
-  MY_INIT(progname);
-  log_init();
-  umask(0117);
-  srand((unsigned int) time(0));
+  return FALSE;
 }
 
-
-#ifndef __WIN__
-/*
-  Become a UNIX service
-  SYNOPSIS
-    daemonize()
-*/
-
-static void daemonize(const char *log_file_name)
-{
-  pid_t pid= fork();
-  switch (pid) {
-  case -1:                                      // parent, fork error
-    die("daemonize(): fork failed, %s", strerror(errno));
-  case 0:                                       // child, fork ok
-    int fd;
-    /*
-      Become a session leader: setsid must succeed because child is
-      guaranteed not to be a process group leader (it belongs to the
-      process group of the parent.)
-      The goal is not to have a controlling terminal.
-    */
-    setsid();
-    /*
-      As we now don't have a controlling terminal we will not receive
-      tty-related signals - no need to ignore them.
-    */
-
-    close(STDIN_FILENO);
-
-    fd= open(log_file_name, O_WRONLY | O_CREAT | O_APPEND | O_NOCTTY,
-                 S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
-    if (fd < 0)
-      die("daemonize(): failed to open log file %s, %s", log_file_name,
-          strerror(errno));
-    dup2(fd, STDOUT_FILENO);
-    dup2(fd, STDERR_FILENO);
-    if (fd != STDOUT_FILENO && fd != STDERR_FILENO)
-      close(fd);
-
-    /* TODO: chroot() and/or chdir() here */
-    break;
-  default:
-    /* successfully exit from parent */
-    exit(0);
-  }
-}
-
-
-enum { CHILD_OK= 0, CHILD_NEED_RESPAWN, CHILD_EXIT_ANGEL };
-
-static volatile sig_atomic_t child_status= CHILD_OK;
-
-/*
-  Signal handler for SIGCHLD: reap child, analyze child exit status, and set
-  child_status appropriately.
-*/
-
-void reap_child(int __attribute__((unused)) signo)
-{
-  int child_exit_status;
-  /* As we have only one child, no need to cycle waitpid */
-  if (waitpid(0, &child_exit_status, WNOHANG) > 0)
-  {
-    if (WIFSIGNALED(child_exit_status))
-      child_status= CHILD_NEED_RESPAWN;
-    else
-      /*
-        As reap_child is not called for SIGSTOP, we should be here only
-        if the child exited normally.
-      */
-      child_status= CHILD_EXIT_ANGEL;
-  }
-}
-
-static volatile sig_atomic_t is_terminated= 0;
-
-/*
-  Signal handler for terminate signals - SIGTERM, SIGHUP, SIGINT.
-  Set termination status and return.
-  (q) do we need to handle SIGQUIT?
-*/
-
-void terminate(int signo)
-{
-  is_terminated= signo;
-}
-
-
-/*
-  Fork a child and monitor it.
-  User can explicitly kill the angel process with SIGTERM/SIGHUP/SIGINT.
-  Angel process will exit silently if mysqlmanager exits normally.
-*/
-
-static void angel()
-{
-  /* install signal handlers */
-  sigset_t zeromask;                            // to sigsuspend in parent
-  struct sigaction sa_chld, sa_term;
-  struct sigaction sa_chld_out, sa_term_out, sa_int_out, sa_hup_out;
-
-  sigemptyset(&zeromask);
-  sigemptyset(&sa_chld.sa_mask);
-  sigemptyset(&sa_term.sa_mask);
-
-  sa_chld.sa_handler= reap_child;
-  sa_chld.sa_flags= SA_NOCLDSTOP;
-  sa_term.sa_handler= terminate;
-  sa_term.sa_flags= 0;
-
-  /* sigaction can fail only on wrong arguments */
-  sigaction(SIGCHLD, &sa_chld, &sa_chld_out);
-  sigaction(SIGTERM, &sa_term, &sa_term_out);
-  sigaction(SIGINT, &sa_term, &sa_int_out);
-  sigaction(SIGHUP, &sa_term, &sa_hup_out);
-
-  /* spawn a child */
-spawn:
-  pid_t pid= fork();
-  switch (pid) {
-  case -1:
-    die("angel(): fork failed, %s", strerror(errno));
-  case 0:                                     // child, success
-    /*
-      restore default actions for signals to let the manager work with
-      signals as he wishes
-    */
-    sigaction(SIGCHLD, &sa_chld_out, 0);
-    sigaction(SIGTERM, &sa_term_out, 0);
-    sigaction(SIGINT, &sa_int_out, 0);
-    sigaction(SIGHUP, &sa_hup_out, 0);
-    /* Here we return to main, and fall into manager */
-    break;
-  default:                                    // parent, success
-    pid= getpid(); /* Get our pid. */
-
-    log_info("Angel pid file: '%s'; PID: %d.",
-             (const char *) Options::Daemon::angel_pid_file_name,
-             (int) pid);
-
-    create_pid_file(Options::Daemon::angel_pid_file_name, pid);
-
-    while (child_status == CHILD_OK && is_terminated == 0)
-      sigsuspend(&zeromask);
-
-    if (is_terminated)
-      log_info("angel got signal %d, exiting", is_terminated);
-    else if (child_status == CHILD_NEED_RESPAWN)
-    {
-      child_status= CHILD_OK;
-      log_error("angel(): mysqlmanager exited abnormally: respawning...");
-      sleep(1); /* don't respawn too fast */
-      goto spawn;
-    }
-    /*
-      mysqlmanager successfully exited, let's silently evaporate
-      If we return to main we will fall into the manager functionality,
-      so let's simply exit().
-    */
-    exit(0);
-  }
-}
 #endif

server-tools/instance-manager/priv.cc

@@ -47,7 +47,7 @@ unsigned long open_files_limit;
 
 
 
-int create_pid_file(const char *pid_file_name, int pid)
+bool create_pid_file(const char *pid_file_name, int pid)
 {
   FILE *pid_file;
 
@@ -58,7 +58,7 @@ int create_pid_file(const char *pid_file_name, int pid)
               (const char *) pid_file_name,
               (const char *) strerror(errno),
               (int) errno);
-    return 1;
+    return TRUE;
   }
 
   if (fprintf(pid_file, "%d\n", (int) pid) <= 0)
@@ -67,10 +67,10 @@ int create_pid_file(const char *pid_file_name, int pid)
               (const char *) pid_file_name,
               (const char *) strerror(errno),
               (int) errno);
-    return 1;
+    return TRUE;
   }
 
   my_fclose(pid_file, MYF(0));
 
-  return 0;
+  return FALSE;
 }

server-tools/instance-manager/priv.h

@@ -94,6 +94,6 @@ extern unsigned long bytes_sent, bytes_received;
 extern unsigned long mysqld_net_retry_count;
 extern unsigned long open_files_limit;
 
-int create_pid_file(const char *pid_file_name, int pid);
+bool create_pid_file(const char *pid_file_name, int pid);
 
 #endif // INCLUDES_MYSQL_INSTANCE_MANAGER_PRIV_H