Re: SIGQUIT on archiver child processes maybe not such a hot idea?

At Wed, 11 Sep 2019 11:01:24 +0900 (Tokyo Standard Time), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
<20190911.110124.96874741.horikyota.ntt@gmail.com>
> At Wed, 11 Sep 2019 01:36:15 +0000, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> wrote in
<0A3221C70F24FB45833433255569204D1FD33579@G01JPEXMBYT05>
> > From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> > > SIGTERM, which needs to be adjusted.  For another, its
> > > SIGQUIT handler does exit(1) not _exit(2), which seems rather
> > > dubious ... should we make it more like the rest?  I think
> > > the reasoning there might've been that if some DBA decides to
> > > SIGQUIT the archiver, we don't need to force a database-wide
> > > reset; but why exactly should we tolerate that?
> > 
> > postmaster doesn't distinguish return codes other than 0 for the archiver, and just starts the archiver unless
postmasteris shutting down.  So we can use _exit(2) like the other children.
 
> > 
> > Can't we use SIGKILL instead of SIGINT/SIGTERM to stop the grandchildren, just in case they are slow to respond to
orignore SIGINT/SIGTERM?  That matches the idea of pg_ctl's immediate shutdown.
 
> 
> Perhaps +1..  immediate -> SIGKILL  fast -> SIGTERM?

We send SIGUSR2 to archive while fast shutdown. It would be
enough for pg_system to do signal(SNGINT, DIG_DFL) andsignal
signal(SIGQUIT, SIG_IGN) then remember the child's pid somewhere.
Then each process send SIGKILL to the remembered process in
sigquit handler.  (I'm not sure what happens if kill(0,
SIGKILL)).

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index f84f882c4c..1fe7b5dd0a 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -108,6 +108,50 @@ static bool pgarch_archiveXlog(char *xlog);
 static bool pgarch_readyXlog(char *xlog);
 static void pgarch_archiveDone(char *xlog);
 
+pid_t system_pid = -1;
+
+static void
+pg_system_kill()
+{
+    if (system_pid >= 0)
+        kill(system_pid, SIGKILL);
+}
+
+static int
+pg_system(char *cmd)
+{
+  int status;
+  long result;
+
+
+  system_pid = fork();
+  if (system_pid == 0)
+  {
+      char *new_argv[4] = {"sh", "-c", cmd, NULL};
+
+      pqsignal(SIGINT, SIG_DFL);
+      pqsignal(SIGQUIT, SIG_IGN);
+      (void) execve ("/bin/sh", (char * const *) new_argv, __environ);
+      _exit(127);
+  }
+
+  if (system_pid < 0)
+      return -1;
+
+  do
+  {
+      result = waitpid(system_pid, &status, 0);
+  } while (result == -1L && errno == EINTR);
+
+  if (result != system_pid)
+  {
+      system_pid = -1;
+      return -1;
+  }
+
+  system_pid = -1;
+  return status;
+}
 
 /* ------------------------------------------------------------
  * Public functions called from postmaster follow
@@ -255,6 +299,9 @@ PgArchiverMain(int argc, char *argv[])
 static void
 pgarch_exit(SIGNAL_ARGS)
 {
+    if (postgres_signal_arg == SIGQUIT)
+        pg_system_kill();
+
     /* SIGQUIT means curl up and die ... */
     exit(1);
 }
@@ -620,7 +667,7 @@ pgarch_archiveXlog(char *xlog)
     snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
     set_ps_display(activitymsg, false);
 
-    rc = system(xlogarchcmd);
+    rc = pg_system(xlogarchcmd);
     if (rc != 0)
     {
         /*
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a5446d54bb..d9b85a5228 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -3964,6 +3964,8 @@ PostmasterStateMachine(void)
 static void
 signal_child(pid_t pid, int signal)
 {
+    if (pid == PgArchPID)
+        elog(LOG, "Send %d to archiver", signal);
     if (kill(pid, signal) < 0)
         elog(DEBUG3, "kill(%ld,%d) failed: %m", (long) pid, signal);
 #ifdef HAVE_SETSID