Date: Fri, 3 Sep 2021 17:16:06 -0400
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: =?utf-8?B?6JSh5qKm5aifKOeOiuS6jik=?= <mengjuan.cmj@alibaba-inc.com>,
	pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Add guc to enable send SIGSTOP to peers when backend exits
 abnormally
Message-ID: <202109032116.mipfjjjgimw5@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="wetnbohpod6fdlh6"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <2498875.1630679973@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/202109032116.mipfjjjgimw5%40alvherre.pgsql>
Precedence: bulk


--wetnbohpod6fdlh6
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2021-Sep-03, Tom Lane wrote:

> "=?UTF-8?B?6JSh5qKm5aifKOeOiuS6jik=?=" <mengjuan.cmj@alibaba-inc.com> writes:
> > I want to share a patch with you, in which I add a guc parameter 'enable_send_stop' to enable set the value of SendStop in postmaster.c more conveniently.  SendStop enable postmaster to send SIGSTOP rather than SIGQUIT to its children when some backend dumps core, and this variable is originally set with -T parameter when start postgres, which is inconvenient to control in some scenarios.
> 
> TBH, I'd sooner rip out SendStop, and simplify the related postmaster
> logic.

I wrote a patch to do that in 2012, after this exchange:
https://postgr.es/m/1333124720-sup-6193@alvh.no-ip.org
I obviously doesn't apply at all anymore, but the thing that prevented
me from sending it was I couldn't find what the mentioned feature was
that would cause all backends to dump core at the time of a crash.
So it seemed to me that we would be ripping out a feature I had used,
with no replacement.

(It applies cleanly on top of 36b7e3da17bc.)

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/
"Cuando no hay humildad las personas se degradan" (A. Christie)

--wetnbohpod6fdlh6
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-Remove-T-postmaster-option.patch"

From d8198f6ad6c814c03c486bcce696fda912393405 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 23 May 2012 22:18:21 -0400
Subject: [PATCH] Remove -T postmaster option

This used to tell postmaster to send SIGSTOP instead of SIGQUIT in case
of trouble, with the intent of letting a hypothetical PG hacker collect
core dumps from every backends by attaching a debugger to them, one by
one.  This has long been superseded by the ability of current operating
systems to dump core of several processes simultaneously.
---
 src/backend/postmaster/postmaster.c | 52 +++++++++--------------------
 1 file changed, 16 insertions(+), 36 deletions(-)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index eeea933b19..bf6166caf4 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -187,7 +187,6 @@ static char ExtraOptions[MAXPGPATH];
  * shared data structures.	(Reinit is currently dead code, though.)
  */
 static bool Reinit = true;
-static int	SendStop = false;
 
 /* still more option variables */
 bool		EnableSSL = false;
@@ -544,7 +543,7 @@ PostmasterMain(int argc, char *argv[])
 	 * tcop/postgres.c (the option sets should not conflict) and with the
 	 * common help() function in main/main.c.
 	 */
