Re: Function to promote standby servers

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

In response to

Responses

Browse pgsql-hackers by date

  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