Merge lp:~akopytov/percona-xtradb-cluster/bug1382797-5.5 into lp:percona-xtradb-cluster/5.5

[Alexey Kopytov]

Bug #1382797: Modify SST code to use fork()/exec() to allow cleanup on
              fatal signals
Bug #1399175: Incorrect POSIX_SPAWN_SETSIGDEF usage in wsrep_utils.cc

Modified wsp::process() to use fork()/exec() instead of posix_spawn() to
have more control on the child process. More specifically:

- the child process now calls prctl() with appropriate arguments (if
  available) before calling exec() so that the SIGTERM signal is
  received by it in case the parent process (i.e. mysqld) is terminated
  either gracefully or ungracefully.

- the child process also call setsid() to create a new session and a
  process group. This is to simplify killing all spawned processes at
  once for child processes. Instead of keeping track of all processes
  started by an SST script and sending termination signals to the
  individually, an SST script can simply kill the entire process group.

Additionally, an optimization to speed up fork() has been implemented to
reduce page table copying overhead as compared to vfork() /
posix_spawn(). Since the InnoDB buffer pool is the major part of mysqld
RSS in most installations, use madvise(..., MADV_DONTFORK) to exclude
buffer pool from memory available to the child on fork().

This revision also fixes bug #1399175 as a side effect, since all code
related to the posix_spawn*() family of functions is replaced.

[Alexey Kopytov (akopytov)]

Forgot to add signal signal-related code. Will repush.

review: Needs Fixing

[Alexey Kopytov (akopytov)]

Passes Jenkins now.

=== modified file 'sql/wsrep_utils.cc'
--- sql/wsrep_utils.cc	2013-11-19 07:24:09 +0000
+++ sql/wsrep_utils.cc	2014-12-08 16:17:50 +0000
@@ -25,7 +25,6 @@
 
 #include <sql_class.h>
 
-#include <spawn.h>    // posix_spawn()
 #include <unistd.h>   // pipe()
 #include <errno.h>    // errno
 #include <string.h>   // strerror()
@@ -34,6 +33,10 @@
 #include <sys/socket.h>
 #include <netdb.h>    // getaddrinfo()
 
+#ifdef HAVE_SYS_PRCTL_H
+#include <sys/prctl.h>
+#endif
+
 extern char** environ; // environment variables
 
 static wsp::string wsrep_PATH;
@@ -89,13 +92,12 @@
 #define STDIN_FD   0
 #define STDOUT_FD  1
 
-#ifndef POSIX_SPAWN_USEVFORK
-# define POSIX_SPAWN_USEVFORK 0
-#endif
-
 process::process (const char* cmd, const char* type)
