Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Heikki Linnakangas]

On 02/04/2019 08:58, Andrey Lepikhov wrote:

On 25/03/2019 15:21, Heikki Linnakangas wrote:

I had another quick look.

I still think using the "generic xlog AM" for this is a wrong level of
abstraction, and we should use the XLOG_FPI records for this directly.
We can extend XLOG_FPI so that it can store multiple pages in a single
record, if it doesn't already handle it.

Another counter-point to using the generic xlog record is that you're
currently doing unnecessary two memcpy's of all pages in the index, in
GenericXLogRegisterBuffer() and GenericXLogFinish(). That's not free.

I guess the generic_log_relation() function can stay where it is, but it
should use XLogRegisterBuffer() and XLogInsert() directly.


Patch set v.3 uses XLOG_FPI records directly.


Thanks! Committed, with some changes:

* I moved the log_relation() function to xlog.c, so that it sits beside 
log_newpage_*() functions. I renamed it to log_newpage_range(), and 
changed the argument so that the caller provides the beginning and end 
block ranges. I added a 'page_std' flag, instead of just assuming that 
all pages use the standard page layout. All of the callers pass 
page_std=true at the moment, but seems better to be explicit about it.


I made those changes because I felt that the function was too narrowly 
tailored for the current callers. The assumption about standard page 
layout, and also the fact that it only logged the main fork. It's more 
flexible now, for any future AMs that might not be exactly like that. It 
feels like it's at the same level of abstraction now as the other 
log_newpage_*() functions. Even if we never need the flexibility, I 
think making the 'page_std' and 'forknum' arguments explicit is good, to 
draw attention to those details, for anyone calling the function.


* I fixed the REDO code. It was trivially broken, it only restored the 
first page in each FPI WAL record.


* Using "fake" unlogged LSNs for GiST index build seemed fishy. I could 
not convince myself that it was safe in all corner cases. In a recently 
initdb'd cluster, it's theoretically possible that the fake LSN counter 
overtakes the real LSN value, and that could lead to strange behavior. 
For example, how would the buffer manager behave, if there was a dirty 
page in the buffer cache with an LSN value that's greater than the 
current WAL flush pointer? I think you'd get "ERROR: xlog flush request 
%X/%X is not satisfied --- flushed only to %X/%X".


I changed that so that we use LSN 1 for all pages during index build. 
That's smaller than any real or fake LSN. Or actually, the fake LSN 
counter used to start at 1 - I bumped that up to 1000, so now it's 
safely smaller. Could've used 0 instead, but there's an assertion in 
gist scan code that didn't like that.



As a benchmark I use the script (test.sql in attachment) which show WAL
size increment during index build. In the table below you can see the
influence of the patch on WAL growth.

Results
===
AM  | master | patch |
GIN | 347 MB | 66 MB |
GiST| 157 MB | 43 MB |
SP-GiST | 119 MB | 38 MB |


Nice!

- Heikki

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Andrey Lepikhov]

On 25/03/2019 15:21, Heikki Linnakangas wrote:

I had another quick look.

I still think using the "generic xlog AM" for this is a wrong level of 
abstraction, and we should use the XLOG_FPI records for this directly. 
We can extend XLOG_FPI so that it can store multiple pages in a single 
record, if it doesn't already handle it.


Another counter-point to using the generic xlog record is that you're 
currently doing unnecessary two memcpy's of all pages in the index, in 
GenericXLogRegisterBuffer() and GenericXLogFinish(). That's not free.


I guess the generic_log_relation() function can stay where it is, but it 
should use XLogRegisterBuffer() and XLogInsert() directly.


Patch set v.3 uses XLOG_FPI records directly.

As a benchmark I use the script (test.sql in attachment) which show WAL 
size increment during index build. In the table below you can see the 
influence of the patch on WAL growth.


Results
===
AM  | master | patch |
GIN | 347 MB | 66 MB |
GiST| 157 MB | 43 MB |
SP-GiST | 119 MB | 38 MB |

--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company
>From 2edd0ada13b4749487d0f046191ef2bcf8b11ca3 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Tue, 2 Apr 2019 09:42:59 +0500
Subject: [PATCH 1/4] Relation-into-WAL-function

---
 src/backend/access/transam/generic_xlog.c | 63 +++
 src/include/access/generic_xlog.h |  3 ++
 2 files changed, 66 insertions(+)

diff --git a/src/backend/access/transam/generic_xlog.c b/src/backend/access/transam/generic_xlog.c
index 5b00b7275b..0d5025f78a 100644
--- a/src/backend/access/transam/generic_xlog.c
+++ b/src/backend/access/transam/generic_xlog.c
@@ -16,6 +16,7 @@
 #include "access/bufmask.h"
 #include "access/generic_xlog.h"
 #include "access/xlogutils.h"
+#include "catalog/pg_control.h"
 #include "miscadmin.h"
 #include "utils/memutils.h"
 
@@ -542,3 +543,65 @@ generic_mask(char *page, BlockNumber blkno)
 
 	mask_unused_space(page);
 }
