Re: make pg_ctl more friendly

On Wed, 2024-01-17 at 17:33 +0800, Junwang Zhao wrote:
> On Wed, Jan 17, 2024 at 4:54 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > I think this needs more comments.  First, in the WaitPMResult enum, we
> > currently have three values -- READY, STILL_STARTING, FAILED.  These are
> > all pretty self-explanatory.  But this patch adds SHUTDOWN_IN_RECOVERY,
> > and that's not at all self-explanatory.  Did postmaster start or not?
> > The enum value's name doesn't make that clear.  So I'd do something like
> >
> > typedef enum
> > {
> >         POSTMASTER_READY,
> >         POSTMASTER_STILL_STARTING,
> >         /*
> >          * postmaster no longer running, because it stopped after recovery
> >          * completed.
> >          */
> >         POSTMASTER_SHUTDOWN_IN_RECOVERY,
> >         POSTMASTER_FAILED,
> > } WaitPMResult;
> >
> > Maybe put the comments in wait_for_postmaster_start instead.
>
> I put the comments in WaitPMResult since we need to add two
> of those if in wait_for_postmaster_start.

I don't think that any comment is needed; the name says it all.

> > Secondly, the patch proposes to add new text to be returned by
> > do_start() when this happens, which would look like this:
> >
> >   waiting for server to start...  shut down while in recovery
> >   update recovery target settings for startup again if needed
> >
> > Is this crystal clear?  I'm not sure.  How about something like this?
> >
> >   waiting for server to start...  done, but not running
> >   server shut down because of recovery target settings
> >
> > variations on the first phrase:
> >
> > "done, no longer running"
> > "done, but no longer running"
> > "done, automatically shut down"
> > "done, automatically shut down after recovery"
>
> I chose the last one because it gives information to users.
> See V5, thanks for the wording ;)

Why not just leave it at plain "done"?
After all, the server was started successfully.
The second message should make sufficiently clear that the server has stopped.


I didn't like the code duplication introduced by the patch, so I rewrote
that part a bit.

Attached is my suggestion.

I'll set the status to "waiting for author"; if you are fine with my version,
I think that the patch is "ready for committer".

Yours,
Laurenz Albe