Re: Return value of pg_promote()

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