On Tue, Sep 27, 2016 at 9:45 AM, Peter Eisentraut
<peter.eisentr...@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.

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to