On Wed, Aug 16, 2023 at 05:02:09PM +0900, Michael Paquier wrote:
> if (kill(PostmasterPid, SIGUSR1) != 0)
> {
> - ereport(WARNING,
> - (errmsg("failed to send signal to postmaster: %m")));
> (void) unlink(PROMOTE_SIGNAL_FILE);
> - PG_RETURN_BOOL(false);
> + ereport(ERROR,
> + (errmsg("failed to send signal to postmaster: %m")));
> }
>
> Shouldn't you assign an error code to this one rather than the
> default one for internal errors, like ERRCODE_SYSTEM_ERROR?
>
> /* return immediately if waiting was not requested */
> @@ -744,7 +743,9 @@ pg_promote(PG_FUNCTION_ARGS)
> * necessity for manual cleanup of all postmaster children.
> */
> if (rc & WL_POSTMASTER_DEATH)
> - PG_RETURN_BOOL(false);
> + ereport(FATAL,
> + (errcode(ERRCODE_ADMIN_SHUTDOWN),
> + errmsg("terminating connection due to unexpected postmaster exit")));
>
> I would add an errcontext here, to let somebody know that the
> connection died while waiting for the promotion to be processed, say
> "while waiting on promotion".
I have just noticed that we do not have a CF entry for this proposal,
so I have added one with Laurenz as author:
https://commitfest.postgresql.org/44/4504/
For now the patch is waiting on author. Could you address my
last review?
--
Michael