Michael Paquier wrote:
> + /* wait for up to a minute for promotion */
> + for (i = 0; i < WAITS_PER_SECOND * WAIT_SECONDS; ++i)
> + {
> + if (!RecoveryInProgress())
> + PG_RETURN_BOOL(true);
> +
> + pg_usleep(1000000L / WAITS_PER_SECOND);
> + }
> I would recommend to avoid pg_usleep and instead use a WaitLatch() or
> similar to generate a wait event. The wait can then also be seen in
> pg_stat_activity, which is useful for monitoring purposes. Using
> RecoveryInProgress is indeed doable, and that's more simple than what I
> thought first.
Agreed, done.
I have introduced a new wait event, because I couldn't find one that fit.
> Something I missed to mention in the previous review: the timeout should
> be manually enforceable, with a default at 60s.
Ok, added as a new parameter "wait_seconds".
> Is the function marked as strict? Per the code it should be, I am not
> able to test now though.
Yes, it is.
> You are missing REVOKE EXECUTE ON FUNCTION pg_promote() in
> system_views.sql, or any users could trigger a promotion, no?
You are right *blush*.
Fixed.
Yours,
Laurenz Albe