-	while ((opt = getopt(argc, argv, "A:B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
+	while ((opt = getopt(argc, argv, "A:B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:st:W:-:")) != -1)
 	{
 		switch (opt)
 		{
@@ -654,16 +653,6 @@ PostmasterMain(int argc, char *argv[])
 				SetConfigOption("log_statement_stats", "true", PGC_POSTMASTER, PGC_S_ARGV);
 				break;
 
-			case 'T':
-
-				/*
-				 * In the event that some backend dumps core, send SIGSTOP,
-				 * rather than SIGQUIT, to all its peers.  This lets the wily
-				 * post_hacker collect core dumps from everyone.
-				 */
-				SendStop = true;
-				break;
-
 			case 't':
 				{
 					const char *tmp = get_stats_option_name(optarg);
@@ -2704,9 +2693,7 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
 			 * to commit hara-kiri.
 			 *
 			 * SIGQUIT is the special signal that says exit without proc_exit
-			 * and let the user know what's going on. But if SendStop is set
-			 * (-s on command line), then we send SIGSTOP instead, so that we
-			 * can get core dumps from all backends by hand.
+			 * and let the user know what's going on.
 			 *
 			 * We could exclude dead_end children here, but at least in the
 			 * SIGSTOP case it seems better to include them.
@@ -2715,9 +2702,8 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
 			{
 				ereport(DEBUG2,
 						(errmsg_internal("sending %s to process %d",
-										 (SendStop ? "SIGSTOP" : "SIGQUIT"),
-										 (int) bp->pid)));
-				signal_child(bp->pid, (SendStop ? SIGSTOP : SIGQUIT));
+										 "SIGQUIT", (int) bp->pid)));
+				signal_child(bp->pid, SIGQUIT);
 			}
 		}
 	}
@@ -2729,9 +2715,8 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
 	{
 		ereport(DEBUG2,
 				(errmsg_internal("sending %s to process %d",
-								 (SendStop ? "SIGSTOP" : "SIGQUIT"),
-								 (int) StartupPID)));
-		signal_child(StartupPID, (SendStop ? SIGSTOP : SIGQUIT));
+								 "SIGQUIT", (int) StartupPID)));
+		signal_child(StartupPID, SIGQUIT);
 	}
 
 	/* Take care of the bgwriter too */
@@ -2741,9 +2726,8 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
 	{
 		ereport(DEBUG2,
 				(errmsg_internal("sending %s to process %d",
-								 (SendStop ? "SIGSTOP" : "SIGQUIT"),
-								 (int) BgWriterPID)));
-		signal_child(BgWriterPID, (SendStop ? SIGSTOP : SIGQUIT));
+								 "SIGQUIT", (int) BgWriterPID)));
+		signal_child(BgWriterPID, SIGQUIT);
 	}
 
 	/* Take care of the checkpointer too */
@@ -2753,9 +2737,8 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
 	{
 		ereport(DEBUG2,
 				(errmsg_internal("sending %s to process %d",
-								 (SendStop ? "SIGSTOP" : "SIGQUIT"),
-								 (int) CheckpointerPID)));
-		signal_child(CheckpointerPID, (SendStop ? SIGSTOP : SIGQUIT));
+								 "SIGQUIT", (int) CheckpointerPID)));
+		signal_child(CheckpointerPID, SIGQUIT);
 	}
 
 	/* Take care of the walwriter too */
@@ -2765,9 +2748,8 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
 	{
 		ereport(DEBUG2,
 				(errmsg_internal("sending %s to process %d",
-								 (SendStop ? "SIGSTOP" : "SIGQUIT"),
-								 (int) WalWriterPID)));
-		signal_child(WalWriterPID, (SendStop ? SIGSTOP : SIGQUIT));
+								 "SIGQUIT", (int) WalWriterPID)));
+		signal_child(WalWriterPID, SIGQUIT);
 	}
 
 	/* Take care of the walreceiver too */
@@ -2777,9 +2759,8 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
 	{
 		ereport(DEBUG2,
 				(errmsg_internal("sending %s to process %d",
-								 (SendStop ? "SIGSTOP" : "SIGQUIT"),
-								 (int) WalReceiverPID)));
-		signal_child(WalReceiverPID, (SendStop ? SIGSTOP : SIGQUIT));
+								 "SIGQUIT", (int) WalReceiverPID)));
+		signal_child(WalReceiverPID, SIGQUIT);
 	}
 
 	/* Take care of the autovacuum launcher too */
@@ -2789,9 +2770,8 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
 	{
 		ereport(DEBUG2,
 				(errmsg_internal("sending %s to process %d",
-								 (SendStop ? "SIGSTOP" : "SIGQUIT"),
-								 (int) AutoVacPID)));
-		signal_child(AutoVacPID, (SendStop ? SIGSTOP : SIGQUIT));
+								 "SIGQUIT", (int) AutoVacPID)));
+		signal_child(AutoVacPID, SIGQUIT);
 	}
 
 	/*
-- 
2.30.2


--wetnbohpod6fdlh6--