Re: Setting pd_lower in GIN metapage

On Fri, Jun 23, 2017 at 11:17 AM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> On 2017/06/23 10:22, Masahiko Sawada wrote:
>> On Thu, Jun 22, 2017 at 6:55 PM, Amit Langote
>> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>> On 2017/06/22 16:56, Michael Paquier wrote:
>>>> 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.
>>>
>>> Oh, wasn't sure about the gin_mask() changes myself. Thanks for checking.
>>>
>>> Actually, the WAL consistency check fails even without patching
>>> gin_mask(), so the problem may be with the main patch itself. That is,
>>> the patch needs to do something else other than just teaching
>>> GinInitMetabuffer() to initialize pd_lower. Will look into that.
>>>
>>
>> I've not read the code deeply but I guess we should use
>> GinInitMetabuffer() in ginRedoUpdateMetapage() instead of
>> GinInitPage(). Maybe also GinInitPage() in ginRedoDeleteListPages() is
>> the same.
>
> That was it, thanks for the pointer.
>
> Attached updated patch, which I confirmed, passes wal_consistency_check = gin.

Thank you for updating the patch. It looks good to me.
BTW I'm inclined to have a regression test case where doing 'make
check' to the streaming replication environment with
wal_consistency_check on standby server so that we can detect a bug
around the wal.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center