On 2017/09/11 18:13, Michael Paquier wrote:
> On Mon, Sep 11, 2017 at 5:40 PM, Amit Langote wrote:
>> On 2017/09/10 15:22, Michael Paquier wrote:
>>> 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.
I updated the patches for GIN, BRIN, and SP-GiST to include the following
changes:
1. Pass REGBUF_STNADARD flag when registering the metapage buffer
2. Pass true for page_std argument of log_newpage() or
log_newpage_buffer() when using it to log the metapage
3. Update comment near where pd_lower is set in the respective metapages,
similar to what Amit K did in his patches for btree and hash metapages.
Did I miss something from the discussion?
Thanks,
Amit
From bf291247e8f49d4558ab70fa335bd5a7bdda88b4 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.
Also tell xlog.c to treat the metapage like a standard page, so any
hole in it is compressed.
---
src/backend/access/gin/gininsert.c | 4 ++--
src/backend/access/gin/ginutil.c | 9 +++++++++
src/backend/access/gin/ginxlog.c | 24 +++++++++---------------
3 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/src/backend/access/gin/gininsert.c
b/src/backend/access/gin/gininsert.c
index 5378011f50..c9aa4ee147 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -348,7 +348,7 @@ ginbuild(Relation heap, Relation index, IndexInfo
*indexInfo)
Page page;
XLogBeginInsert();
- XLogRegisterBuffer(0, MetaBuffer, REGBUF_WILL_INIT);
+ XLogRegisterBuffer(0, MetaBuffer, REGBUF_WILL_INIT |
REGBUF_STANDARD);
XLogRegisterBuffer(1, RootBuffer, REGBUF_WILL_INIT);
recptr = XLogInsert(RM_GIN_ID, XLOG_GIN_CREATE_INDEX);
@@ -447,7 +447,7 @@ ginbuildempty(Relation index)
START_CRIT_SECTION();
GinInitMetabuffer(MetaBuffer);
MarkBufferDirty(MetaBuffer);
- log_newpage_buffer(MetaBuffer, false);
+ log_newpage_buffer(MetaBuffer, true);
GinInitBuffer(RootBuffer, GIN_LEAF);
MarkBufferDirty(RootBuffer);
log_newpage_buffer(RootBuffer, false);
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index 136ea27718..e926649dd2 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -374,6 +374,15 @@ 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, as long as the
+ * buffer containing the page is passed to XLogRegisterBuffer() as a
+ * REGBUF_STANDARD page.
+ */
+ ((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..f5c11b2d9a 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);
@@ -768,6 +768,7 @@ void
gin_mask(char *pagedata, BlockNumber blkno)
{
Page page = (Page) pagedata;
+ PageHeader pagehdr = (PageHeader) page;
GinPageOpaque opaque;
mask_page_lsn(page);
@@ -776,18 +777,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 if (pagehdr->pd_lower != 0)
+ mask_unused_space(page);
}
--
2.11.0
From d5357e311be2aa71f842335f8684a7600493044c 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
Also tell xlog.c to treat the metapage like a standard page, so any
hole in it is compressed.
---
src/backend/access/brin/brin.c | 4 ++--
src/backend/access/brin/brin_pageops.c | 9 +++++++++
src/backend/access/brin/brin_xlog.c | 9 +++++++--
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index b3aa6d1ced..e6909d7aea 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -685,7 +685,7 @@ brinbuild(Relation heap, Relation index, IndexInfo
*indexInfo)
XLogBeginInsert();
XLogRegisterData((char *) &xlrec, SizeOfBrinCreateIdx);
- XLogRegisterBuffer(0, meta, REGBUF_WILL_INIT);
+ XLogRegisterBuffer(0, meta, REGBUF_WILL_INIT | REGBUF_STANDARD);
recptr = XLogInsert(RM_BRIN_ID, XLOG_BRIN_CREATE_INDEX);
@@ -742,7 +742,7 @@ brinbuildempty(Relation index)
brin_metapage_init(BufferGetPage(metabuf), BrinGetPagesPerRange(index),
BRIN_CURRENT_VERSION);
MarkBufferDirty(metabuf);
- log_newpage_buffer(metabuf, false);
+ log_newpage_buffer(metabuf, true);
END_CRIT_SECTION();
UnlockReleaseBuffer(metabuf);
diff --git a/src/backend/access/brin/brin_pageops.c
b/src/backend/access/brin/brin_pageops.c
index 80f803e438..117322ed17 100644
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -491,6 +491,15 @@ 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 not
essential
+ * but it makes the page look compressible to xlog.c, as long as the
+ * buffer containing the page is passed to XLogRegisterBuffer() as a
+ * REGBUF_STANDARD page.
+ */
+ ((PageHeader) page)->pd_lower =
+ ((char *) metadata + sizeof(BrinMetaPageData)) - (char *) page;
}
/*
diff --git a/src/backend/access/brin/brin_xlog.c
b/src/backend/access/brin/brin_xlog.c
index dff7198a39..1309d44b04 100644
--- a/src/backend/access/brin/brin_xlog.c
+++ b/src/backend/access/brin/brin_xlog.c
@@ -331,14 +331,19 @@ void
brin_mask(char *pagedata, BlockNumber blkno)
{
Page page = (Page) pagedata;
+ PageHeader pagehdr = (PageHeader) page;
mask_page_lsn(page);
mask_page_hint_bits(page);
- if (BRIN_IS_REGULAR_PAGE(page))
+ /*
+ * Regular brin pages contain unused space which needs to be masked.
+ * Similarly for meta pages, but only if pd_lower has been set.
+ */
+ if (BRIN_IS_REGULAR_PAGE(page) ||
+ (BRIN_IS_META_PAGE(page) && pagehdr->pd_lower != 0))
{
- /* Regular brin pages contain unused space which needs to be
masked. */
mask_unused_space(page);
}
}
--
2.11.0
From e6a5b734d61171dee1df39137439cbd48ed56b44 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
Also tell xlog.c to treat the metapage like a standard page, so any
hole in it is compressed.
---
src/backend/access/spgist/spginsert.c | 4 ++--
src/backend/access/spgist/spgutils.c | 9 +++++++++
src/backend/access/spgist/spgxlog.c | 8 +++-----
3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/backend/access/spgist/spginsert.c
b/src/backend/access/spgist/spginsert.c
index e4b2c29b0e..80b82e1602 100644
--- a/src/backend/access/spgist/spginsert.c
+++ b/src/backend/access/spgist/spginsert.c
@@ -110,7 +110,7 @@ spgbuild(Relation heap, Relation index, IndexInfo
*indexInfo)
* Replay will re-initialize the pages, so don't take full pages
* images. No other data to log.
*/
- XLogRegisterBuffer(0, metabuffer, REGBUF_WILL_INIT);
+ XLogRegisterBuffer(0, metabuffer, REGBUF_WILL_INIT |
REGBUF_STANDARD);
XLogRegisterBuffer(1, rootbuffer, REGBUF_WILL_INIT |
REGBUF_STANDARD);
XLogRegisterBuffer(2, nullbuffer, REGBUF_WILL_INIT |
REGBUF_STANDARD);
@@ -173,7 +173,7 @@ spgbuildempty(Relation index)
smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_METAPAGE_BLKNO,
(char *) page, true);
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
- SPGIST_METAPAGE_BLKNO, page, false);
+ SPGIST_METAPAGE_BLKNO, page, true);
/* Likewise for the root page. */
SpGistInitPage(page, SPGIST_LEAF);
diff --git a/src/backend/access/spgist/spgutils.c
b/src/backend/access/spgist/spgutils.c
index 22f64b0103..291a1ae571 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -534,6 +534,15 @@ 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 not
essential
+ * but it makes the page look compressible to xlog.c, as long as the
+ * buffer containing the page is passed to XLogRegisterBuffer() as a
+ * REGBUF_STANDARD page.
+ */
+ ((PageHeader) page)->pd_lower =
+ ((char *) metadata + sizeof(SpGistMetaPageData)) - (char *)
page;
}
/*
diff --git a/src/backend/access/spgist/spgxlog.c
b/src/backend/access/spgist/spgxlog.c
index c440d21715..bf209416f2 100644
--- a/src/backend/access/spgist/spgxlog.c
+++ b/src/backend/access/spgist/spgxlog.c
@@ -1033,15 +1033,13 @@ void
spg_mask(char *pagedata, BlockNumber blkno)
{
Page page = (Page) pagedata;
+ PageHeader pagehdr = (PageHeader) page;
mask_page_lsn(page);
mask_page_hint_bits(page);
- /*
- * Any SpGist page other than meta contains unused space which needs to
be
- * masked.
- */
- if (!SpGistPageIsMeta(page))
+ /* Mask the unused space, provided the page's pd_lower is set. */
+ if (pagehdr->pd_lower != 0)
mask_unused_space(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