+
+/*
+ * Function for WAL-logging all pages of a relation.
+ * Caller is responsible for locking the relation exclusively.
+ */
+void
+log_relation(Relation rel)
+{
+	BlockNumber 		blkno = 0;
+	BlockNumber 		nblocks;
+	Bufferbufpack[XLR_MAX_BLOCK_ID];
+
+	CHECK_FOR_INTERRUPTS();
+
+	/*
+	 * Iterate over all index pages and WAL-logging it. Pages are grouping into
+	 * the packages before adding to a WAL-record. Zero pages are not logged.
+	 */
+	nblocks = RelationGetNumberOfBlocks(rel);
+	while (blkno < nblocks)
+	{
+		XLogRecPtr recptr;
+		int8 nbufs = 0;
+		int8 i;
+
+		/*
+		 * Assemble package of relation blocks. Try to combine the maximum
+		 * possible number of blocks in one record.
+		 */
+		while (nbufs < XLR_MAX_BLOCK_ID && blkno < nblocks)
+		{
+			Buffer buf = ReadBuffer(rel, blkno);
+
+			if (!PageIsNew(BufferGetPage(buf)))
+bufpack[nbufs++] = buf;
+			else
+ReleaseBuffer(buf);
+			blkno++;
+		}
+
+		XLogBeginInsert();
+		XLogEnsureRecordSpace(nbufs, 0);
+
+		START_CRIT_SECTION();
+		for (i = 0; i < nbufs; i++)
+		{
+			LockBuffer(bufpack[i], BUFFER_LOCK_EXCLUSIVE);
+			XLogRegisterBuffer(i, bufpack[i], REGBUF_FORCE_IMAGE | REGBUF_STANDARD);
+		}
+
+		recptr = XLogInsert(RM_XLOG_ID, XLOG_FPI);
+
+		for (i = 0; i < nbufs; i++)
+		{
+			Page page = BufferGetPage(bufpack[i]);
+			PageSetLSN(page, recptr);
+			MarkBufferDirty(bufpack[i]);
+			UnlockReleaseBuffer(bufpack[i]);
+		}
+		END_CRIT_SECTION();
+	}
+}
diff --git a/src/include/access/generic_xlog.h b/src/include/access/generic_xlog.h
index cb5b5b713a..8abfa486c7 100644
--- a/src/include/access/generic_xlog.h
+++ b/src/include/access/generic_xlog.h
@@ -42,4 +42,7 @@ extern const char *generic_identify(uint8 info);
 extern void generic_desc(StringInfo buf, XLogReaderState *record);
 extern void generic_mask(char *pagedata, BlockNumber blkno);
 
+/* other utils */
+extern void log_relation(Relation rel);
+
 #endif			/* GENERIC_XLOG_H */
-- 
2.17.1

>From 8857657efa8f3347010d3b251315f800dd03bf8d Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Tue, 2 Apr 2019 09:43:20 +0500
Subject: [PATCH 2/4] GIN-Optimal-WAL-Usage

---
 src/backend/access/gin/ginbtree.c |  6 ++---
 src/backend/access/gin/gindatapage.c  |  9 
 src/backend/access/gin/ginentrypage.c |  2 +-
 src/backend/access/gin/gininsert.c| 30 ++--
 src/backend/access/gin/ginutil.c  |  4 ++--
 src/backend/access/gin/ginvacuum.c|  2 +-
 src/backend/access/gin/ginxlog.c  | 33 ---
 src/backend/access/rmgrdesc/gindesc.c |  6 -
 src/include/access/gin.h  |  3 ++-
 src/include/access/ginxlog.h  |  2 --
 10 files changed, 26 insertions(+), 71 deletions(-)

diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c
index 533949e46a..9f82eef8c3 100644
--- a/src/backend/access/gin/ginbtree.c
+++ b/src/backend/access/gin/ginbtree.c
@@ -396,7 +396,7 @@ ginPlaceToPage(GinBtree btree, 

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Andrey Lepikhov]

On 26/03/2019 15:59, Heikki Linnakangas wrote:

On 26/03/2019 11:29, Andrey Lepikhov wrote:

On 25/03/2019 15:21, Heikki Linnakangas wrote:

Hmm. When do we create all-zero pages during index build? That seems
pretty surprising.


GIST uses buffered pages. During GIST build it is possible (very rarely)
what no one index tuple was written to the block page before new block
was allocated. And the page has become an all-zero page.
You can't have problems in the current GIST code, because it writes into
the WAL only changed pages.


Looking at the code, I don't see how that could happen. The only place 
where the GiST index file is extended is in gistNewBuffer(), and all 
callers of that initialize the page immediately after the call. What am 
I missing?

Sorry, This issue was found in SP-GiST AM. You can show it:
1. Apply v2 version of the patch set (see attachment).
2. In the generic_log_relation() routine set logging on PageIsNew(buf)
3. Run script t1.sql (in attachment).

This problem can be resolved by calling MarkBufferDirty() after 
SpGistInitBuffer() in the allocNewBuffer() routine. But in this case we 
will write to the WAL more pages than necessary.
To avoid it in the patch '0001-Relation-into-WAL-function' I do not 
write new pages to the WAL.


Attached patch set is not final version. It is needed for demonstration 
of 'all-zero pages' issue only. The sentence for the direct use of 
XLOG_FPI records will be considered in v3.


--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company


t1.sql
Description: application/sql
>From d3093aa9a7628979b892d31449eda6228ef169ce Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Mon, 1 Apr 2019 08:33:46 +0500
Subject: [PATCH 1/4] Relation-into-WAL-function

---
 src/backend/access/transam/generic_xlog.c | 48 +++
 src/include/access/generic_xlog.h |  3 ++
 2 files changed, 51 insertions(+)

diff --git a/src/backend/access/transam/generic_xlog.c b/src/backend/access/transam/generic_xlog.c
index 5b00b7275b..c22e361747 100644
--- a/src/backend/access/transam/generic_xlog.c
+++ b/src/backend/access/transam/generic_xlog.c
@@ -542,3 +542,51 @@ generic_mask(char *page, BlockNumber blkno)
 
 	mask_unused_space(page);
 }
+
+/*
+ * Function to write generic xlog for every existing block of a relation.
+ * Caller is responsible for locking the relation exclusively.
+ */
+void
+generic_log_relation(Relation rel)
+{
+	BlockNumber 		blkno;
+	BlockNumber 		nblocks;
+	int	npbuf = 0;
+	GenericXLogState	*state = NULL;
+	Bufferbufpack[MAX_GENERIC_XLOG_PAGES];
+
+	CHECK_FOR_INTERRUPTS();
+	nblocks = RelationGetNumberOfBlocks(rel);
+
+	/*
+	 * Iterate over all index pages and WAL-logging it. Pages are grouping into
+	 * the packages before adding to a WAL-record. Zero-pages are
+	 * not logged.
+	 */
+	for (blkno = 0; blkno < nblocks; blkno++)
+	{
+		Buffer	buf;
+
+		buf = ReadBuffer(rel, blkno);
+		if (!PageIsNew(BufferGetPage(buf)))
+		{
+			if (npbuf == 0)
+state = GenericXLogStart(rel);
+
+			LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
+			GenericXLogRegisterBuffer(state, buf, GENERIC_XLOG_FULL_IMAGE);
+			bufpack[npbuf++] = buf;
+		}
+		else
+			ReleaseBuffer(buf);
+
+		if ((npbuf == MAX_GENERIC_XLOG_PAGES) || (blkno == nblocks-1))
+		{
+			GenericXLogFinish(state);
+
+			for (; npbuf > 0; npbuf--)
+UnlockReleaseBuffer(bufpack[npbuf-1]);
+		}
+	}
+}
diff --git a/src/include/access/generic_xlog.h b/src/include/access/generic_xlog.h
index cb5b5b713a..e3bbf014cc 100644
--- a/src/include/access/generic_xlog.h
+++ b/src/include/access/generic_xlog.h
@@ -42,4 +42,7 @@ extern const char *generic_identify(uint8 info);
 extern void generic_desc(StringInfo buf, XLogReaderState *record);
 extern void generic_mask(char *pagedata, BlockNumber blkno);
 
+/* other utils */
+extern void generic_log_relation(Relation rel);
+
 #endif			/* GENERIC_XLOG_H */
-- 
2.17.1

>From 9a0172346c8a942b6a493aca8c47452256a2932f Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Mon, 1 Apr 2019 08:37:32 +0500
Subject: [PATCH 2/4] GIN-Optimal-WAL-Usage

---
 src/backend/access/gin/ginbtree.c |  6 ++---
 src/backend/access/gin/gindatapage.c  |  9 
 src/backend/access/gin/ginentrypage.c |  2 +-
 src/backend/access/gin/gininsert.c| 30 ++--
 src/backend/access/gin/ginutil.c  |  4 ++--
 src/backend/access/gin/ginvacuum.c|  2 +-
 src/backend/access/gin/ginxlog.c  | 33 ---
 src/backend/access/rmgrdesc/gindesc.c |  6 -
 src/include/access/gin.h  |  3 ++-
 src/include/access/ginxlog.h  |  2 --
 10 files changed, 26 insertions(+), 71 deletions(-)

diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c
index 533949e46a..9f82eef8c3 100644
--- a/src/backend/access/gin/ginbtree.c
+++ b/src/backend/access/gin/ginbtree.c
@@ -396,7 +396,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 		/* It will 

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Heikki Linnakangas]

On 26/03/2019 11:29, Andrey Lepikhov wrote:

On 25/03/2019 15:21, Heikki Linnakangas wrote:

Hmm. When do we create all-zero pages during index build? That seems
pretty surprising.


GIST uses buffered pages. During GIST build it is possible (very rarely)
what no one index tuple was written to the block page before new block
was allocated. And the page has become an all-zero page.
You can't have problems in the current GIST code, because it writes into
the WAL only changed pages.


Looking at the code, I don't see how that could happen. The only place 
where the GiST index file is extended is in gistNewBuffer(), and all 
callers of that initialize the page immediately after the call. What am 
I missing?


- Heikki

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Andrey Lepikhov]

On 25/03/2019 15:21, Heikki Linnakangas wrote:

On 25/03/2019 09:57, David Steele wrote:

On 2/6/19 2:08 PM, Andrey Lepikhov wrote:

The patchset had a problem with all-zero pages, has appeared at index
build stage: the generic_log_relation() routine sends all pages into the
WAL. So  lsn field at all-zero page was initialized and the
PageIsVerified() routine detects it as a bad page.
The solution may be:
1. To improve index build algorithms and eliminate the possibility of
not used pages appearing.
2. To mark each page as 'dirty' right after initialization. In this case
we will got 'empty' page instead of the all-zeroed.
3. Do not write into the WAL all-zero pages.


Hmm. When do we create all-zero pages during index build? That seems 
pretty surprising.
GIST uses buffered pages. During GIST build it is possible (very rarely) 
what no one index tuple was written to the block page before new block 
was allocated. And the page has become an all-zero page.
You can't have problems in the current GIST code, because it writes into 
the WAL only changed pages.
But the idea of the patch is traversing blocks of index relation 
one-by-one after end of index building process and write this blocks to 
the WAL. In this case we will see all-zeroed pages and need to check it.



On 04.02.2019 10:04, Michael Paquier wrote:

On Tue, Dec 18, 2018 at 10:41:48AM +0500, Andrey Lepikhov wrote:

Ok. It is used only for demonstration.


The latest patch set needs a rebase, so moved to next CF, waiting on
author as this got no reviews.


The patch no longer applies so marked Waiting on Author.

Alexander, Heikki, are either of you planning to review the patch in
this CF?


I had another quick look.

I still think using the "generic xlog AM" for this is a wrong level of 
abstraction, and we should use the XLOG_FPI records for this directly. 
We can extend XLOG_FPI so that it can store multiple pages in a single 
record, if it doesn't already handle it.


Another counter-point to using the generic xlog record is that you're 
currently doing unnecessary two memcpy's of all pages in the index, in 
GenericXLogRegisterBuffer() and GenericXLogFinish(). That's not free.


I guess the generic_log_relation() function can stay where it is, but it 
should use XLogRegisterBuffer() and XLogInsert() directly.
Ok. This patch waited feedback for a long time. I will check the GIST 
code changes from previous review and will try to use your advice.


- Heikki


--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Heikki Linnakangas]

On 25/03/2019 09:57, David Steele wrote:

On 2/6/19 2:08 PM, Andrey Lepikhov wrote:

The patchset had a problem with all-zero pages, has appeared at index
build stage: the generic_log_relation() routine sends all pages into the
WAL. So  lsn field at all-zero page was initialized and the
PageIsVerified() routine detects it as a bad page.
The solution may be:
1. To improve index build algorithms and eliminate the possibility of
not used pages appearing.
2. To mark each page as 'dirty' right after initialization. In this case
we will got 'empty' page instead of the all-zeroed.
3. Do not write into the WAL all-zero pages.


Hmm. When do we create all-zero pages during index build? That seems 
pretty surprising.



On 04.02.2019 10:04, Michael Paquier wrote:

On Tue, Dec 18, 2018 at 10:41:48AM +0500, Andrey Lepikhov wrote:

Ok. It is used only for demonstration.


The latest patch set needs a rebase, so moved to next CF, waiting on
author as this got no reviews.


The patch no longer applies so marked Waiting on Author.

Alexander, Heikki, are either of you planning to review the patch in
this CF?


I had another quick look.

I still think using the "generic xlog AM" for this is a wrong level of 
abstraction, and we should use the XLOG_FPI records for this directly. 
We can extend XLOG_FPI so that it can store multiple pages in a single 
record, if it doesn't already handle it.


Another counter-point to using the generic xlog record is that you're 
currently doing unnecessary two memcpy's of all pages in the index, in 
GenericXLogRegisterBuffer() and GenericXLogFinish(). That's not free.


I guess the generic_log_relation() function can stay where it is, but it 
should use XLogRegisterBuffer() and XLogInsert() directly.


- Heikki

Re: Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[David Steele]

On 2/6/19 2:08 PM, Andrey Lepikhov wrote:

The patchset had a problem with all-zero pages, has appeared at index
build stage: the generic_log_relation() routine sends all pages into the
WAL. So  lsn field at all-zero page was initialized and the
PageIsVerified() routine detects it as a bad page.
The solution may be:
1. To improve index build algorithms and eliminate the possibility of
not used pages appearing.
2. To mark each page as 'dirty' right after initialization. In this case
we will got 'empty' page instead of the all-zeroed.
3. Do not write into the WAL all-zero pages.

In the patchset (see attachment) I used approach No.3.

On 04.02.2019 10:04, Michael Paquier wrote:

On Tue, Dec 18, 2018 at 10:41:48AM +0500, Andrey Lepikhov wrote:

Ok. It is used only for demonstration.


The latest patch set needs a rebase, so moved to next CF, waiting on
author as this got no reviews.


The patch no longer applies so marked Waiting on Author.

Alexander, Heikki, are either of you planning to review the patch in 
this CF?


Regards,
--
-David
da...@pgmasters.net

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Andrey Lepikhov]

The patchset had a problem with all-zero pages, has appeared at index
build stage: the generic_log_relation() routine sends all pages into the
WAL. So  lsn field at all-zero page was initialized and the
PageIsVerified() routine detects it as a bad page.
The solution may be:
1. To improve index build algorithms and eliminate the possibility of
not used pages appearing.
2. To mark each page as 'dirty' right after initialization. In this case
we will got 'empty' page instead of the all-zeroed.
3. Do not write into the WAL all-zero pages.

In the patchset (see attachment) I used approach No.3.

On 04.02.2019 10:04, Michael Paquier wrote:
> On Tue, Dec 18, 2018 at 10:41:48AM +0500, Andrey Lepikhov wrote:
>> Ok. It is used only for demonstration.
> 
> The latest patch set needs a rebase, so moved to next CF, waiting on
> author as this got no reviews.
> --
> Michael
> 

-- 
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company
From ec20e8896181cf8b26755acaa6028a62a0c709e7 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Wed, 6 Feb 2019 14:39:59 +0500
Subject: [PATCH 1/4] Relation-into-WAL-function

---
 src/backend/access/transam/generic_xlog.c | 48 +++
 src/include/access/generic_xlog.h |  3 ++
 2 files changed, 51 insertions(+)

diff --git a/src/backend/access/transam/generic_xlog.c b/src/backend/access/transam/generic_xlog.c
index 5b00b7275b..c22e361747 100644
--- a/src/backend/access/transam/generic_xlog.c
+++ b/src/backend/access/transam/generic_xlog.c
@@ -542,3 +542,51 @@ generic_mask(char *page, BlockNumber blkno)
 
 	mask_unused_space(page);
 }
+
+/*
+ * Function to write generic xlog for every existing block of a relation.
+ * Caller is responsible for locking the relation exclusively.
+ */
+void
+generic_log_relation(Relation rel)
+{
+	BlockNumber 		blkno;
+	BlockNumber 		nblocks;
+	int	npbuf = 0;
+	GenericXLogState	*state = NULL;
+	Bufferbufpack[MAX_GENERIC_XLOG_PAGES];
+
+	CHECK_FOR_INTERRUPTS();
+	nblocks = RelationGetNumberOfBlocks(rel);
+
+	/*
+	 * Iterate over all index pages and WAL-logging it. Pages are grouping into
+	 * the packages before adding to a WAL-record. Zero-pages are
+	 * not logged.
+	 */
+	for (blkno = 0; blkno < nblocks; blkno++)
+	{
+		Buffer	buf;
+
+		buf = ReadBuffer(rel, blkno);
+		if (!PageIsNew(BufferGetPage(buf)))
+		{
+			if (npbuf == 0)
+state = GenericXLogStart(rel);
+
+			LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
+			GenericXLogRegisterBuffer(state, buf, GENERIC_XLOG_FULL_IMAGE);
+			bufpack[npbuf++] = buf;
+		}
+		else
+			ReleaseBuffer(buf);
+
+		if ((npbuf == MAX_GENERIC_XLOG_PAGES) || (blkno == nblocks-1))
+		{
+			GenericXLogFinish(state);
+
+			for (; npbuf > 0; npbuf--)
+UnlockReleaseBuffer(bufpack[npbuf-1]);
+		}
+	}
+}
diff --git a/src/include/access/generic_xlog.h b/src/include/access/generic_xlog.h
index cb5b5b713a..e3bbf014cc 100644
--- a/src/include/access/generic_xlog.h
+++ b/src/include/access/generic_xlog.h
@@ -42,4 +42,7 @@ extern const char *generic_identify(uint8 info);
 extern void generic_desc(StringInfo buf, XLogReaderState *record);
 extern void generic_mask(char *pagedata, BlockNumber blkno);
 
