On 2017/06/23 10:22, Masahiko Sawada wrote: > 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: >>> 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. Thanks, Amit
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c index 91e4a8cf70..52de599b29 100644 --- a/src/backend/access/gin/ginutil.c +++ b/src/backend/access/gin/ginutil.c @@ -372,6 +372,13 @@ GinInitMetabuffer(Buffer b) metadata->nDataPages = 0; metadata->nEntries = 0; metadata->ginVersion = GIN_CURRENT_VERSION; + + /* + * Set pd_lower just past the end of the metadata. This is not essential + * but it makes the page look compressible to xlog.c. + */ + ((PageHeader) page)->pd_lower = + ((char *) metadata + sizeof(GinMetaPageData)) - (char *) page; } /* diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c index 7ba04e324f..6d2e528852 100644 --- a/src/backend/access/gin/ginxlog.c +++ b/src/backend/access/gin/ginxlog.c @@ -514,7 +514,7 @@ ginRedoUpdateMetapage(XLogReaderState *record) Assert(BufferGetBlockNumber(metabuffer) == GIN_METAPAGE_BLKNO); metapage = BufferGetPage(metabuffer); - GinInitPage(metapage, GIN_META, BufferGetPageSize(metabuffer)); + GinInitMetabuffer(metabuffer); memcpy(GinPageGetMeta(metapage), &data->metadata, sizeof(GinMetaPageData)); PageSetLSN(metapage, lsn); MarkBufferDirty(metabuffer); @@ -656,7 +656,7 @@ ginRedoDeleteListPages(XLogReaderState *record) Assert(BufferGetBlockNumber(metabuffer) == GIN_METAPAGE_BLKNO); metapage = BufferGetPage(metabuffer); - GinInitPage(metapage, GIN_META, BufferGetPageSize(metabuffer)); + GinInitMetabuffer(metabuffer); memcpy(GinPageGetMeta(metapage), &data->metadata, sizeof(GinMetaPageData)); PageSetLSN(metapage, lsn); @@ -776,18 +776,11 @@ gin_mask(char *pagedata, BlockNumber blkno) mask_page_hint_bits(page); /* - * GIN metapage doesn't use pd_lower/pd_upper. Other page types do. Hence, - * we need to apply masking for those pages. + * For GIN_DELETED page, the page is initialized to empty. Hence, mask + * the page content. */ - if (opaque->flags != GIN_META) - { - /* - * For GIN_DELETED page, the page is initialized to empty. Hence, mask - * the page content. - */ - if (opaque->flags & GIN_DELETED) - mask_page_content(page); - else - mask_unused_space(page); - } + if (opaque->flags & GIN_DELETED) + mask_page_content(page); + else + mask_unused_space(page); }
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers