From: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Function to promote standby servers |
Date: | 2018-10-19 11:32:42 |
Message-ID: | c8e34448ee75f516e392a71e5f15d5abf07fffb9.camel@cybertec.at |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
0001-Add-pg_promote-to-promote-standby-servers-V5.patch | text/x-patch | 12.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2018-10-19 12:38:35 | Re: [HACKERS] Transactions involving multiple postgres foreign servers, take 2 |
Previous Message | Amit Langote | 2018-10-19 09:55:09 | Re: partition tree inspection functions |