My Salesforce colleagues observed a failure mode in which a bug in the
crash recovery logic caused the startup process to get a SEGV while trying
to recover after a backend crash. The postmaster should have given up at
that point, but instead it kept on respawning the startup process, which
of course just kept on crashing. The cause seems to be a bug in my old
commit 442231d7f71764b8: if FatalError has been set, we suppose that the
startup process died because we SIGQUIT'd it, which is simply wrong in
this case.
AFAICS the only way to fix this properly is to explicitly track whether we
sent the startup process a kill signal. I started out with a separate
boolean, but after awhile decided that it'd be better to invent an enum
representing the startup process state, which could also subsume the
existing but rather ad-hoc flag RecoveryError. So that led me to the
attached patch.
Any thoughts/objections?
regards, tom lane
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index df8037b..77636a2 100644
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
*************** static pid_t StartupPID = 0,
*** 249,254 ****
--- 249,265 ----
PgStatPID = 0,
SysLoggerPID = 0;
+ /* Startup process's status */
+ typedef enum
+ {
+ STARTUP_NOT_RUNNING,
+ STARTUP_RUNNING,
+ STARTUP_SIGNALED, /* we sent it a SIGQUIT or SIGKILL */
+ STARTUP_CRASHED
+ } StartupStatusEnum;
+
+ static StartupStatusEnum StartupStatus = STARTUP_NOT_RUNNING;
+
/* Startup/shutdown state */
#define NoShutdown 0
#define SmartShutdown 1
*************** static pid_t StartupPID = 0,
*** 258,264 ****
static int Shutdown = NoShutdown;
static bool FatalError = false; /* T if recovering from backend crash */
- static bool RecoveryError = false; /* T if WAL recovery failed */
/*
* We use a simple state machine to control startup, shutdown, and
--- 269,274 ----
*************** static bool RecoveryError = false; /* T
*** 301,308 ****
* states, nor in PM_SHUTDOWN states (because we don't enter those states
* when trying to recover from a crash). It can be true in PM_STARTUP state,
* because we don't clear it until we've successfully started WAL redo.
- * Similarly, RecoveryError means that we have crashed during recovery, and
- * should not try to restart.
*/
typedef enum
{
--- 311,316 ----
*************** PostmasterMain(int argc, char *argv[])
*** 1246,1251 ****
--- 1254,1260 ----
*/
StartupPID = StartupDataBase();
Assert(StartupPID != 0);
+ StartupStatus = STARTUP_RUNNING;
pmState = PM_STARTUP;
/* Some workers may be scheduled to start now */
*************** reaper(SIGNAL_ARGS)
*** 2591,2596 ****
--- 2600,2606 ----
if (Shutdown > NoShutdown &&
(EXIT_STATUS_0(exitstatus) || EXIT_STATUS_1(exitstatus)))
{
+ StartupStatus = STARTUP_NOT_RUNNING;
pmState = PM_WAIT_BACKENDS;
/* PostmasterStateMachine logic does the rest */
continue;
*************** reaper(SIGNAL_ARGS)
*** 2600,2605 ****
--- 2610,2616 ----
{
ereport(LOG,
(errmsg("shutdown at recovery target")));
+ StartupStatus = STARTUP_NOT_RUNNING;
Shutdown = SmartShutdown;
TerminateChildren(SIGTERM);
pmState = PM_WAIT_BACKENDS;
*************** reaper(SIGNAL_ARGS)
*** 2624,2639 ****
/*
* After PM_STARTUP, any unexpected exit (including FATAL exit) of
* the startup process is catastrophic, so kill other children,
! * and set RecoveryError so we don't try to reinitialize after
! * they're gone. Exception: if FatalError is already set, that
! * implies we previously sent the startup process a SIGQUIT, so
* that's probably the reason it died, and we do want to try to
* restart in that case.
*/
if (!EXIT_STATUS_0(exitstatus))
{
! if (!FatalError)
! RecoveryError = true;
HandleChildCrash(pid, exitstatus,
_("startup process"));
continue;
--- 2635,2652 ----
/*
* After PM_STARTUP, any unexpected exit (including FATAL exit) of
* the startup process is catastrophic, so kill other children,
! * and set StartupStatus so we don't try to reinitialize after
! * they're gone. Exception: if StartupStatus is STARTUP_SIGNALED,
! * then we previously sent the startup process a SIGQUIT; so
* that's probably the reason it died, and we do want to try to
* restart in that case.
*/
if (!EXIT_STATUS_0(exitstatus))
{
! if (StartupStatus == STARTUP_SIGNALED)
! StartupStatus = STARTUP_NOT_RUNNING;
! else
! StartupStatus = STARTUP_CRASHED;
HandleChildCrash(pid, exitstatus,
_("startup process"));
continue;
*************** reaper(SIGNAL_ARGS)
*** 2642,2647 ****
--- 2655,2661 ----
/*
* Startup succeeded, commence normal operations
*/
+ StartupStatus = STARTUP_NOT_RUNNING;
FatalError = false;
Assert(AbortStartTime == 0);
ReachedNormalRunning = true;
*************** HandleChildCrash(int pid, int exitstatus
*** 3190,3196 ****
--- 3204,3213 ----
/* Take care of the startup process too */
if (pid == StartupPID)
+ {
StartupPID = 0;
+ StartupStatus = STARTUP_CRASHED;
+ }
else if (StartupPID != 0 && take_action)
{
ereport(DEBUG2,
*************** HandleChildCrash(int pid, int exitstatus
*** 3198,3203 ****
--- 3215,3221 ----
(SendStop ? "SIGSTOP" : "SIGQUIT"),
(int) StartupPID)));
signal_child(StartupPID, (SendStop ? SIGSTOP : SIGQUIT));
+ StartupStatus = STARTUP_SIGNALED;
}
/* Take care of the bgwriter too */
*************** PostmasterStateMachine(void)
*** 3589,3601 ****
}
/*
! * If recovery failed, or the user does not want an automatic restart
! * after backend crashes, wait for all non-syslogger children to exit, and
! * then exit postmaster. We don't try to reinitialize when recovery fails,
! * because more than likely it will just fail again and we will keep
! * trying forever.
*/
! if (pmState == PM_NO_CHILDREN && (RecoveryError || !restart_after_crash))
ExitPostmaster(1);
/*
--- 3607,3620 ----
}
/*
! * If the startup process failed, or the user does not want an automatic
! * restart after backend crashes, wait for all non-syslogger children to
! * exit, and then exit postmaster. We don't try to reinitialize when the
! * startup process fails, because more than likely it will just fail again
! * and we will keep trying forever.
*/
! if (pmState == PM_NO_CHILDREN &&
! (StartupStatus == STARTUP_CRASHED || !restart_after_crash))
ExitPostmaster(1);
/*
*************** PostmasterStateMachine(void)
*** 3615,3620 ****
--- 3634,3640 ----
StartupPID = StartupDataBase();
Assert(StartupPID != 0);
+ StartupStatus = STARTUP_RUNNING;
pmState = PM_STARTUP;
/* crash recovery started, reset SIGKILL flag */
AbortStartTime = 0;
*************** TerminateChildren(int signal)
*** 3746,3752 ****
--- 3766,3776 ----
{
SignalChildren(signal);
if (StartupPID != 0)
+ {
signal_child(StartupPID, signal);
+ if (signal == SIGQUIT || signal == SIGKILL)
+ StartupStatus = STARTUP_SIGNALED;
+ }
if (BgWriterPID != 0)
signal_child(BgWriterPID, signal);
if (CheckpointerPID != 0)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
