Re: Make mesage at end-of-recovery less scary.

At Thu, 3 Mar 2022 15:39:44 +0530, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote in
> The new changes made in the patch look good. Thanks to the recent
> changes to speed WAL insertion that have helped us catch this bug.

Thanks for the quick checking.

> One small comment:
>
> record = (XLogRecord *) (state->readBuf + RecPtr % XLOG_BLCKSZ);
> - total_len = record->xl_tot_len;
>
> Do you think we need to change the position of the comments written
> for above code that says:

Yeah, I didn't do that since it is about header verification. But as
you pointed, the result still doesn't look perfect.

On second thought the two seems repeating the same things. Thus I
merged the two comments together. In this verion 16 it looks like
this.

> /*
> * Validate the record header.
> *
> * Even though we use an XLogRecord pointer here, the whole record header
> * might not fit on this page. If the whole record header is on this page,
> * validate it immediately. Even otherwise xl_tot_len must be on this page
> * (it is the first field of MAXALIGNed records), but we still cannot
> * access any further fields until we've verified that we got the whole
> * header, so do just a basic sanity check on record length, and validate
> * the rest of the header after reading it from the next page. The length
> * check is necessary here to ensure that we enter the "Need to reassemble
> * record" code path below; otherwise we might fail to apply
> * ValidXLogRecordHeader at all.
> */
> record = (XLogRecord *) (state->readBuf + RecPtr % XLOG_BLCKSZ);
>
> if (targetRecOff <= XLOG_BLCKSZ - SizeOfXLogRecord)

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center