From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15804: Assertion failure when using logging_collector with EXEC_BACKEND
Date: Tue, 10 Sep 2019 10:58:52 -0400
Message-ID: <20705.1568127532@sss.pgh.pa.us> (raw)
In-Reply-To: <20190910053923.GC11737@paquier.xyz>
References: <22995.1558278322@sss.pgh.pa.us>
<27719.1558282181@sss.pgh.pa.us>
<16df9d01-8b03-2664-a18c-16c40186f5ab@2ndQuadrant.com>
<2702.1558289980@sss.pgh.pa.us>
<20190520012728.GC1923@paquier.xyz>
<1664.1558325417@sss.pgh.pa.us>
<7927.1558364465@sss.pgh.pa.us>
<31457.1568057723@sss.pgh.pa.us>
<20190910051115.GB22934@paquier.xyz>
<6728.1568093331@sss.pgh.pa.us>
<20190910053923.GC11737@paquier.xyz>
Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Sep 10, 2019 at 01:28:51AM -0400, Tom Lane wrote:
>> But the external PID file is a different story. I can believe that
>> something might be expecting the sockets to be open before we create
>> that file --- and since we're not using that file for an interlock,
>> there's no reason to be in a hurry to create it. I think it'd make
>> sense to move that step further down, so it's still done after the
>> create-sockets step.
> That's the actual take here. We cannot assume that nobody is relying
> on that. If they do, it could be tricky to rework this logic. Moving
> the external file write a bit later does not sound like a bad thing in
> itself to keep more consistency with the past.
Here's a version that does it like that. After studying things a bit,
it seemed like a good idea to also move RemovePgTempFiles() down,
so that that possibly-slow step still occurs after making the external
PID file. Everything else that we've moved to before that step
should be reasonably quick.
regards, tom lane
Attachments:
[text/x-diff] postpone-port-opening-3.patch (7.9K, ../20705.1568127532@sss.pgh.pa.us/2-postpone-port-opening-3.patch)
download | inline diff:diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.cindex a5446d5..bf4376c 100644--- a/src/backend/postmaster/postmaster.c+++ b/src/backend/postmaster/postmaster.c@@ -993,7 +993,103 @@ PostmasterMain(int argc, char *argv[])
*/
InitializeMaxBackends();
- /* Report server startup in log */+ /*+ * Set up shared memory and semaphores.+ */+ reset_shared();++ /*+ * Estimate number of openable files. This must happen after setting up+ * semaphores, because on some platforms semaphores count as open files.+ */+ set_max_safe_fds();++ /*+ * Set reference point for stack-depth checking.+ */+ set_stack_base();++ /*+ * Initialize pipe (or process handle on Windows) that allows children to+ * wake up from sleep on postmaster death.+ */+ InitPostmasterDeathWatchHandle();++#ifdef WIN32++ /*+ * Initialize I/O completion port used to deliver list of dead children.+ */+ win32ChildQueue = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 1);+ if (win32ChildQueue == NULL)+ ereport(FATAL,+ (errmsg("could not create I/O completion port for child queue")));+#endif++#ifdef EXEC_BACKEND+ /* Write out nondefault GUC settings for child processes to use */+ write_nondefault_variables(PGC_POSTMASTER);+#endif++ /*+ * Forcibly remove the files signaling a standby promotion request.+ * Otherwise, the existence of those files triggers a promotion too early,+ * whether a user wants that or not.+ *+ * This removal of files is usually unnecessary because they can exist+ * only during a few moments during a standby promotion. However there is+ * a race condition: if pg_ctl promote is executed and creates the files+ * during a promotion, the files can stay around even after the server is+ * brought up to new master. Then, if new standby starts by using the+ * backup taken from that master, the files can exist at the server+ * startup and should be removed in order to avoid an unexpected+ * promotion.+ *+ * Note that promotion signal files need to be removed before the startup+ * process is invoked. Because, after that, they can be used by+ * postmaster's SIGUSR1 signal handler.+ */+ RemovePromoteSignalFiles();++ /* Do the same for logrotate signal file */+ RemoveLogrotateSignalFiles();++ /* Remove any outdated file holding the current log filenames. */+ if (unlink(LOG_METAINFO_DATAFILE) < 0 && errno != ENOENT)+ ereport(LOG,+ (errcode_for_file_access(),+ errmsg("could not remove file \"%s\": %m",+ LOG_METAINFO_DATAFILE)));++ /*+ * If enabled, start up syslogger collection subprocess+ */+ SysLoggerPID = SysLogger_Start();++ /*+ * Reset whereToSendOutput from DestDebug (its starting state) to+ * DestNone. This stops ereport from sending log messages to stderr unless+ * Log_destination permits. We don't do this until the postmaster is+ * fully launched, since startup failures may as well be reported to+ * stderr.+ *+ * If we are in fact disabling logging to stderr, first emit a log message+ * saying so, to provide a breadcrumb trail for users who may not remember+ * that their logging is configured to go somewhere else.+ */+ if (!(Log_destination & LOG_DESTINATION_STDERR))+ ereport(LOG,+ (errmsg("ending log output to stderr"),+ errhint("Future log output will go to log destination \"%s\".",+ Log_destination_string)));++ whereToSendOutput = DestNone;++ /*+ * Report server startup in log. While we could emit this much earlier,+ * it seems best to do so after starting the log collector, if we intend+ * to use one.+ */
ereport(LOG,
(errmsg("starting %s", PG_VERSION_STR)));
@@ -1173,50 +1269,12 @@ PostmasterMain(int argc, char *argv[])
AddToDataDirLockFile(LOCK_FILE_LINE_LISTEN_ADDR, "");
/*
- * Set up shared memory and semaphores.- */- reset_shared();-- /*- * Estimate number of openable files. This must happen after setting up- * semaphores, because on some platforms semaphores count as open files.- */- set_max_safe_fds();-- /*- * Set reference point for stack-depth checking.- */- set_stack_base();-- /*- * Initialize pipe (or process handle on Windows) that allows children to- * wake up from sleep on postmaster death.- */- InitPostmasterDeathWatchHandle();--#ifdef WIN32-- /*- * Initialize I/O completion port used to deliver list of dead children.- */- win32ChildQueue = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 1);- if (win32ChildQueue == NULL)- ereport(FATAL,- (errmsg("could not create I/O completion port for child queue")));-#endif-- /*
* Record postmaster options. We delay this till now to avoid recording
- * bogus options (eg, NBuffers too high for available memory).+ * bogus options (eg, unusable port number).
*/
if (!CreateOptsFile(argc, argv, my_exec_path))
ExitPostmaster(1);
-#ifdef EXEC_BACKEND- /* Write out nondefault GUC settings for child processes to use */- write_nondefault_variables(PGC_POSTMASTER);-#endif-
/*
* Write the external PID file if requested
*/
@@ -1248,60 +1306,6 @@ PostmasterMain(int argc, char *argv[])
RemovePgTempFiles();
/*
- * Forcibly remove the files signaling a standby promotion request.- * Otherwise, the existence of those files triggers a promotion too early,- * whether a user wants that or not.- *- * This removal of files is usually unnecessary because they can exist- * only during a few moments during a standby promotion. However there is- * a race condition: if pg_ctl promote is executed and creates the files- * during a promotion, the files can stay around even after the server is- * brought up to new master. Then, if new standby starts by using the- * backup taken from that master, the files can exist at the server- * startup and should be removed in order to avoid an unexpected- * promotion.- *- * Note that promotion signal files need to be removed before the startup- * process is invoked. Because, after that, they can be used by- * postmaster's SIGUSR1 signal handler.- */- RemovePromoteSignalFiles();-- /* Do the same for logrotate signal file */- RemoveLogrotateSignalFiles();-- /* Remove any outdated file holding the current log filenames. */- if (unlink(LOG_METAINFO_DATAFILE) < 0 && errno != ENOENT)- ereport(LOG,- (errcode_for_file_access(),- errmsg("could not remove file \"%s\": %m",- LOG_METAINFO_DATAFILE)));-- /*- * If enabled, start up syslogger collection subprocess- */- SysLoggerPID = SysLogger_Start();-- /*- * Reset whereToSendOutput from DestDebug (its starting state) to- * DestNone. This stops ereport from sending log messages to stderr unless- * Log_destination permits. We don't do this until the postmaster is- * fully launched, since startup failures may as well be reported to- * stderr.- *- * If we are in fact disabling logging to stderr, first emit a log message- * saying so, to provide a breadcrumb trail for users who may not remember- * that their logging is configured to go somewhere else.- */- if (!(Log_destination & LOG_DESTINATION_STDERR))- ereport(LOG,- (errmsg("ending log output to stderr"),- errhint("Future log output will go to log destination \"%s\".",- Log_destination_string)));-- whereToSendOutput = DestNone;-- /*
* Initialize stats collection subsystem (this does NOT start the
* collector process!)
*/
diff --git a/src/include/utils/pidfile.h b/src/include/utils/pidfile.hindex b2dcca6..21e7ccf 100644--- a/src/include/utils/pidfile.h+++ b/src/include/utils/pidfile.h@@ -28,7 +28,8 @@
*
* Lines 6 and up are added via AddToDataDirLockFile() after initial file
* creation; also, line 5 is initially empty and is changed after the first
- * Unix socket is opened.+ * Unix socket is opened. Onlookers should not assume that lines 4 and up+ * are filled in any particular order.
*
* Socket lock file(s), if used, have the same contents as lines 1-5, with
* line 5 being their own directory.
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-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, andrew.dunstan@2ndquadrant.com, andres@anarazel.de, peter.eisentraut@2ndquadrant.com, yuli.khodorkovskiy@crunchydata.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15804: Assertion failure when using logging_collector with EXEC_BACKEND
In-Reply-To: <20705.1568127532@sss.pgh.pa.us>
* 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