Re: WAL consistency check facility

From: Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Geoghegan <pg(at)heroku(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila(at)enterprisedb(dot)com>
Subject: Re: WAL consistency check facility
Date: 2016-09-14 05:56:21
Message-ID: CAGz5QCJvOQ8E5H-0cZeeYf8BJEFqVe0+VX8H_FxAnM0XHjR3_A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 14, 2016 at 6:34 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
>>>2) Regarding page comparison:
>>>We could just use memcpy() here. compareImages was useful to get a
>>>clear image of what the inconsistencies were, but you don't do that
>>>anymore.
>> memcmp(), right?
>
>Yep :)
If I use memcmp(), I won't get the byte location where the first mismatch
has occurred. It will be helpful to display the byte location which causes
an inconsistency.

>>>6)
>>>+ /*
>>>+ * Remember that, if WAL consistency check is enabled for
>>>the current rmid,
>>>+ * we always include backup image with the WAL record.
>>>But, during redo we
>>>+ * restore the backup block only if needs_backup is set.
>>>+ */
>>>+ if (needs_backup)
>>>+ bimg.bimg_info |= BKPIMAGE_IS_REQUIRED_FOR_REDO;
>>>This should use wal_consistency[rmid]?
>>
>> needs_backup is set when XLogRecordAssemble decides that backup image
>> should be included in the record for redo purpose. This image will be
>> restored during
>> redo. BKPIMAGE_IS_REQUIRED_FOR_REDO indicates whether the included
>> image should be restored during redo(or has_image should be set or not).
>
>When decoding a record, I think that you had better not use has_image
>to assume that a FPW has to be double-checked. This has better be a
>different boolean flag, say check_page or similar. This way after
>decoding a record it is possible to know if there is a PFW, and if a
>check on it is needed or not.
I've done some modifications which discards the necessity of adding
anything in DecodeXLogRecord().

Master
---------------
- If wal_consistency check is enabled or needs_backup is set in
XLogRecordAssemble(), we do a fpw.
- If a fpw is to be done, then fork_flags is set with BKPBLOCK_HAS_IMAGE,
which in turns set has_image flag while decoding the record.
- If a fpw needs to be restored during redo, i.e., needs_backup is true,
then bimg_info is set with BKPIMAGE_IS_REQUIRED_FOR_REDO.

Standby
---------------
- In XLogReadBufferForRedoExtended(), if both XLogRecHasBlockImage() and
XLogRecHasBlockImageForRedo()(added by me*) return true, we restore the
backup image.
- In checkConsistency, we only check if XLogRecHasBlockImage() returns true
when wal_consistency check is enabled for this rmid.

*XLogRecHasBlockImageForRedo() checks whether bimg_info is set with
BKPIMAGE_IS_REQUIRED_FOR_REDO.

Thoughts?

--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-09-14 06:01:41 Re: WAL consistency check facility
Previous Message Michael Paquier 2016-09-14 05:51:48 Re: kqueue