On Mon, Mar 27, 2017 at 11:20 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Alexander Korotkov <a.korot...@postgrespro.ru> writes: >> On Mon, Mar 27, 2017 at 4:48 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> 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. >> Should we consider fixing them? > > 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?
While I agree with that in general, we are taking about 2PC files that are on disk in patch 0001, so I would think that in this case ERRCODE_DATA_CORRUPTED is the most adapted (or ERRCODE_TWOPHASE_CORRUPTED?). The other WARNING messages refer to stale files of already committed transactions, which are not actually corrupted. What about in this case having a ERRCODE_TWOPHASE_INVALID? Updated patches are attached, I did not change the WARNING portion though as I am not sure what's the consensus on the matter. -- Michael
0001-Change-detection-of-corrupted-2PC-files-as-FATAL.patch
Description: Binary data
0002-Minimize-window-between-history-file-and-end-of-reco.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
