Fix double-XLogBeginInsert call in GIN page splits.

If data checksums or wal_log_hints is on, and a GIN page is split, the code
to find a new, empty, block was called after having already called
XLogBeginInsert(). That causes an assertion failure or PANIC, if finding the
new block involves updating a FSM page that had not been modified since last
checkpoint, because that update is WAL-logged, which calls XLogBeginInsert
again. Nested XLogBeginInsert calls are not supported.

To fix, rearrange GIN code so that XLogBeginInsert is called later, after
finding the victim buffers.

Reported by Jeff Janes.

3 files changed, 11 insertions, 9 deletions

diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c
index 3f92c56d3c0..f0ff91aba2f 100644
--- a/src/backend/access/gin/ginbtree.c
+++ b/src/backend/access/gin/ginbtree.c
@@ -358,20 +358,15 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 	 * placeToPage can register some data to the WAL record.
 	 *
 	 * If placeToPage returns INSERTED, placeToPage has already called
-	 * START_CRIT_SECTION(), and we're responsible for calling
-	 * END_CRIT_SECTION. When it returns INSERTED, it is also responsible for
-	 * registering any data required to replay the operation with
-	 * XLogRegisterData(0, ...). It may only add data to block index 0; the
-	 * main data of the WAL record is reserved for this function.
+	 * START_CRIT_SECTION() and XLogBeginInsert(), and registered any data
+	 * required to replay the operation, in block index 0. We're responsible
+	 * for filling in the main data portion of the WAL record, calling
+	 * XLogInsert(), and END_CRIT_SECTION.
 	 *
 	 * If placeToPage returns SPLIT, we're wholly responsible for WAL logging.
 	 * Splits happen infrequently, so we just make a full-page image of all
 	 * the pages involved.
 	 */
-
-	if (RelationNeedsWAL(btree->index))
-		XLogBeginInsert();
-
 	rc = btree->placeToPage(btree, stack->buffer, stack,
 							insertdata, updateblkno,
 							&newlpage, &newrpage);
@@ -558,6 +553,8 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
 		{
 			XLogRecPtr	recptr;
 
+			XLogBeginInsert();
+
 			/*
 			 * We just take full page images of all the split pages. Splits
 			 * are uncommon enough that it's not worth complicating the code
diff --git a/src/backend/access/gin/gindatapage.c b/src/backend/access/gin/gindatapage.c
index 8e81f6c8687..ec8c94bcbd1 100644
--- a/src/backend/access/gin/gindatapage.c
+++ b/src/backend/access/gin/gindatapage.c
@@ -600,7 +600,10 @@ dataPlaceToPageLeaf(GinBtree btree, Buffer buf, GinBtreeStack *stack,
 		 */
 		MemoryContextSwitchTo(oldCxt);
 		if (RelationNeedsWAL(btree->index))
+		{
+			XLogBeginInsert();
 			registerLeafRecompressWALData(buf, leaf);
+		}
 		START_CRIT_SECTION();
 		dataPlaceToPageLeafRecompress(buf, leaf);
 
@@ -1120,6 +1123,7 @@ dataPlaceToPageInternal(GinBtree btree, Buffer buf, GinBtreeStack *stack,
 		data.offset = off;
 		data.newitem = *pitem;
 
+		XLogBeginInsert();
 		XLogRegisterBuffer(0, buf, REGBUF_STANDARD);
 		XLogRegisterBufData(0, (char *) &data,
 							sizeof(ginxlogInsertDataInternal));
diff --git a/src/backend/access/gin/ginentrypage.c b/src/backend/access/gin/ginentrypage.c
index 9997eae1bcd..c912e60a112 100644
--- a/src/backend/access/gin/ginentrypage.c
+++ b/src/backend/access/gin/ginentrypage.c
@@ -557,6 +557,7 @@ entryPlaceToPage(GinBtree btree, Buffer buf, GinBtreeStack *stack,
 		data.isDelete = insertData->isDelete;
 		data.offset = off;
 
+		XLogBeginInsert();
 		XLogRegisterBuffer(0, buf, REGBUF_STANDARD);
 		XLogRegisterBufData(0, (char *) &data,
 							offsetof(ginxlogInsertEntry, tuple));