From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
cc: Kuntal Ghosh <kuntalghosh.2007@gmail.com>, Thunder <thunder1@126.com>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: PANIC :Call AbortTransaction when transaction id is no normal
In-reply-to: <15939.1557754652@sss.pgh.pa.us>
References: <3850b11a.5121.16aaf827e4a.Coremail.thunder1@126.com>
 <CAGz5QCJKDptF4R0xHzM9YLxbLwZHA9enW9zjOi+vF93HVycVgw@mail.gmail.com>
 <20190513081018.GC2273@paquier.xyz> <15939.1557754652@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Mon, 13 May 2019 09:37:32 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <5732.1557771243.0@sss.pgh.pa.us>
Date: Mon, 13 May 2019 14:15:10 -0400
Message-ID: <5787.1557771310@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5732.1557771243.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable

I wrote:
> If we do anything at all about this, my thought would just be to change
> bootstrap_signals() so that it points all the signal handlers at
> quickdie(), or maybe something equivalent to quickdie() but printing
> a more apropos message, or even just set them all to SIGDFL since that
> means process termination for all of these.  die() isn't really the righ=
t
> thing, precisely because it thinks it can trigger transaction abort,
> which makes no sense in bootstrap mode.

After further thought I like the SIG_DFL answer, as per attached proposed
patch.  With this, you get SIGINT behavior like this if you manage to
catch it in bootstrap mode (which is not that easy these days):

selecting default max_connections ... 100
selecting default shared_buffers ... 128MB
selecting default timezone ... America/New_York
creating configuration files ... ok
running bootstrap script ... ^Cchild process was terminated by signal 2: I=
nterrupt
initdb: removing data directory "/home/postgres/testversion/data"

That seems perfectly fine from here.

> But ... that code's been like that for decades and nobody's complained
> before.  Why are we worried about bootstrap's response to signals at all=
?

I'm still wondering why the OP cares.  Still, the PANIC message that you
get right now is potentially confusing.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-bootstrap-signal-handling.patch";
	charset="us-ascii"
Content-ID: <5732.1557771243.2@sss.pgh.pa.us>
Content-Description: fix-bootstrap-signal-handling.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/boo=
tstrap.c
index d8776e1..825433d 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -558,11 +558,16 @@ bootstrap_signals(void)
 {
 	Assert(!IsUnderPostmaster);
 =

-	/* Set up appropriately for interactive use */
-	pqsignal(SIGHUP, die);
-	pqsignal(SIGINT, die);
-	pqsignal(SIGTERM, die);
-	pqsignal(SIGQUIT, die);
+	/*
+	 * We don't actually need any non-default signal handling in bootstrap
+	 * mode; "curl up and die" is a sufficient response for all these cases.
+	 * But let's just make sure the signals are set that way, since our pare=
nt
+	 * process initdb has them set differently.
+	 */
+	pqsignal(SIGHUP, SIG_DFL);
+	pqsignal(SIGINT, SIG_DFL);
+	pqsignal(SIGTERM, SIG_DFL);
+	pqsignal(SIGQUIT, SIG_DFL);
 }
 =

 /*

------- =_aaaaaaaaaa0--