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

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: ashu(dot)coek88(at)gmail(dot)com
Cc: pashkin(dot)elfe(at)gmail(dot)com, michael(at)paquier(dot)xyz, bossartn(at)amazon(dot)com, david(at)pgmasters(dot)net, peter(dot)eisentraut(at)2ndquadrant(dot)com, andres(at)anarazel(dot)de, pgsql-hackers(at)lists(dot)postgresql(dot)org, jtc331(at)gmail(dot)com, robertmhaas(at)gmail(dot)com
Subject: Re: Make mesage at end-of-recovery less scary.
Date: 2022-03-04 00:43:59
Message-ID: 20220304.094359.2159611925830858692.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v16-0001-Make-End-Of-Recovery-error-less-scary.patch text/x-patch 19.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2022-03-04 01:09:19 Re: pg_stop_backup() v2 incorrectly marked as proretset
Previous Message Kyotaro Horiguchi 2022-03-04 00:10:48 Re: standby recovery fails (tablespace related) (tentative patch and discussion)