Thread: Re: [COMMITTERS] pgsql: pg_ctl: Detect current standby state from pg_control
Re: [COMMITTERS] pgsql: pg_ctl: Detect current standby state from pg_control
From
Peter Eisentraut
Date:
On 9/26/16 7:56 PM, Peter Eisentraut wrote: > On 9/26/16 8:53 AM, Tom Lane wrote: >> I think that it's 100% pointless for get_control_dbstate >> to be worried about transient CRC failures. If writes to pg_control >> aren't atomic then we have problems enormously larger than whether >> "pg_ctl promote" throws an error or not. > > The new code was designed to handle that, but if we think it's not an > issue, then we can simplify it. I'm on it. How about this? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
Re: [COMMITTERS] pgsql: pg_ctl: Detect current standby state from pg_control
From
Michael Paquier
Date:
On Tue, Sep 27, 2016 at 9:45 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 9/26/16 7:56 PM, Peter Eisentraut wrote:
>> On 9/26/16 8:53 AM, Tom Lane wrote:
>>> I think that it's 100% pointless for get_control_dbstate
>>> to be worried about transient CRC failures. If writes to pg_control
>>> aren't atomic then we have problems enormously larger than whether
>>> "pg_ctl promote" throws an error or not.
>>
>> The new code was designed to handle that, but if we think it's not an
>> issue, then we can simplify it. I'm on it.
>
> How about this?
+ if (crc_ok_p)
+ *crc_ok_p = false;
+ else
+ {
+ pfree(ControlFile);
+ return NULL;
+ }
Seems overcomplicated to me. How about returning the control file all
the time and let the caller pfree the result? You could then use
crc_ok in pg_ctl.c's get_control_dbstate() to do the decision-making.
Also, if the sleep() loop is removed for promote -w, we assume that
we'd fail immediately in case of a corrupted control file. Could it be
possible that after a couple of seconds the backend gets it right and
overwrites a correct control file on top of a corrupted one? I am just
curious about such possibilities, still I am +1 for removing the
pg_usleep loop and failing right away as you propose.
If we do the renaming of get_controlfile(), it may be also a good idea
to do the same for get_configdata() in config_info.c for consistency.
That's not really critical IMHO though.
--
Michael
Re: [COMMITTERS] pgsql: pg_ctl: Detect current standby state from pg_control
From
Michael Paquier
Date:
On Tue, Sep 27, 2016 at 9:55 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > Seems overcomplicated to me. How about returning the control file all > the time and let the caller pfree the result? You could then use > crc_ok in pg_ctl.c's get_control_dbstate() to do the decision-making. In short I would just go with the attached and call it a day. -- Michael
Attachment
Re: [COMMITTERS] pgsql: pg_ctl: Detect current standby state from pg_control
From
Peter Eisentraut
Date:
On 9/28/16 12:44 AM, Michael Paquier wrote: > On Tue, Sep 27, 2016 at 9:55 AM, Michael Paquier > <michael.paquier@gmail.com> wrote: >> > Seems overcomplicated to me. How about returning the control file all >> > the time and let the caller pfree the result? You could then use >> > crc_ok in pg_ctl.c's get_control_dbstate() to do the decision-making. > In short I would just go with the attached and call it a day. Pushed that way. Thanks! -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services