Re: corruption of WAL page header is never reported

On Mon, 19 Jul 2021 06:32:28 -0300
Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:

> Em seg., 19 de jul. de 2021 às 06:15, Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>
> escreveu:
>
> > On Mon, 19 Jul 2021 17:47:07 +0900 (JST)
> > Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> >
> > > At Mon, 19 Jul 2021 16:00:39 +0900, Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>
> > wrote in
> > > > Your patch doesn't fix the issue that the error message is never
> > reported in
> > > > standby mode. When a WAL page header is broken, the standby would
> > silently repeat
> > > > retrying forever.
> > >
> > > Ok, I see your point and agree to that.
> > >
> > > > I think we have to let users know the corruption of WAL page header
> > even in
> > > > standby mode, not? A corruption of WAL record header is always
> > reported,
> > > > by the way. (See that XLogReadRecord is calling ValidXLogRecordHeader.)
> > >
> > > Howeer, I'm still on the opinion that we don't need to check that
> > > while in standby mode.
> > >
> > > How about the attached?
> >
> > On Mon, 19 Jul 2021 17:50:16 +0900 (JST)
> > Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> >
> > > me> Howeer, I'm still on the opinion that we don't need to check that
> > > me> while in standby mode.
> > >
> > > Of course it's typo of "while not in standby mode".
> >
> > Thanks for updating the patch. I agree with you.
> >
> > I think it is nice to fix to perform the check only during standby mode
> > because it make a bit clearer why we check it immediately in XLogPageRead.
> >
> And as I had reviewed, your first patch was wrong and now with the Kyotaro
> version,
> to keep the same behavior, it is necessary to reset the error, correct?

Well, I think my first patch was not wrong. The difference with the latest
patch is just whether we perform the additional check when we are not in
standby mode or not. The behavior is basically the same although which function
detects and prints the page-header error in cases of crash recovery is different.
On the other hand, in your patch, the error message was always ommitted in cases
of crash recovery, and it seemed to me wrong.

Regards,
Yugo Nagata

--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>