Re: Setting pd_lower in GIN metapage

On Mon, Sep 11, 2017 at 5:40 PM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> On 2017/09/10 15:22, Michael Paquier wrote:
>> On Sun, Sep 10, 2017 at 3:15 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>> On Sat, Sep 9, 2017 at 9:00 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>>>>> On Fri, Sep 8, 2017 at 5:24 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>>> In short, this patch needs a significant rewrite, and more analysis than
>>>>>> you've done so far on whether there's actually any benefit to be gained.
>>>>>> It might not be worth messing with.
>>>>
>>>>> I did some measurements of the compressibility of the GIN meta page,
>>>>> looking at its FPWs with and without wal_compression and you are
>>>>> right: there is no direct compressibility effect when setting pd_lower
>>>>> on the meta page. However, it seems to me that there is an argument
>>>>> still pleading on favor of this patch for wal_consistency_checking.
>>>>
>>>> I think that would be true if we did both my point 1 and 2, so that
>>>> the wal replay functions could trust pd_lower to be sane in all cases.
>>>> But really, if you have to touch all the places that write these
>>>> metapages, you might as well mark them REGBUF_STANDARD while at it.
>>>>
>>>>> The same comment ought to be mentioned for btree.
>>>>
>>>> Yeah, I was wondering if we ought not clean up btree/hash while at it.
>>>> At the very least, their existing comments saying that it's inessential
>>>> to set pd_lower could use some more detail about why or why not.
>>>>
>>>
>>> +1. I think we can even use REGBUF_STANDARD in the hash for metapage
>>> where currently it is not used. I can give a try to write a patch for
>>> hash/btree part if you want.
>>
>> Coordinating efforts here would be nice. If you, Amit K, are taking
>> care of a patch for btree and hash, would you, Amit L, write the part
>> for GIN, BRIN and SpGist? This needs a careful lookup as many code
>> paths need a lookup so it may take time. Please note that I don't mind
>> double-checking this part if you don't have enough room to do so.
>
> Sorry, I didn't have time today to carefully go through the recent
> discussion on this thread (starting with Tom's email wherein he said he
> set the status of the patch to Waiting on Author). I will try tomorrow.

Thanks for the update! Once you get to this point, please let me know
if you would like to work on a more complete patch for brin, gin and
spgist. If you don't have enough room, I am fine to produce something.
--
Michael