Hi Michael,
On Thu, Aug 17, 2023 at 6:07 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> 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?
Thanks for reviewing the patch and adding a CF entry for it. PFA patch
that addresses your review comments.
And... Sorry for the delayed response. I totally missed it.
--
With Regards,
Ashutosh Sharma.