+/* other utils */
+extern void generic_log_relation(Relation rel);
+
 #endif			/* GENERIC_XLOG_H */
-- 
2.17.1

From 6fa828f6737d9c2dbcd2c2ce61a150e79e2bc1d2 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Wed, 6 Feb 2019 14:40:29 +0500
Subject: [PATCH 2/4] GIN-Optimal-WAL-Usage

---
 src/backend/access/gin/ginbtree.c |  6 ++---
 src/backend/access/gin/gindatapage.c  |  9 
 src/backend/access/gin/ginentrypage.c |  2 +-
 src/backend/access/gin/gininsert.c| 30 ++--
 src/backend/access/gin/ginutil.c  |  4 ++--
 src/backend/access/gin/ginvacuum.c|  2 +-
 src/backend/access/gin/ginxlog.c  | 33 ---
 src/backend/access/rmgrdesc/gindesc.c |  6 -
 src/include/access/gin.h  |  3 ++-
 src/include/access/ginxlog.h  |  2 --
 10 files changed, 26 insertions(+), 71 deletions(-)

diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c
index 533949e46a..9f82eef8c3 100644
--- a/src/backend/access/gin/ginbtree.c
+++ b/src/backend/access/gin/ginbtree.c
@@ -396,7 +396,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 		/* It will fit, perform the insertion */
 		START_CRIT_SECTION();
 
-		if (RelationNeedsWAL(btree->index))
+		if (RelationNeedsWAL(btree->index) && !btree->isBuild)
 		{
 			XLogBeginInsert();
 			XLogRegisterBuffer(0, stack->buffer, REGBUF_STANDARD);
@@ -417,7 +417,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 			MarkBufferDirty(childbuf);
 		}
 
-		if (RelationNeedsWAL(btree->index))
+		if (RelationNeedsWAL(btree->index) && !btree->isBuild)
 		{
 			XLogRecPtr	recptr;
 			ginxlogInsert xlrec;
@@ -595,7 +595,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 		}
 
 		/* write WAL record */
-		if 

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Andrey Lepikhov]

On 04.02.2019 10:04, Michael Paquier wrote:
> On Tue, Dec 18, 2018 at 10:41:48AM +0500, Andrey Lepikhov wrote:
>> Ok. It is used only for demonstration.
> 
> The latest patch set needs a rebase, so moved to next CF, waiting on
> author as this got no reviews.
The new version in attachment.
> --
> Michael
> 

-- 
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company
From afc270f37cd082fb6e9f4ad694ea4cb123d98062 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Tue, 5 Feb 2019 08:11:47 +0500
Subject: [PATCH 1/4] Relation-into-WAL-function

---
 src/backend/access/spgist/spgutils.c  |  1 +
 src/backend/access/transam/generic_xlog.c | 39 +++
 src/include/access/generic_xlog.h |  3 ++
 3 files changed, 43 insertions(+)

diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index 8e63c1fad2..b782bc2338 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -550,6 +550,7 @@ SpGistInitBuffer(Buffer b, uint16 f)
 {
 	Assert(BufferGetPageSize(b) == BLCKSZ);
 	SpGistInitPage(BufferGetPage(b), f);
+	MarkBufferDirty(b);
 }
 
 /*
diff --git a/src/backend/access/transam/generic_xlog.c b/src/backend/access/transam/generic_xlog.c
index 5b00b7275b..643da48345 100644
--- a/src/backend/access/transam/generic_xlog.c
+++ b/src/backend/access/transam/generic_xlog.c
@@ -542,3 +542,42 @@ generic_mask(char *page, BlockNumber blkno)
 
 	mask_unused_space(page);
 }
+
+/*
+ * Function to write generic xlog for every existing block of a relation.
+ * Caller is responsible for locking the relation exclusively.
+ */
+void
+generic_log_relation(Relation rel)
+{
+	BlockNumber blkno;
+	BlockNumber nblocks = RelationGetNumberOfBlocks(rel);
+
+	for (blkno = 0; blkno < nblocks; )
+	{
+		GenericXLogState	*state;
+		Bufferbuffer[MAX_GENERIC_XLOG_PAGES];
+		int	i,
+			blocks_pack;
+
+		CHECK_FOR_INTERRUPTS();
+
+		blocks_pack = ((nblocks-blkno) < MAX_GENERIC_XLOG_PAGES) ?
+		(nblocks-blkno) : MAX_GENERIC_XLOG_PAGES;
+
+		state = GenericXLogStart(rel);
+
+		for (i = 0 ; i < blocks_pack; i++)
+		{
+			buffer[i] = ReadBuffer(rel, blkno);
+			LockBuffer(buffer[i], BUFFER_LOCK_EXCLUSIVE);
+			GenericXLogRegisterBuffer(state, buffer[i], GENERIC_XLOG_FULL_IMAGE);
+			blkno++;
+		}
+
+		GenericXLogFinish(state);
+
+		for (i = 0 ; i < blocks_pack; i++)
+			UnlockReleaseBuffer(buffer[i]);
+	}
+}
diff --git a/src/include/access/generic_xlog.h b/src/include/access/generic_xlog.h
index cb5b5b713a..e3bbf014cc 100644
--- a/src/include/access/generic_xlog.h
+++ b/src/include/access/generic_xlog.h
@@ -42,4 +42,7 @@ extern const char *generic_identify(uint8 info);
 extern void generic_desc(StringInfo buf, XLogReaderState *record);
 extern void generic_mask(char *pagedata, BlockNumber blkno);
 
+/* other utils */
+extern void generic_log_relation(Relation rel);
+
 #endif			/* GENERIC_XLOG_H */
-- 
2.17.1

