In pursuit of the problem with standby servers sometimes not responding to fast shutdowns [1], I spent awhile staring at the postmaster's state-machine logic. I have not found a cause for that problem, but I have identified some other things that seem like bugs:
1. sigusr1_handler ignores PMSIGNAL_ADVANCE_STATE_MACHINE unless the current state is PM_WAIT_BACKUP or PM_WAIT_BACKENDS. This restriction seems useless and shortsighted: PostmasterStateMachine should behave sanely regardless of our state, and sigusr1_handler really has no business assuming anything about why a child is asking for a state machine reconsideration. But it's not just not future-proof, it's a live bug even for the one existing use-case, which is that a new walsender sends this signal after it's re-marked itself as being a walsender rather than a normal backend. Consider this sequence of events: * System is running as a hot standby and allowing cascaded replication. There are no live backends. * New replication connection request is received and forked off. (At this point the postmaster thinks this child is a normal session backend.) * SIGTERM (Smart Shutdown) is received. Postmaster will transition to PM_WAIT_READONLY. I don't think it would have autovac or bgworker or bgwriter or walwriter children, but if so, assume they all exit before the next step. Postmaster will continue to sleep, waiting for its one "normal" child backend to finish. * Replication connection request completes, so child re-marks itself as a walsender and sends PMSIGNAL_ADVANCE_STATE_MACHINE. * Postmaster ignores signal because it's in the "wrong" state, so it doesn't realize it now has no normal backend children. * Postmaster waits forever, or at least till DBA loses patience and sends a stronger signal. This scenario doesn't explain the buildfarm failures since those don't involve smart shutdowns (and I think they don't involve cascaded replication either). Still, it's clearly a bug, which I think we should fix by removing the pointless restriction on whether PostmasterStateMachine can be called. Also, I'm inclined to think that that should be the *last* step in sigusr1_handler, not randomly somewhere in the middle. As coded, it's basically assuming that no later action in sigusr1_handler could affect anything that PostmasterStateMachine cares about, which even if it's true today is another highly not-future-proof assumption. 2. MaybeStartWalReceiver will clear the WalReceiverRequested flag even if it fails to launch a child process for some reason. This is just dumb; it should leave the flag set so that we'll try again next time through the postmaster's idle loop. 3. PostmasterStateMachine's handling of PM_SHUTDOWN_2 is: if (pmState == PM_SHUTDOWN_2) { /* * PM_SHUTDOWN_2 state ends when there's no other children than * dead_end children left. There shouldn't be any regular backends * left by now anyway; what we're really waiting for is walsenders and * archiver. * * Walreceiver should normally be dead by now, but not when a fast * shutdown is performed during recovery. */ if (PgArchPID == 0 && CountChildren(BACKEND_TYPE_ALL) == 0 && WalReceiverPID == 0) { pmState = PM_WAIT_DEAD_END; } } The comment about walreceivers is confusing, and it's also wrong. Maybe it was valid when written, but today it's easy to trace the logic and see that we can only get to PM_SHUTDOWN_2 state from PM_SHUTDOWN state, and we can only get to PM_SHUTDOWN state when there is no live walreceiver (cf processing of PM_WAIT_BACKENDS state), and we won't attempt to launch a new walreceiver while in PM_SHUTDOWN or PM_SHUTDOWN_2 state, so it's impossible for there to be any walreceiver here. I think we should just remove that comment and the WalReceiverPID == 0 test. Comments? I think at least the first two points need to be back-patched. regards, tom lane [1] https://www.postgresql.org/message-id/20190416070119.gk2...@paquier.xyz