Re: Refactoring postmaster's code to cleanup after child exit

On 08/08/2024 13:47, Thomas Munro wrote:
> On Windows, if a child process exits with ERROR_WAIT_NO_CHILDREN, it's
> now logged with that exit code, instead of 0. Also, if a bgworker
> exits with ERROR_WAIT_NO_CHILDREN, it's now treated as crashed and is
> restarted. Previously it was treated as a normal exit.
>
> Interesting. So when that error was first specially handled in this thread:
>
> https://www.postgresql.org/message-id/flat/AANLkTimCTkNKKrHCd3Ot6kAsrSS7SeDpOTcaLsEP7i%2BM%40mail.gmail.com#41f60947571b75377f04af67ba6baf40
>
> ... it went from being considered a crash, to being considered like
> exit(0). It's true that shared memory can't be corrupted by a process
> that never enters main(), but it's better not to hide the true reason
> for the failure (if it is still possible -- I don't find many
> references to that phenomenon in recent times). Clobbering exitstatus
> with 0 doesn't seem right at all, now that we have background workers
> whose restart behaviour is affected by that.

I adjusted this ERROR_WAIT_NO_CHILDREN a little more, to avoid logging
the death of the child twice in some cases.

>> * v3-0003-Fix-comment-on-processes-being-kept-over-a-restar.patch
>
> +1

Committed the patches up to and including this one, with tiny comment
changes.

>> * v3-0004-Consolidate-postmaster-code-to-launch-background-.patch
>
> Much of the code in process_pm_child_exit() to launch replacement
> processes when one exits or when progressing to next postmaster state
> was unnecessary, because the ServerLoop will launch any missing
> background processes anyway. Remove the redundant code and let
> ServerLoop handle it.

I'm going to work a little more on the comments on this one before
committing; I had just moved all the "If we have lost the XXX, try to
start a new one" comments as is, but they look pretty repetitive now.

Thanks for the review!

--
Heikki Linnakangas
Neon (https://neon.tech)