From 25dc935da9d5502a06fb9dc1eea4912fa0f48be1 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Tue, 5 Feb 2019 08:13:29 +0500
Subject: [PATCH 2/4] GIN-Optimal-WAL-Usage

---
 src/backend/access/gin/ginbtree.c |  6 ++---
 src/backend/access/gin/gindatapage.c  | 10 
 src/backend/access/gin/ginentrypage.c |  2 +-
 src/backend/access/gin/gininsert.c| 30 ++--
 src/backend/access/gin/ginutil.c  |  4 ++--
 src/backend/access/gin/ginvacuum.c|  2 +-
 src/backend/access/gin/ginxlog.c  | 33 ---
 src/backend/access/rmgrdesc/gindesc.c |  6 -
 src/include/access/gin.h  |  3 ++-
 src/include/access/ginxlog.h  |  2 --
 10 files changed, 27 insertions(+), 71 deletions(-)

diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c
index 533949e46a..9f82eef8c3 100644
--- a/src/backend/access/gin/ginbtree.c
+++ b/src/backend/access/gin/ginbtree.c
@@ -396,7 +396,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 		/* It will fit, perform the insertion */
 		START_CRIT_SECTION();
 
-		if (RelationNeedsWAL(btree->index))
+		if (RelationNeedsWAL(btree->index) && !btree->isBuild)
 		{
 			XLogBeginInsert();
 			XLogRegisterBuffer(0, stack->buffer, REGBUF_STANDARD);
@@ -417,7 +417,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 			MarkBufferDirty(childbuf);
 		}
 
-		if (RelationNeedsWAL(btree->index))
+		if (RelationNeedsWAL(btree->index) && !btree->isBuild)
 		{
 			XLogRecPtr	recptr;
 			ginxlogInsert xlrec;
@@ -595,7 +595,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 		}
 
 		/* write WAL record */
-		if (RelationNeedsWAL(btree->index))
+		if (RelationNeedsWAL(btree->index) && !btree->isBuild)
 		{
 			XLogRecPtr	recptr;
 
diff --git a/src/backend/access/gin/gindatapage.c b/src/backend/access/gin/gindatapage.c
index 3ad8b76710..a73cf944b3 100644
--- a/src/backend/access/gin/gindatapage.c
+++ b/src/backend/access/gin/gindatapage.c
@@ -593,7 +593,7 @@ 

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Michael Paquier]

On Tue, Dec 18, 2018 at 10:41:48AM +0500, Andrey Lepikhov wrote:
> Ok. It is used only for demonstration.

The latest patch set needs a rebase, so moved to next CF, waiting on
author as this got no reviews.
--
Michael


signature.asc
Description: PGP signature

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Andrey Lepikhov]

On 30.11.2018 15:10, Dmitry Dolgov wrote:

Thank you. I played a bit with this patch, and can confirm visible WAL size
reduction (it's rather obvious, but still). Although, probably due to lack of
my knowledge here, I wonder how does it work when an index is build
concurrently?


Routine generate_xlog_for_rel() works only at phase 2 of concurrent 
index building.
At the phase 3, during validate_index() execution we use aminsert() -> 
PlaceToPage() mechanism as before the patch.

In the concurrent mode, I do not expect any problems, caused by the patch.




Benchmarks:
---

Test: pgbench -f gin-WAL-test.sql -t 5:
---
master:
Latency average: 27696.299 ms
WAL size: 2.66 GB


Does it makes sense to measure latency of the entire script, since it contains
also some preparation work? Of course it still shows the difference between
patched version and master, but probably in a more noisy way.


Ok. It is used only for demonstration.

--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Dmitry Dolgov]

> On Tue, Jul 31, 2018 at 6:36 AM Andrey Lepikhov  
> wrote:
>
> With the consent of Anastasia I will improving this patch further.
> Attachment contains next version of the patch set.

Thank you. I played a bit with this patch, and can confirm visible WAL size
reduction (it's rather obvious, but still). Although, probably due to lack of
my knowledge here, I wonder how does it work when an index is build
concurrently?

> Benchmarks:
> ---
>
> Test: pgbench -f gin-WAL-test.sql -t 5:
> ---
> master:
> Latency average: 27696.299 ms
> WAL size: 2.66 GB

Does it makes sense to measure latency of the entire script, since it contains
also some preparation work? Of course it still shows the difference between
patched version and master, but probably in a more noisy way.

I'm moving this patch to the next CF.

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Andrey Lepikhov]

With the consent of Anastasia I will improving this patch further.
Attachment contains next version of the patch set.

11.07.2018 00:03, Heikki Linnakangas пишет:

On 28/02/18 18:03, Anastasia Lubennikova wrote:



Implementation is based on generic_xlog.


Why? I think we should just add a log_relation() function in 
xloginsert.c directly, alongside log_newpage_buffer().



I have some arguments to stay this functionality at generic_xlog module:
1. xloginsert.c functions work on low level of abstraction, use buffers 
and pages.
2. Code size using generic_xlog service functions looks more compact and 
safe.
This makes the assumption that all the pages in these indexes used the 
standard page layout. I think that's a valid assumption, but needs at 
least a comment on that. And perhaps an Assert, to check that 
pd_lower/upper look sane.

Done


As a further optimization, would it be a win to WAL-log multiple pages 
in each record?
In this version of the patch we use simple optimization: pack 
XLR_NORMAL_MAX_BLOCK_ID blocks pieces into each WAL-record.


This leaves the XLOG_*_CREATE_INDEX WAL record types unused, BTW.


Done

- Heikki



Benchmarks:
---

Test: pgbench -f gin-WAL-test.sql -t 5:
---
master:
Latency average: 27696.299 ms
WAL size: 2.66 GB

patched
Latency average: 22812.103 ms
WAL size: 1.23 GB


Test: pgbench -f gist-WAL-test.sql -t 5:

master:
Latency average: 19928.284 ms
WAL size: 1.25 GB

patched
Latency average: 18175.064 ms
WAL size: 0.63 GB


