Re: Crash on promotion when recovery.conf is renamed - Mailing list pgsql-hackers

From Alexander Korotkov
Subject Re: Crash on promotion when recovery.conf is renamed
Date
Msg-id CAPpHfdsyC2J5-kUXrK0v3n6U8SHMm20JqT+3d3N6Xjyj4-TybA@mail.gmail.com
Whole thread Raw
In response to [HACKERS] Crash on promotion when recovery.conf is renamed  (Magnus Hagander <magnus@hagander.net>)
Responses Re: Crash on promotion when recovery.conf is renamed  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Mon, Mar 27, 2017 at 4:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> writes:
> All other places in twophase.c and most places in other files put ereport() and errmsg() on separate lines.  I think it would be better to align with surrounding code.

> +                             ereport(FATAL, (errmsg("corrupted two-phase file \"%s\"",

Actually, the *real* problem with that coding is it lacks a SQLSTATE
(errcode call).  The only places where it's acceptable to leave that
out are for internal "can't happen" cases, which this surely isn't.

Surrounding code also has ereports lacking SQLSTATE.  And that isn't "can't happen" case as well.

if (ControlFile->backupEndRequired)
ereport(FATAL,
(errmsg("WAL ends before end of online backup"),
 errhint("All WAL generated while online backup was taken must be available at recovery.")));
else if (!XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
ereport(FATAL,
(errmsg("WAL ends before end of online backup"),
 errhint("Online backup started with pg_start_backup() must be ended with pg_stop_backup(), and all WAL up to that point must be available at recovery.")));
else
ereport(FATAL,
  (errmsg("WAL ends before consistent recovery point")));

Should we consider fixing them?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

 

pgsql-hackers by date:

Previous
From: Craig Ringer
Date:
Subject: Re: Logical decoding on standby
Next
From: Robert Haas
Date:
Subject: Re: standardized backwards incompatibility tag for commits