Sorry, please let me add something.
At Thu, 02 Sep 2021 13:17:16 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
> At Thu, 2 Sep 2021 12:19:25 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
> > Also I'm tempted to move ereport() and reset of errmsg_buf to
> > under next_record_is_invalid as follows. That is, in standby mode
> > whenever we find an invalid record and retry reading WAL page
> > in XLogPageRead(), we report the error message and reset it.
> > For now in XLogPageRead(), only XLogReaderValidatePageHeader()
> > sets errmsg_buf, but in the future other code or function doing that
> > may be added. For that case, the following change seems more elegant.
> > Thought?
>
> I don't think it is good choice to conflate read-failure and header
> validation failure from the view of modularity.
In other words, XLogReaderValidatePageHeader is foreign for
XLogPageRead and the function indeuces the need of extra care for
errormsg_buf that is not relevant to the elog-capable module.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center