Test: pgbench -f spgist-WAL-test.sql -t 5:
--
master:
Latency average: 11529.384 ms
WAL size: 1.07 GB

patched
Latency average: 9330.828 ms
WAL size: 0.6 GB

--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company
>From db400ce9532536da36812dbf0456e756a0ea4724 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Tue, 31 Jul 2018 07:22:17 +0500
Subject: [PATCH 1/4] Relation-into-WAL-function

---
 src/backend/access/transam/generic_xlog.c | 62 +++
 src/include/access/generic_xlog.h |  3 ++
 2 files changed, 65 insertions(+)

diff --git a/src/backend/access/transam/generic_xlog.c b/src/backend/access/transam/generic_xlog.c
index ce023548ae..8397b58ee7 100644
--- a/src/backend/access/transam/generic_xlog.c
+++ b/src/backend/access/transam/generic_xlog.c
@@ -80,6 +80,7 @@ static void computeRegionDelta(PageData *pageData,
 static void computeDelta(PageData *pageData, Page curpage, Page targetpage);
 static void applyPageRedo(Page page, const char *delta, Size deltaSize);
 
+static void standard_page_layout_check(Buffer buf);
 
 /*
  * Write next fragment into pageData's delta.
@@ -545,3 +546,64 @@ generic_mask(char *page, BlockNumber blkno)
 
 	mask_unused_space(page);
 }
+
+/*
+ * Check page layout.
+ * Caller must lock the buffer
+ */
+static void
+standard_page_layout_check(Buffer buf)
+{
+	PageHeader ph = (PageHeader) BufferGetPage(buf);
+
+	Assert((ph->pd_lower >= SizeOfPageHeaderData) &&
+		   (ph->pd_lower <= ph->pd_upper) &&
+		   (ph->pd_upper <= ph->pd_special) &&
+		   (ph->pd_special <= BLCKSZ) &&
+		   (ph->pd_special == MAXALIGN(ph->pd_special)));
+}
+
+/*
+ * Function to write generic xlog for every existing block of a relation.
+ * Caller is responsible for locking the relation exclusively.
+ */
+void
+generic_log_relation(Relation rel)
+{
+	BlockNumber blkno;
+	BlockNumber nblocks;
+
+	nblocks = RelationGetNumberOfBlocks(rel);
+
+	elog(DEBUG2, "generic_log_relation '%s', nblocks %u BEGIN.",
+		 RelationGetRelationName(rel), nblocks);
+
+	for (blkno = 0; blkno < nblocks; )
+	{
+		GenericXLogState	*state;
+		Bufferbuffer[MAX_GENERIC_XLOG_PAGES];
+		int	counter,
+			blocks_pack;
+
+		CHECK_FOR_INTERRUPTS();
+
+		blocks_pack = ((nblocks-blkno) < MAX_GENERIC_XLOG_PAGES) ?
+		(nblocks-blkno) : MAX_GENERIC_XLOG_PAGES;
+
+		state = GenericXLogStart(rel);
+
+		for (counter = 0 ; counter < blocks_pack; counter++)
+		{
+			buffer[counter] = ReadBuffer(rel, blkno++);
+			standard_page_layout_check(buffer[counter]);
+			LockBuffer(buffer[counter], BUFFER_LOCK_EXCLUSIVE);
+			GenericXLogRegisterBuffer(state, buffer[counter], GENERIC_XLOG_FULL_IMAGE);
+		}
+
+		GenericXLogFinish(state);
+
+		for (counter = 0 ; counter < blocks_pack; counter++)
+			UnlockReleaseBuffer(buffer[counter]);
+	}
+	elog(DEBUG2, "generic_log_relation '%s' END.", RelationGetRelationName(rel));
+}
diff --git a/src/include/access/generic_xlog.h b/src/include/access/generic_xlog.h
index b23e1f684b..1f4b3b7030 100644
--- a/src/include/access/generic_xlog.h
+++ b/src/include/access/generic_xlog.h
@@ -42,4 +42,7 @@ extern const char *generic_identify(uint8 info);
 extern void generic_desc(StringInfo buf, XLogReaderState *record);
 extern void generic_mask(char *pagedata, BlockNumber blkno);
 
+/* other utils */
+extern void generic_log_relation(Relation rel);
+
 #endif	

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Alexander Korotkov]

On Tue, Jul 10, 2018 at 10:03 PM Heikki Linnakangas  wrote:
>
> On 28/02/18 18:03, Anastasia Lubennikova wrote:
> > I want to propose a bunch of patches which allow to reduce WAL traffic
> > generated by CREATE INDEX for GiST, GIN and SP-GiST. Similarly to b-tree
> > and RUM, we can now log index pages of other access methods only once
> > in the end of indexbuild process.
>
> Makes sense. Is there any scenario where the current method is better?
> In theory, doing another pass through the whole index, to create the WAL
> records, requires more I/O. But in practice, it seems that the reduction
> in WAL is almost certainly a win.

It's frequently advised to move WAL to the separate storage device.
Then, it's not hard to imagine how current method can be faster, when
storage device containing index is much more loaded than storage
device containing WAL.  But despite being faster it would still
consume way more overall I/O resources.  I'm not sure if we should
provide some option to switch between WAL methods (or heuristics)...

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[David Steele]

Hi Anastasia,

On 2/28/18 11:03 AM, Anastasia Lubennikova wrote:
> I want to propose a bunch of patches which allow to reduce WAL traffic
> generated by CREATE INDEX for GiST, GIN and SP-GiST. Similarly to b-tree
> and RUM, we can now log index pages of other access methods only once
> in the end of indexbuild process. Implementation is based on generic_xlog.
> 
> Not only it decreases the amount of WAL generated, but also completely
> eliminates WAL overhead in case of error during index build.

This seems to be a worthwhile patch, but it was submitted to the 2018-03
CF at the last moment with no prior discussion or review as far as I can
tell. It appears to be non-trivial and therefore not a good fit for the
last CF for PG11.

