Re: Crash on promotion when recovery.conf is renamed

From: pgsql-hackers-owner@postgresql.org
> [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Tom Lane
> "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.

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
messageso 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
alreadylogged something. In         * StandbyMode that only happens if we have been triggered, so we         *
shouldn'tloop anymore in that case.         */        if (errormsg)            ereport(emode_for_corrupt_record(emode,
                                          RecPtr ? RecPtr : EndRecPtr),            (errmsg_internal("%s", errormsg) /*
alreadytranslated */ ));
 

Regards
Takayuki Tsunakawa