-    : str_(cmd ? strdup(cmd) : strdup("")), io_(NULL), err_(EINVAL), pid_(0)
+    : str_(cmd ? strdup(cmd) : strdup("")), io_(NULL), err_(0), pid_(0)
 {
+    int sig;
+    struct sigaction sa;
+
     if (0 == str_)
     {
         WSREP_ERROR ("Can't allocate command line of size: %zu", strlen(cmd));
@@ -128,105 +130,99 @@
     int const child_end  (parent_end == PIPE_READ ? PIPE_WRITE : PIPE_READ);
     int const close_fd   (parent_end == PIPE_READ ? STDOUT_FD : STDIN_FD);
 
-    char* const pargv[4] = { strdup("sh"), strdup("-c"), strdup(str_), NULL };
-    if (!(pargv[0] && pargv[1] && pargv[2]))
-    {
-        err_ = ENOMEM;
-        WSREP_ERROR ("Failed to allocate pargv[] array.");
-        goto cleanup_pipe;
-    }
-
-    posix_spawnattr_t attr;
-    err_ = posix_spawnattr_init (&attr);
-    if (err_)
-    {
-        WSREP_ERROR ("posix_spawnattr_init() failed: %d (%s)",
-                     err_, strerror(err_));
-        goto cleanup_pipe;
-    }
-
-    err_ = posix_spawnattr_setflags (&attr, POSIX_SPAWN_SETSIGDEF |
-                                            POSIX_SPAWN_USEVFORK);
-    if (err_)
-    {
-        WSREP_ERROR ("posix_spawnattr_setflags() failed: %d (%s)",
-                     err_, strerror(err_));
-        goto cleanup_attr;
-    }
-
-    posix_spawn_file_actions_t fact;
-    err_ = posix_spawn_file_actions_init (&fact);
-    if (err_)
-    {
-        WSREP_ERROR ("posix_spawn_file_actions_init() failed: %d (%s)",
-                     err_, strerror(err_));
-        goto cleanup_attr;
-    }
-
-    // close child's stdout|stdin depending on what we returning
-    err_ = posix_spawn_file_actions_addclose (&fact, close_fd);
-    if (err_)
-    {
-        WSREP_ERROR ("posix_spawn_file_actions_addclose() failed: %d (%s)",
-                     err_, strerror(err_));
-        goto cleanup_fact;
-    }
-
-    // substitute our pipe descriptor in place of the closed one
-    err_ = posix_spawn_file_actions_adddup2 (&fact,
-                                             pipe_fds[child_end], close_fd);
-    if (err_)
-    {
-        WSREP_ERROR ("posix_spawn_file_actions_addup2() failed: %d (%s)",
-                     err_, strerror(err_));
-        goto cleanup_fact;
-    }
-
-    err_ = posix_spawnp (&pid_, pargv[0], &fact, &attr, pargv, environ);
-    if (err_)
-    {
-        WSREP_ERROR ("posix_spawnp(%s) failed: %d (%s)",
-                     pargv[2], err_, strerror(err_));
-        pid_ = 0; // just to make sure it was not messed up in the call
-        goto cleanup_fact;
-    }
-
-    io_ = fdopen (pipe_fds[parent_end], type);
-
-    if (io_)
-    {
+    pid_ = fork();
+
+    if (pid_ == -1)
+    {
+      err_= errno;
+      WSREP_ERROR ("fork() failed: %d (%s)", err_, strerror(err_));
+      pid_ = 0;
+      goto cleanup;
+    }
+    else if (pid_ > 0)
+    {
+      /* Parent */
+
+      io_ = fdopen (pipe_fds[parent_end], type);
+
+      if (io_)
+      {
         pipe_fds[parent_end] = -1; // skip close on cleanup
-    }
-    else
-    {
+      }
+      else
+      {
         err_ = errno;
         WSREP_ERROR ("fdopen() failed: %d (%s)", err_, strerror(err_));
-    }
-
-cleanup_fact:
-    int err; // to preserve err_ code
-    err = posix_spawn_file_actions_destroy (&fact);
-    if (err)
-    {
-        WSREP_ERROR ("posix_spawn_file_actions_destroy() failed: %d (%s)\n",
-                     err, strerror(err));
-    }
-
-cleanup_attr:
-    err = posix_spawnattr_destroy (&attr);
-    if (err)
-    {
-        WSREP_ERROR ("posix_spawnattr_destroy() failed: %d (%s)",
-                     err, strerror(err));
-    }
-
-cleanup_pipe:
+      }
+
+      goto cleanup;
+    }
+
+    /* Child */
+
+#ifdef PR_SET_PDEATHSIG
+    /*
+      Ensure this process gets SIGTERM when the server is terminated for
+      whatever reasons.
+    */
+
+    if (prctl(PR_SET_PDEATHSIG, SIGTERM))
+    {
+      sql_perror("prctl() failed");
+      _exit(EXIT_FAILURE);
+    }
+#endif
+
+    /* Reset all ignored signals to SIG_DFL */
+
+    memset(&sa, 0, sizeof(sa));
+    sa.sa_handler = SIG_DFL;
+
+    for (sig = 1; sig < NSIG; sig++)
+    {
+      /*
+        For some signals sigaction() even when SIG_DFL handler is set, so don't
+        check for return value here.
+      */
+      sigaction(sig, &sa, NULL);
+    }
+
+    /*
+      Make the child process a session/group leader. This is required to
+      simplify cleanup procedure for SST process, so that all processes spawned
+      by the SST script could be killed by killing the entire process group.
+    */
+
+    if (setsid() < 0)
+    {
+      sql_perror("setsid() failed");
+      _exit(EXIT_FAILURE);
+    }
+
+    /* close child's stdout|stdin depending on what we returning */
+
+    if (close(close_fd) < 0)
+    {
+      sql_perror("close() failed");
+      _exit(EXIT_FAILURE);
+    }
+
+    /* substitute our pipe descriptor in place of the closed one */
+
+    if (dup2(pipe_fds[child_end], close_fd) < 0)
+    {
+      sql_perror("dup2() failed");
+      _exit(EXIT_FAILURE);
+    }
+
+    execlp("sh", "sh", "-c", str_, NULL);
+
+    sql_perror("execlp() failed");
+    _exit(EXIT_FAILURE);
+
+cleanup:
     if (pipe_fds[0] >= 0) close (pipe_fds[0]);
     if (pipe_fds[1] >= 0) close (pipe_fds[1]);
-
-    free (pargv[0]);
-    free (pargv[1]);
-    free (pargv[2]);
 }
 
 process::~process ()
 
=== modified file 'storage/innobase/os/os0proc.c'
--- storage/innobase/os/os0proc.c	2013-08-13 11:26:01 +0000
+++ storage/innobase/os/os0proc.c	2014-12-08 16:17:50 +0000
@@ -38,6 +38,10 @@
 #include <sys/utsname.h>	/* uname() */
 #endif
 
+#if defined(WITH_WSREP) && defined(UNIV_LINUX)
+#include <sys/mman.h>		/* madvise() */
+#endif
+
 /* FreeBSD for example has only MAP_ANON, Linux has MAP_ANONYMOUS and
 MAP_ANON but MAP_ANON is marked as deprecated */
 #if defined(MAP_ANONYMOUS)
@@ -229,6 +233,16 @@
 		memset(ptr, '\0', size);
 	}
 
+#if defined(WITH_WSREP) && defined(UNIV_LINUX)
+	/* Do not make the pages from this block available to the child after a
+	fork(). This is required to speed up process spawning for Galera SST. */
+
+	if (madvise(ptr, size, MADV_DONTFORK)) {
+		fprintf(stderr, "InnoDB: Warning: madvise(MADV_DONTFORK) is "
+			"not supported by the kernel. Spawning SST processes "
+			"can be slow.\n");
+	}
+#endif
 	return(ptr);
 }