Re: Setting pd_lower in GIN metapage

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Setting pd_lower in GIN metapage
Date: 2017-06-22 07:56:59
Message-ID: CAB7nPqQu6b3PE4J4XUqZZXEHja=OZCUnYG3LrBSw2bcixptx1g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jun 21, 2017 at 9:42 AM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> On 2017/06/20 20:37, Amit Kapila wrote:
>> On Tue, Jun 20, 2017 at 1:50 PM, Amit Langote
>> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>> On 2017/06/19 23:31, Tom Lane wrote:
>>>> I'd suggest a rule like "if pd_lower is smaller than SizeOfPageHeaderData
>>>> then don't trust it, but assume all of the page is valid data".
>>>
>>> Actually, such a check is already in place in the tool, whose condition
>>> looks like:
>>>
>>> if (PageGetPageSize(header) == BLCKSZ &&
>>> PageGetPageLayoutVersion(header) == PG_PAGE_LAYOUT_VERSION &&
>>> (header->pd_flags & ~PD_VALID_FLAG_BITS) == 0 &&
>>> header->pd_lower >= SizeOfPageHeaderData &&
>>> header->pd_lower <= header->pd_upper &&
>>> header->pd_upper <= header->pd_special &&
>>> header->pd_special <= BLCKSZ &&
>>> header->pd_special == MAXALIGN(header->pd_special) && ...
>>>
>>> which even GIN metapage passes, making it an eligible data page and hence
>>> for omitting the hole between pd_lower and pd_upper.
>>>
>>
>> Won't checking for GIN_META in header->pd_flags gives you what you want?
>
> GIN_META flag is not written into pd_flags but GinPageOpaqueData.flags,
> which still requires including GIN's private header.

Did you check this patch with wal_consistency_checking? I am getting
failures so your patch does not have the masking of GIN pages
completely right:
FATAL: inconsistent page found, rel 1663/16385/28133, forknum 0, blkno 0
CONTEXT: WAL redo at 0/39379EB8 for Gin/UPDATE_META_PAGE:
That's easily reproducible with installcheck and a standby replaying
the changes. I did not look at the code in details to see what you may
be missing here.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kang Yuzhe 2017-06-22 08:05:03 SQL MERGE patches for PostgreSQL Versions
Previous Message Masahiko Sawada 2017-06-22 07:26:39 Re: Get stuck when dropping a subscription during synchronizing table