v2-0001-Fix-nbtree-metapage-cache-upgrade-bug.patch

From d03e6be539c51d98a6ec0bc90a063edc16b1b202 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Mon, 15 Jul 2019 17:51:03 -0700
Subject: [PATCH v2] Fix nbtree metapage cache upgrade bug.

It's not okay to update a cached copy of an nbtree index's metapage
metadata at a point where it isn't also being updated on disk.  Commit
0a64b45152b seems to have gotten this wrong.

This is almost a straight revert of commit 0a64b45152b, except the
changes to contrib/pageinspect remain, and the failing assertions are
updated to deal with the possibility that an index might be pg_upgrade'd
from an earlier version.

Discussion: https://postgr.es/m/CAH2-Wzmj6pz98qZ6+Ro-=tHvyBJ6q0yxHV8QLOr6O0mE20Nw9Q@mail.gmail.com
---
 src/backend/access/nbtree/nbtpage.c | 70 ++++++++---------------------
 src/include/access/nbtree.h         |  2 +-
 2 files changed, 20 insertions(+), 52 deletions(-)

diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 50455db9af..5cf7a3f6d9 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -33,7 +33,6 @@
 #include "storage/predicate.h"
 #include "utils/snapmgr.h"
 
-static void _bt_cachemetadata(Relation rel, BTMetaPageData *input);
 static BTMetaPageData *_bt_getmeta(Relation rel, Buffer metabuf);
 static bool _bt_mark_page_halfdead(Relation rel, Buffer buf, BTStack stack);
 static bool _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf,
@@ -109,45 +108,6 @@ _bt_upgrademetapage(Page page)
 		((char *) metad + sizeof(BTMetaPageData)) - (char *) page;
 }
 
-/*
- * Cache metadata from input meta page to rel->rd_amcache.
- */
-static void
-_bt_cachemetadata(Relation rel, BTMetaPageData *input)
-{
-	BTMetaPageData *cached_metad;
-
-	/* We assume rel->rd_amcache was already freed by caller */
-	Assert(rel->rd_amcache == NULL);
-	rel->rd_amcache = MemoryContextAlloc(rel->rd_indexcxt,
-										 sizeof(BTMetaPageData));
-
-	/* Meta page should be of supported version */
-	Assert(input->btm_version >= BTREE_MIN_VERSION &&
-		   input->btm_version <= BTREE_VERSION);
-
-	cached_metad = (BTMetaPageData *) rel->rd_amcache;
-	if (input->btm_version >= BTREE_NOVAC_VERSION)
-	{
-		/* Version with compatible meta-data, no need to upgrade */
-		memcpy(cached_metad, input, sizeof(BTMetaPageData));
-	}
-	else
-	{
-		/*
-		 * Upgrade meta-data: copy available information from meta-page and
-		 * fill new fields with default values.
-		 *
-		 * Note that we cannot upgrade to version 4+ without a REINDEX, since
-		 * extensive on-disk changes are required.
-		 */
-		memcpy(cached_metad, input, offsetof(BTMetaPageData, btm_oldest_btpo_xact));
-		cached_metad->btm_version = BTREE_NOVAC_VERSION;
-		cached_metad->btm_oldest_btpo_xact = InvalidTransactionId;
-		cached_metad->btm_last_cleanup_num_heap_tuples = -1.0;
-	}
-}
-
 /*
  * Get metadata from share-locked buffer containing metapage, while performing
  * standard sanity checks.  Sanity checks here must match _bt_getroot().
@@ -483,7 +443,9 @@ _bt_getroot(Relation rel, int access)
 		/*
 		 * Cache the metapage data for next time
 		 */
-		_bt_cachemetadata(rel, metad);
+		rel->rd_amcache = MemoryContextAlloc(rel->rd_indexcxt,
+											 sizeof(BTMetaPageData));
+		memcpy(rel->rd_amcache, metad, sizeof(BTMetaPageData));
 
 		/*
 		 * We are done with the metapage; arrange to release it via first
@@ -650,6 +612,7 @@ _bt_getrootheight(Relation rel)
 		 */
 		if (metad->btm_root == P_NONE)
 		{
+			Assert(metad->btm_version >= BTREE_MIN_VERSION);
 			_bt_relbuf(rel, metabuf);
 			return 0;
 		}
@@ -657,16 +620,18 @@ _bt_getrootheight(Relation rel)
 		/*
 		 * Cache the metapage data for next time
 		 */
-		_bt_cachemetadata(rel, metad);
-		/* We shouldn't have cached it if any of these fail */
-		Assert(metad->btm_magic == BTREE_MAGIC);
-		Assert(metad->btm_version >= BTREE_NOVAC_VERSION);
-		Assert(metad->btm_fastroot != P_NONE);
+		rel->rd_amcache = MemoryContextAlloc(rel->rd_indexcxt,
+											 sizeof(BTMetaPageData));
+		memcpy(rel->rd_amcache, metad, sizeof(BTMetaPageData));
 		_bt_relbuf(rel, metabuf);
 	}
 
 	/* Get cached page */
 	metad = (BTMetaPageData *) rel->rd_amcache;
+	/* We shouldn't have cached it if any of these fail */
+	Assert(metad->btm_magic == BTREE_MAGIC);
+	Assert(metad->btm_version >= BTREE_MIN_VERSION);
+	Assert(metad->btm_fastroot != P_NONE);
 
 	return metad->btm_fastlevel;
 }
@@ -701,6 +666,7 @@ _bt_heapkeyspace(Relation rel)
 		{
 			uint32		btm_version = metad->btm_version;
 
+			Assert(metad->btm_version >= BTREE_MIN_VERSION);
 			_bt_relbuf(rel, metabuf);
 			return btm_version > BTREE_NOVAC_VERSION;
 		}
@@ -708,16 +674,18 @@ _bt_heapkeyspace(Relation rel)
 		/*
 		 * Cache the metapage data for next time
 		 */
-		_bt_cachemetadata(rel, metad);
-		/* We shouldn't have cached it if any of these fail */
-		Assert(metad->btm_magic == BTREE_MAGIC);
-		Assert(metad->btm_version >= BTREE_NOVAC_VERSION);
-		Assert(metad->btm_fastroot != P_NONE);
+		rel->rd_amcache = MemoryContextAlloc(rel->rd_indexcxt,
+											 sizeof(BTMetaPageData));
+		memcpy(rel->rd_amcache, metad, sizeof(BTMetaPageData));
 		_bt_relbuf(rel, metabuf);
 	}
 
 	/* Get cached page */
 	metad = (BTMetaPageData *) rel->rd_amcache;
+	/* We shouldn't have cached it if any of these fail */
+	Assert(metad->btm_magic == BTREE_MAGIC);
+	Assert(metad->btm_version >= BTREE_MIN_VERSION);
+	Assert(metad->btm_fastroot != P_NONE);
 
 	return metad->btm_version > BTREE_NOVAC_VERSION;
 }
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index a3583f225b..1b312311ea 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -97,7 +97,7 @@ typedef BTPageOpaqueData *BTPageOpaque;
 typedef struct BTMetaPageData
 {
 	uint32		btm_magic;		/* should contain BTREE_MAGIC */
-	uint32		btm_version;	/* should contain BTREE_VERSION */
+	uint32		btm_version;	/* nbtree version (always <= BTREE_VERSION) */
 	BlockNumber btm_root;		/* current root location */
 	uint32		btm_level;		/* tree level of the root page */
 	BlockNumber btm_fastroot;	/* current "fast" root location */
-- 
2.17.1