Re: Crash on promotion when recovery.conf is renamed

From: pgsql-hackers-owner(at)postgresql(dot)org
> [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Tom Lane
> "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)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.

Oh, I overlooked it. But...

> Yup. Just remember that the default is
> XX000 E ERRCODE_INTERNAL_ERROR internal_error
>
> If that's not how you want the error case reported, you need an errcode()
> call.
>
> We might need more ERRCODEs than are there now, if none of the existing
> ones seem to fit these cases. There's already ERRCODE_DATA_CORRUPTED and
> ERRCODE_INDEX_CORRUPTED; maybe we need ERRCODE_WAL_CORRUPTED, for
> example?

I'd be always happy if the error code is more specific, but maybe that would be a separate patch. WAL corruption message so far doesn't accompany a specific error code like this in xlog.c:

/*
* We only end up here without a message when XLogPageRead()
* failed - in that case we already logged something. In
* StandbyMode that only happens if we have been triggered, so we
* shouldn't loop anymore in that case.
*/
if (errormsg)
ereport(emode_for_corrupt_record(emode,
RecPtr ? RecPtr : EndRecPtr),
(errmsg_internal("%s", errormsg) /* already translated */ ));

Regards
Takayuki Tsunakawa