Re: Setting pd_lower in GIN metapage

On Tue, Sep 19, 2017 at 9:27 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Sep 19, 2017 at 12:40 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Mon, Sep 18, 2017 at 4:03 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> On Mon, Sep 18, 2017 at 11:16 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>>> On Sat, Sep 16, 2017 at 7:15 PM, Michael Paquier
>>>> <michael.paquier@gmail.com> wrote:
>>>>>
>>>>
>>>> You have already noticed above that it will help when
>>>> wal_checking_consistency is used and that was the main motivation to
>>>> pass REGBUF_STANDARD apart from maintaining consistency.  It is not
>>>> clear to me what is bothering you.  If your only worry about these
>>>> patches is that you want this sentence to be removed from the comment
>>>> because you think it is obvious or doesn't make much sense, then I
>>>> think we can leave this decision to committer.  I have added it based
>>>> on Tom's suggestion above thread about explaining why it is
>>>> inessential or essential to set pd_lower.  I think Amit Langote just
>>>> tried to mimic what I have done in hash and btree patches to maintain
>>>> consistency.  I am also not very sure if we should write some detailed
>>>> comment or leave the existing comment as it is.  I think it is just a
>>>> matter of different perspective.
>>>
>>> What is disturbing me a bit is that the existing comments mention
>>> something that could be supported (the compression of pages), but
>>> that's actually not done and this is unlikely to happen because the
>>> number of bytes associated to a meta page is going to be always
>>> cheaper than a FPW, which would cost in CPU to store it for
>>> compression is enabled. So I think that we should switch comments to
>>> mention that pd_lower is set so as this helps with page masking, but
>>> we don't take advantage of XLOG compression in the code.
>>>
>>
>> I think that is not true because we do need FPW for certain usages of
>> metapage.  Consider a case in _hash_doinsert where register metabuf
>> with just
>> REGBUF_STANDARD, it can take advantage of removing the hole if
>> pd_lower is set to its correct position.
>
> I am not saying that no index AMs take advantage FPW compressibility
> for their meta pages. There are cases like this one, as well as one
> code path in BRIN where this is useful, and it is useful as well when
> logging FPWs of the init forks for unlogged relations.
>

Hmm, why is it useful for logging FPWs of the init forks for unlogged
relations?  We don't use REGBUF_STANDARD in those cases.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com