On Thu, Jun 22, 2017 at 6:55 PM, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: > On 2017/06/22 16:56, Michael Paquier wrote: >> On Wed, Jun 21, 2017 at 9:42 AM, Amit Langote >> <langote_amit...@lab.ntt.co.jp> wrote: >>> On 2017/06/20 20:37, Amit Kapila wrote: >>>> On Tue, Jun 20, 2017 at 1:50 PM, Amit Langote >>>> <langote_amit...@lab.ntt.co.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. > > 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. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers