On 2017/06/27 10:22, Michael Paquier wrote:
> On Mon, Jun 26, 2017 at 4:11 PM, Masahiko Sawada <sawada.m...@gmail.com>
> wrote:
>> Thank you for the patches! I checked additional patches for brin and
>> spgist. They look good to me.
>
> Last versions are still missing something: brin_mask() and spg_mask()
> can be updated so as mask_unused_space() is called for meta pages.
> Except that the patches look to be on the right track.
Thanks for the review.
I updated brin_mask() and spg_mask() in the attached updated patches so
that they consider meta pages as containing unused space.
> By the way, as this is an optimization and not an actual bug, could
> you add this patch to the next commit fest? I don't think that this
> should get into 10. The focus is to stabilize things lately.
Sure, done.
Thanks,
Amit
From b05760739a36f3247019e75a9029ece7ab0bb09c 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 3f197e13dcef968e8f0a5ff17cf0c328458cde34 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 +++++++
src/backend/access/brin/brin_xlog.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
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;
}
/*
diff --git a/src/backend/access/brin/brin_xlog.c
b/src/backend/access/brin/brin_xlog.c
index dff7198a39..36e6e1411a 100644
--- a/src/backend/access/brin/brin_xlog.c
+++ b/src/backend/access/brin/brin_xlog.c
@@ -336,7 +336,7 @@ brin_mask(char *pagedata, BlockNumber blkno)
mask_page_hint_bits(page);
- if (BRIN_IS_REGULAR_PAGE(page))
+ if (BRIN_IS_REGULAR_PAGE(page) || BRIN_IS_META_PAGE(page))
{
/* Regular brin pages contain unused space which needs to be
masked. */
mask_unused_space(page);
--
2.11.0
From 32c5f42c17e9fae57ed12c2326470de5e20f964c 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 +++++++
src/backend/access/spgist/spgxlog.c | 7 +------
2 files changed, 8 insertions(+), 6 deletions(-)
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;
}
/*
diff --git a/src/backend/access/spgist/spgxlog.c
b/src/backend/access/spgist/spgxlog.c
index c440d21715..9ea3fd8e8d 100644
--- a/src/backend/access/spgist/spgxlog.c
+++ b/src/backend/access/spgist/spgxlog.c
@@ -1038,10 +1038,5 @@ spg_mask(char *pagedata, BlockNumber blkno)
mask_page_hint_bits(page);
- /*
- * Any SpGist page other than meta contains unused space which needs to
be
- * masked.
- */
- if (!SpGistPageIsMeta(page))
- mask_unused_space(page);
+ 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
