Horton

Thread

Commits

  1. For wal_consistency_checking, mask page checksum as well as page LSN.

  1. "inconsistent page found" with checksum and wal_consistency_checking enabled

    Ashwin Agrawal <aagrawal@pivotal.io> — 2017-09-19T20:23:58Z

    Currently, page checksum is not masked by Page masking routines used
by wal_consistency_checking facility. So, when running `make installcheck`
with data checksum enabled and wal_consistency_checking='all', it easily
and consistently FATALs with "inconsistent page found".

If anything needs to be masked on Page to perform / pass wal consistency
checking, definitely checksum is not going to match and hence must be
masked as well. Attaching patch to fix the same, installcheck passes with
checksums enabled and wal_consistency_checking='all' with the fix.

Clubbed to perform the masking with lsn as it sounds logical to have them
together, as lsn is masked is all the cases so far and such is needed for
checksum as well.

Thank You,
Ashwin Agrawal

  2. Re: "inconsistent page found" with checksum and wal_consistency_checking enabled

    Michael Paquier <michael.paquier@gmail.com> — 2017-09-20T04:52:15Z

    On Wed, Sep 20, 2017 at 5:23 AM, Ashwin Agrawal <aagrawal@pivotal.io> wrote:
> Currently, page checksum is not masked by Page masking routines used by
> wal_consistency_checking facility. So, when running `make installcheck` with
> data checksum enabled and wal_consistency_checking='all', it easily and
> consistently FATALs with "inconsistent page found".

Indeed. This had better be fixed before PG10 is out. I am adding an open item.

> If anything needs to be masked on Page to perform / pass wal consistency
> checking, definitely checksum is not going to match and hence must be masked
> as well. Attaching patch to fix the same, installcheck passes with checksums
> enabled and wal_consistency_checking='all' with the fix.
>
> Clubbed to perform the masking with lsn as it sounds logical to have them
> together, as lsn is masked is all the cases so far and such is needed for
> checksum as well.

Agreed.

  * In consistency checks, the LSN of the two pages compared will likely be
- * different because of concurrent operations when the WAL is generated
- * and the state of the page when WAL is applied.
+ * different because of concurrent operations when the WAL is generated and
+ * the state of the page when WAL is applied. Also, mask out checksum as
+ * masking anything else on page means checksum is not going to match as well.
  */
Nit: Using "the LSN and the checksum" instead of the "the LSN".
-- 
Michael

  3. Re: "inconsistent page found" with checksum and wal_consistency_checking enabled

    Kuntal Ghosh <kuntalghosh.2007@gmail.com> — 2017-09-20T05:26:07Z

    On Wed, Sep 20, 2017 at 10:22 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Wed, Sep 20, 2017 at 5:23 AM, Ashwin Agrawal <aagrawal@pivotal.io> wrote:
>> Currently, page checksum is not masked by Page masking routines used by
>> wal_consistency_checking facility. So, when running `make installcheck` with
>> data checksum enabled and wal_consistency_checking='all', it easily and
>> consistently FATALs with "inconsistent page found".
>
> Indeed. This had better be fixed before PG10 is out. I am adding an open item.
>
Oops and surprised! How come we missed that previously. If page lsns
are different, checksums will be different as well. Anyhow, nice catch
and thanks for the patch.


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

  4. Re: "inconsistent page found" with checksum and wal_consistency_checking enabled

    Michael Paquier <michael.paquier@gmail.com> — 2017-09-20T05:35:06Z

    On Wed, Sep 20, 2017 at 2:26 PM, Kuntal Ghosh
<kuntalghosh.2007@gmail.com> wrote:
> On Wed, Sep 20, 2017 at 10:22 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Wed, Sep 20, 2017 at 5:23 AM, Ashwin Agrawal <aagrawal@pivotal.io> wrote:
>>> Currently, page checksum is not masked by Page masking routines used by
>>> wal_consistency_checking facility. So, when running `make installcheck` with
>>> data checksum enabled and wal_consistency_checking='all', it easily and
>>> consistently FATALs with "inconsistent page found".
>>
>> Indeed. This had better be fixed before PG10 is out. I am adding an open item.
>>
> Oops and surprised! How come we missed that previously. If page lsns
> are different, checksums will be different as well. Anyhow, nice catch
> and thanks for the patch.

That happens. We have really covered maaany points during many rounds
of reviews, still I am not surprised to see one or two things that
fell into the cracks.
-- 
Michael

  5. Re: "inconsistent page found" with checksum and wal_consistency_checking enabled

    Kuntal Ghosh <kuntalghosh.2007@gmail.com> — 2017-09-20T06:02:26Z

    On Wed, Sep 20, 2017 at 11:05 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Wed, Sep 20, 2017 at 2:26 PM, Kuntal Ghosh
> <kuntalghosh.2007@gmail.com> wrote:
>> On Wed, Sep 20, 2017 at 10:22 AM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> On Wed, Sep 20, 2017 at 5:23 AM, Ashwin Agrawal <aagrawal@pivotal.io> wrote:
>>>> Currently, page checksum is not masked by Page masking routines used by
>>>> wal_consistency_checking facility. So, when running `make installcheck` with
>>>> data checksum enabled and wal_consistency_checking='all', it easily and
>>>> consistently FATALs with "inconsistent page found".
>>>
>>> Indeed. This had better be fixed before PG10 is out. I am adding an open item.
>>>
>> Oops and surprised! How come we missed that previously. If page lsns
>> are different, checksums will be different as well. Anyhow, nice catch
>> and thanks for the patch.
>
> That happens. We have really covered maaany points during many rounds
> of reviews, still I am not surprised to see one or two things that
> fell into the cracks.
Yup, that's true. :-)


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

  6. Re: "inconsistent page found" with checksum and wal_consistency_checking enabled

    Noah Misch <noah@leadboat.com> — 2017-09-22T14:42:33Z

    On Wed, Sep 20, 2017 at 01:52:15PM +0900, Michael Paquier wrote:
> On Wed, Sep 20, 2017 at 5:23 AM, Ashwin Agrawal <aagrawal@pivotal.io> wrote:
> > Currently, page checksum is not masked by Page masking routines used by
> > wal_consistency_checking facility. So, when running `make installcheck` with
> > data checksum enabled and wal_consistency_checking='all', it easily and
> > consistently FATALs with "inconsistent page found".
> 
> Indeed. This had better be fixed before PG10 is out. I am adding an open item.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com

  7. Re: "inconsistent page found" with checksum and wal_consistency_checking enabled

    Robert Haas <robertmhaas@gmail.com> — 2017-09-22T18:36:31Z

    On Wed, Sep 20, 2017 at 12:52 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Indeed. This had better be fixed before PG10 is out. I am adding an open item.

This seems a little hyperbolic to me.  Sure, it's a new bug in v10,
and sure, it's always better to fix bugs sooner rather than later, but
there's nothing particularly serious or urgent about this bug as
compared to any other one.

I've committed the proposed patch to master and REL_10_STABLE.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company