pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Michael Paquier <michael@paquier.xyz>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: Fujii Masao <fujii@postgresql.org>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Weird failure with latches in curculio on v15
Date: Sat, 4 Feb 2023 11:47:16 +0900
Message-ID: <Y93HNBilEWbfGR1d@paquier.xyz> (raw)
In-Reply-To: <20230203185417.GA243728@nathanxps13>
References: <20230201165801.33ydbxvjdbomjqa7@alap3.anarazel.de>
	<CA+TgmoYbHybheHPxrhG82RT8=eaT9AF_feM4ijAM=gPEoFm-tw@mail.gmail.com>
	<20230201172016.7c3tg2omorvyxw6h@alap3.anarazel.de>
	<1384520.1675272439@sss.pgh.pa.us>
	<20230201181827.5pcnqdnt3nlcjgex@alap3.anarazel.de>
	<Y9sQiStcAGmnd5MH@paquier.xyz>
	<20230203071557.ofq56vmuxx5jgbey@alap3.anarazel.de>
	<1645348.1675409043@sss.pgh.pa.us>
	<20230203074422.wx2ysoraqh46zqg2@alap3.anarazel.de>
	<20230203185417.GA243728@nathanxps13>

On Fri, Feb 03, 2023 at 10:54:17AM -0800, Nathan Bossart wrote:
> 0001 is just v1-0001 from upthread.  This moves Pre/PostRestoreCommand to
> surround only the call to system().  I think this should get us closer to
> pre-v15 behavior.

+   if (exitOnSigterm)
+       PreRestoreCommand();
+
    rc = system(command);
+
+   if (exitOnSigterm)
+       PostRestoreCommand();

I don't really want to let that hanging around on HEAD much longer, so
I'm OK to do that for HEAD, then figure out what needs to be done for
the older issue at hand.

+   /*
+    * PreRestoreCommand() is used to tell the SIGTERM handler for the startup
+    * process that it is okay to proc_exit() right away on SIGTERM.  This is
+    * done for the duration of the system() call because there isn't a good
+    * way to break out while it is executing.  Since we might call proc_exit()
+    * in a signal handler here, it is extremely important that nothing but the
+    * system() call happens between the calls to PreRestoreCommand() and
+    * PostRestoreCommand().  Any additional code must go before or after this
+    * section.
+    */

Still, it seems to me that the large comment block in shell_restore()
ought to be moved to ExecuteRecoveryCommand(), no?  The assumptions
under which one can use exitOnSigterm and failOnSignal could be
completed in the header of the function based on that.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

view thread (78+ messages)  latest in thread

Message-ID: <Y93HNBilEWbfGR1d@paquier.xyz>
Permalink:  ../Y93HNBilEWbfGR1d@paquier.xyz/
Also on:    postgresql.org/message-id/Y93HNBilEWbfGR1d@paquier.xyz

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: michael@paquier.xyz, nathandbossart@gmail.com, andres@anarazel.de, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, thomas.munro@gmail.com, fujii@postgresql.org, pgsql-hackers@lists.postgresql.org
  Subject: Re: Weird failure with latches in curculio on v15
  In-Reply-To: <Y93HNBilEWbfGR1d@paquier.xyz>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox