At Thu, 2 Sep 2021 14:44:31 +0900, Fujii Masao <masao.fu...@oss.nttdata.com> wrote in > > > On 2021/09/02 13:17, Kyotaro Horiguchi wrote: > > Did you read the comment just above? > > Yes!
Glad to hear that, or..:p > > xlog.c:12523 > >> * Check the page header immediately, so that we can retry immediately > >> if > >> * it's not valid. This may seem unnecessary, because XLogReadRecord() > >> * validates the page header anyway, and would propagate the failure up > >> to > >> * ReadRecord(), which would retry. However, there's a corner case with > >> * continuation records, if a record is split across two pages such that > > So when not in standby mode, the same check is performed by xlogreader > > which has the responsibility to validate the binary data read by > > XLogPageRead. The page-header validation is a compromise to save a > > specific case. > > Yes, so XLogPageRead() can skip the validation check of page head if > not > in standby mode. On the other hand, there is no problem if it still > performs > the validation check as it does for now. No? Practically yes, and it has always been like that as you say. > > I don't think it is good choice to conflate read-failure and header > > validation failure from the view of modularity. > > I don't think that the proposed change does that. But maybe I failed It's about your idea in a recent mail. not about the proposed patch(es). > to get > your point yet... Anyway the proposed change just tries to reset > errormsg_buf whenever XLogPageRead() retries, whatever error happened > before. Also if errormsg_buf is set at that moment, it's reported. I believe errmsg_buf is an interface to emit error messages dedicated to xlogreader that doesn't have an access to elog facility, and xlogreader doesn't (or ought not to or expect to) suppose non-xlogreader callback functions set the variable. In that sense I don't think theoriginally proposed patch is proper for the reason that the non-xlogreader callback function may set errmsg_buf. This is what I meant by the word "modularity". For that reason I avoided in my second proposal to call XLogReaderValidatePageHeader() at all while not in standby mode, because calling the validator function while in non-standby mode results in the non-xlogreader function return errmsg_buf. Of course we can instead always consume errmsg_buf in the function but I don't like to shadow the caller's task. Does that makes sense? regards. -- Kyotaro Horiguchi NTT Open Source Software Center