I have moved it to the 2018-09 CF.

Regards,
-- 
-David
da...@pgmasters.net

Reduce amount of WAL generated by CREATE INDEX for gist, gin and sp-gist

[Anastasia Lubennikova]

Hi,

I want to propose a bunch of patches which allow to reduce WAL traffic
generated by CREATE INDEX for GiST, GIN and SP-GiST. Similarly to b-tree
and RUM, we can now log index pages of other access methods only once
in the end of indexbuild process. Implementation is based on generic_xlog.

Not only it decreases the amount of WAL generated, but also completely
eliminates WAL overhead in case of error during index build.

I also attached sql scripts which I used to measure xlog size.
They show that pg_wal_lsn_diff for patched version is from 3 to 5 times 
smaller.



Not sure if regression tests are needed, since it is just an optimization.
But I do not mind to add them if someone feels that it is necessary.

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

commit 179285fb5175d715c20fc95eca3087b6a1899ed9
Author: Anastasia 
Date:   Wed Feb 28 17:45:54 2018 +0300

add function generate_xlog_for_rel()

diff --git a/src/backend/access/transam/generic_xlog.c b/src/backend/access/transam/generic_xlog.c
index ce02354..dd2c041 100644
--- a/src/backend/access/transam/generic_xlog.c
+++ b/src/backend/access/transam/generic_xlog.c
@@ -545,3 +545,34 @@ generic_mask(char *page, BlockNumber blkno)
 
 	mask_unused_space(page);
 }
+
+/*
+ * Function to write generic xlog for every existing block of a relation.
+ * Caller is responsible for locking the relation exclusively.
+ */
+void
+generate_xlog_for_rel(Relation rel)
+{
+	BlockNumber blkno;
+	BlockNumber nblocks;
+
+	nblocks = RelationGetNumberOfBlocks(rel);
+
+	elog(DEBUG2, "generate_xlog_for_rel '%s', nblocks %u BEGIN.",
+		 RelationGetRelationName(rel), nblocks);
+
+	for (blkno = 0; blkno < nblocks; blkno++)
+	{
+		Buffer	buffer;
+		GenericXLogState *state;
+
+		CHECK_FOR_INTERRUPTS();
+
+		buffer = ReadBuffer(rel, blkno);
+		LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+
+		state = GenericXLogStart(rel);
+		GenericXLogRegisterBuffer(state, buffer, GENERIC_XLOG_FULL_IMAGE);
+		GenericXLogFinish(state);
+
+		UnlockReleaseBuffer(buffer);
+	}
+	elog(DEBUG2, "generate_xlog_for_rel '%s' END.", RelationGetRelationName(rel));
+}
diff --git a/src/include/access/generic_xlog.h b/src/include/access/generic_xlog.h
index b23e1f6..33be157 100644
--- a/src/include/access/generic_xlog.h
+++ b/src/include/access/generic_xlog.h
@@ -42,4 +42,7 @@ extern const char *generic_identify(uint8 info);
 extern void generic_desc(StringInfo buf, XLogReaderState *record);
 extern void generic_mask(char *pagedata, BlockNumber blkno);
 
+/* other utils */
+void generate_xlog_for_rel(Relation rel);
+
 #endif			/* GENERIC_XLOG_H */
commit e176bd8f650a4b112fa2e61960a27cb57329138c
Author: Anastasia 
Date:   Wed Feb 28 17:53:15 2018 +0300

optimal WAL for gin

diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c
index 37070b3..c615d3c 100644
--- a/src/backend/access/gin/ginbtree.c
+++ b/src/backend/access/gin/ginbtree.c
@@ -388,7 +388,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 		/* It will fit, perform the insertion */
 		START_CRIT_SECTION();
 
-		if (RelationNeedsWAL(btree->index))
+		if (RelationNeedsWAL(btree->index) && !btree->isBuild)
 		{
 			XLogBeginInsert();
 			XLogRegisterBuffer(0, stack->buffer, REGBUF_STANDARD);
@@ -409,7 +409,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 			MarkBufferDirty(childbuf);
 		}
 
-		if (RelationNeedsWAL(btree->index))
+		if (RelationNeedsWAL(btree->index) && !btree->isBuild)
 		{
 			XLogRecPtr	recptr;
 			ginxlogInsert xlrec;
@@ -566,7 +566,7 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 		}
 
 		/* write WAL record */
-		if (RelationNeedsWAL(btree->index))
+		if (RelationNeedsWAL(btree->index) && !btree->isBuild)
 		{
 			XLogRecPtr	recptr;
 
diff --git a/src/backend/access/gin/gindatapage.c b/src/backend/access/gin/gindatapage.c
index f9daaba..349baa7 100644
--- a/src/backend/access/gin/gindatapage.c
+++ b/src/backend/access/gin/gindatapage.c
@@ -592,7 +592,7 @@ dataBeginPlaceToPageLeaf(GinBtree btree, Buffer buf, GinBtreeStack *stack,
 		 * Great, all the items fit on a single page.  If needed, prepare data
 		 * for a WAL record describing the changes we'll make.
 		 */
-		if (RelationNeedsWAL(btree->index))
+		if (RelationNeedsWAL(btree->index) && !btree->isBuild)
 			computeLeafRecompressWALData(leaf);
 
 		/*
@@ -629,6 +629,7 @@ dataBeginPlaceToPageLeaf(GinBtree btree, Buffer buf, GinBtreeStack *stack,
 		 * subsequent insertions will probably also go to the end. This packs
 		 * the index somewhat tighter when appending to a table, which is very
 		 * common.
+		 *
 		 */
 		if (!btree->isBuild)
 		{
@@ -718,7 +719,7 @@ dataExecPlaceToPageLeaf(GinBtree btree, Buffer buf, GinBtreeStack *stack,
 	dataPlaceToPageLeafRecompress(buf, leaf);
 
 	/* If needed, register WAL data built by computeLeafRecompressWALData */
-	if (RelationNeedsWAL(btree->index))
+	if