Re: Reorder shutdown sequence, to flush pgstats later

Hi,

On 2025-01-13 12:20:39 +0000, Bertrand Drouvot wrote:
> > We have
> > multiple copies of code to go to FatalError, that doesn't seem great.
>
> + * FIXME: This should probably not have a copy fo the code to
> + * transition into FatalError state.
> + */
> + ereport(LOG,
> + (errmsg("WAL was shut down unexpectedly")));
>
> Indeed, might be worth extracting this into a helper function or such.

That is quite a rats nest of issues :(. There are quite a few oddities around
the related code. The attached series contains a few commits trying to unify
the paths transitioning to FatalError = true into the new HandleFatalError().

The current code has the weird behaviour of going through PM_WAIT_BACKENDS. I
left it like that for now. In fact more paths do so now, because we did so for
PM_WAIT_XLOG_SHUTDOWN but *didn't* do so for PM_WAIT_XLOG_ARCHIVAL, which
seems rather weird.

I suspect it'd be better to switch directly to PM_DEAD_END and have
HandleFatalError always do ConfigurePostmasterWaitSet(false), but that's left
for another time.

After the earlier commit that just moves the code I turned the existing if ()
into a switch so that whoever adds new states is told to look at that code by
the compiler.

Thoughts?

> > Another thing I started worrying about is that the commit added
> > PM_WAIT_CHECKPOINTER *after* PM_WAIT_DEAD_END. However, we have a few places
> > where we directly jump to PM_WAIT_DEAD_END in fatal situations:

Attached patch is implemented that way.

Greetings,

Andres Freund