On 2017/06/26 10:54, Michael Paquier wrote:
> On Fri, Jun 23, 2017 at 11:17 AM, Amit Langote
> <langote_amit...@lab.ntt.co.jp> wrote:
>> That was it, thanks for the pointer.
>
> GinInitMetabuffer() sets up pd_lower and pd_upper anyway using
> PageInit so the check of PageIsVerified is guaranteed to work in any
> case. Upgraded pages will still have their pd_lower set to the
> previous values, and new pages will have the optimization. So this
> patch is actually harmless for past pages, while newer ones are seen
> as more compressible.
Right.
>> Attached updated patch, which I confirmed, passes wal_consistency_check =
>> gin.
>
> I have spent some time looking at this patch, playing with pg_upgrade
> to check the state of the page upgraded. And this looks good to me.
Thanks for the review.
> One thing that I noticed is that this optimization could as well
> happen for spgist meta pages. What do others think?
I agree. As Sawada-san mentioned, brin metapage code can use a similar patch.
So attached are three patches for gin, brin, and sp-gist respectively.
Both brin and sp-gist cases didn't require any special consideration for
passing wal_consistency_checking, as the patch didn't cause brin and
sp-gist metapages to become invalid when recreated on standby (remember
that patch 0001 needed to be updated for that).
Thanks,
Amit
From 05d15c1ed3a49dda0e82f9fc18c9af3c6c15ebff Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Fri, 23 Jun 2017 11:20:41 +0900
Subject: [PATCH 1/3] Set pd_lower correctly in the GIN metapage.
---
src/backend/access/gin/ginutil.c | 7 +++++++
src/backend/access/gin/ginxlog.c | 23 ++++++++---------------
2 files changed, 15 insertions(+), 15 deletions(-)
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);
}
--
2.11.0
From 260b2536520ab665c44ccc1a78053013dc202c6f Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 26 Jun 2017 15:13:32 +0900
Subject: [PATCH 2/3] Set pd_lower correctly in the BRIN index metapage
---
src/backend/access/brin/brin_pageops.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/backend/access/brin/brin_pageops.c
b/src/backend/access/brin/brin_pageops.c
index 80f803e438..8762356433 100644
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -491,6 +491,13 @@ brin_metapage_init(Page page, BlockNumber pagesPerRange,
uint16 version)
* revmap page to be created when the index is.
*/
metadata->lastRevmapPage = 0;
+
+ /*
+ * Set pd_lower just past the end of the metadata. This is to log full
+ * page image of metapage in xloginsert.c.
+ */
+ ((PageHeader) page)->pd_lower =
+ ((char *) metadata + sizeof(BrinMetaPageData)) - (char *) page;
}
/*
--
2.11.0
From 8ce6e5493879b4d381628f1199498c7b0de82c59 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 26 Jun 2017 15:23:34 +0900
Subject: [PATCH 3/3] Set pd_lower correctly in the SP-GiST index metapage
---
src/backend/access/spgist/spgutils.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/backend/access/spgist/spgutils.c
b/src/backend/access/spgist/spgutils.c
index 8656af453c..f031c8c68c 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -534,6 +534,13 @@ SpGistInitMetapage(Page page)
/* initialize last-used-page cache to empty */
for (i = 0; i < SPGIST_CACHED_PAGES; i++)
metadata->lastUsedPages.cachedPage[i].blkno =
InvalidBlockNumber;
+
+ /*
+ * Set pd_lower just past the end of the metadata. This is to log full
+ * page image of metapage in xloginsert.c.
+ */
+ ((PageHeader) page)->pd_lower =
+ ((char *) metadata + sizeof(SpGistMetaPageData)) - (char *)
page;
}
/*
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
