Re: Function to promote standby servers

Masahiko Sawada wrote:
> On Mon, Oct 22, 2018 at 3:01 PM Michael Paquier <michael@paquier.xyz> wrote:
> > Regarding the documentation, wouldn't it be more adapted to list the new
> > function under the section "Recovery Control Functions"?  Not only does
> > the new function signal the postmaster, but it also creates the
> > promotion trigger file.

Hmm, yes, that's probably the first place where people would look.
I guess the implementation lead me to categorize it as a "signaling function",
and it also wouldn't be wrong there.

> > Anyway, I have been looking in depth at the patch, and I finish with the
> > attached.  Here are some notes:
> > [...]
> > - WL_TIMEOUT could have been reached, leading to no further retries
> > (remove for example the part related to the trigger file creation and
> > try to trigger the promotion, the wait time is incorrect).

Ok.

> > - Documentation has been reworked.
> > - The new wait event is documented.

Thanks.

> > - position including unistd.h was not correct in xlogfuncs.c.
> > - Timeouts for the tests are made a tad longer.  Some buildfarm machines
> > don't like a promotion wait of 10s.
> > - a catalog version bump is included, just a personal reminder..
> > - Indentatio has been run.

Thanks.

> Thank you for workig on this. There is one review comment for the latest patch.
> 
> +     if (FreeFile(promote_file))
> +               ereport(ERROR,
> +                               (errcode_for_file_access(),
> +                                errmsg("could not write file \"%s\": %m",
> +                                               PROMOTE_SIGNAL_FILE)));
> 
> Maybe we should unlink PROMOTE_SIGNAL_FILE before erroring.

Yes, that cannot hurt.

Yours,
Laurenz Albe