cleaning up a few CLOG-related things

From aebab6ae8726b968b63e2079f84d751e5341fb4f Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Mon, 25 Jan 2021 11:18:44 -0500
Subject: [PATCH v1 3/4] In TrimCLOG(), don't reset
 XactCtl->shared->latest_page_number.

Since the CLOG page number is not recorded directly in the checkpoint
record, we have to use ShmemVariableCache->nextXid to figure out the
latest CLOG page number at the start of recovery. However, as recovery
progresses, replay of CLOG/EXTEND records will update our notion of
the latest page number, and we should rely on that being accurate
rather than recomputing the value based on an updated notion of
nextXid. ShmemVariableCache->nextXid is only an approximation
during recovery anyway, whereas CLOG/EXTEND records are an
authoritative representation of how the SLRU has been updated.

Commit [ INSERT COMMIT ID FOR 0002 HERE ] makes this
simplification possible, as before that change clog_redo() might
have injected a bogus value here, and we'd want to get rid of
that before entering normal running.
---
 src/backend/access/transam/clog.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 5ae962b86e..6fa4713fb4 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -773,11 +773,6 @@ TrimCLOG(void)
 
 	LWLockAcquire(XactSLRULock, LW_EXCLUSIVE);
 
-	/*
-	 * Re-Initialize our idea of the latest page number.
-	 */
-	XactCtl->shared->latest_page_number = pageno;
-
 	/*
 	 * Zero out the remainder of the current clog page.  Under normal
 	 * circumstances it should be zeroes already, but it seems at least
-- 
2.24.3 (Apple Git-128)

From c3fff35ea0c5b243a2cc19e3408efd020c31dd12 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Mon, 25 Jan 2021 11:08:08 -0500
Subject: [PATCH v1 2/4] In clog_redo(), don't set
 XactCtl->shared->latest_page_number.

The comment is no longer accurate, and hasn't been entirely accurate
since Hot Standby was introduced. The original idea here was that
StartupCLOG() wouldn't be called until the end of recovery and
therefore this value would be uninitialized when this code is
reached, but Hot Standby made that true only when hot_standby=off,
and commit [ INSERT COMMIT ID FOR 0001 HERE ] means
that this value is now always initialized before replay even starts.

The original purpose of this code was to bypass the sanity check
in SimpleLruTruncate(), which will no longer occur: now, if something
is wrong, that sanity check might trip during recovery. That's
probably a good thing, because in the current code base
latest_page_number should always be initialized and therefore we
expect that the sanity check should pass. If it doesn't, something
has gone wrong, and complaining about it is appropriate.
---
 src/backend/access/transam/clog.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 410d02a394..5ae962b86e 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -1011,12 +1011,6 @@ clog_redo(XLogReaderState *record)
 
 		memcpy(&xlrec, XLogRecGetData(record), sizeof(xl_clog_truncate));
 
-		/*
-		 * During XLOG replay, latest_page_number isn't set up yet; insert a
-		 * suitable value to bypass the sanity test in SimpleLruTruncate.
-		 */
-		XactCtl->shared->latest_page_number = xlrec.pageno;
-
 		AdvanceOldestClogXid(xlrec.oldestXact);
 
 		SimpleLruTruncate(XactCtl, xlrec.pageno);
-- 
2.24.3 (Apple Git-128)

From addfc07b80e304194129423eabe78a4608335220 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Mon, 25 Jan 2021 11:32:11 -0500
Subject: [PATCH v1 4/4] In StartupCLOG(), correct an off-by-one error.

nextXid represents the first XID that hasn't yet been allocated, not
the last one that gets allocated. So, we should use the previous XID
to determine the last CLOG page that ought to exist.

As far as we currently know, this mistake doesn't have any adverse
consequences at present, but it seems better to make the logic
more correct.
---
 src/backend/access/transam/clog.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 6fa4713fb4..bad883d5a2 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -742,6 +742,23 @@ ZeroCLOGPage(int pageno, bool writeXlog)
 	return slotno;
 }
 
+/*
+ * Based on ShmemVariableCache->nextXid, compute the latest CLOG page that
+ * is expected to exist.
+ */
+static int
+FullTransactionIdToLatestPageNumber(FullTransactionId nextXid)
+{
+	/*
+	 * If nextXid coresponds to the first transaction on a CLOG page, then
+	 * the CLOG page will not have been initialized yet, but if it corresponds
+	 * to the second or later transaction on the page, then it should already
+	 * exist. See GetNewTransactionId().
+	 */
+	FullTransactionIdRetreat(&nextXid);
+	return TransactionIdToPage(XidFromFullTransactionId(nextXid));
+}
+
 /*
  * This must be called ONCE during postmaster or standalone-backend startup,
  * after StartupXLOG has initialized ShmemVariableCache->nextXid.
@@ -749,16 +766,12 @@ ZeroCLOGPage(int pageno, bool writeXlog)
 void
 StartupCLOG(void)
 {
-	TransactionId xid = XidFromFullTransactionId(ShmemVariableCache->nextXid);
-	int			pageno = TransactionIdToPage(xid);
+	int			pageno;
 
+	/* Initialize our idea of the latest page number. */
+	pageno = FullTransactionIdToLatestPageNumber(ShmemVariableCache->nextXid);
 	LWLockAcquire(XactSLRULock, LW_EXCLUSIVE);
-
-	/*
-	 * Initialize our idea of the latest page number.
-	 */
 	XactCtl->shared->latest_page_number = pageno;
-
 	LWLockRelease(XactSLRULock);
 }
 
-- 
2.24.3 (Apple Git-128)

From cb14fe3a02fe3db03e46d0371ac6cc8a2b113576 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Mon, 25 Jan 2021 11:04:22 -0500
Subject: [PATCH v1 1/4] Move StartupCLOG() calls to just after we initialize
 ShmemVariableCache.

Previously, the hot_standby=off code path did this at end of recovery,
while the hot_standby=on code path did it at the beginning of recovery.
It's better to do this in only one place because (a) it's simpler,
(b) StartupCLOG() is trivial so trying to postpone the work isn't
useful, and (c) this will make it possible to simplify some other
logic.
---
 src/backend/access/transam/xlog.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 470e113b33..1cfee5c270 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6859,6 +6859,13 @@ StartupXLOG(void)
 	 */
 	StartupReorderBuffer();
 
+	/*
+	 * Startup CLOG. This must be done after ShmemVariableCache->nextXid
+	 * has been initialized and before we accept connections or begin WAL
+	 * replay.
+	 */
+	StartupCLOG();
+
 	/*
 	 * Startup MultiXact. We need to do this early to be able to replay
 	 * truncations.
@@ -7129,11 +7136,10 @@ StartupXLOG(void)
 			ProcArrayInitRecovery(XidFromFullTransactionId(ShmemVariableCache->nextXid));
 
 			/*
-			 * Startup commit log and subtrans only.  MultiXact and commit
+			 * Startup subtrans only.  CLOG, MultiXact and commit
 			 * timestamp have already been started up and other SLRUs are not
 			 * maintained during recovery and need not be started yet.
 			 */
-			StartupCLOG();
 			StartupSUBTRANS(oldestActiveXID);
 
 			/*
@@ -7949,14 +7955,11 @@ StartupXLOG(void)
 	LWLockRelease(ProcArrayLock);
 
 	/*
-	 * Start up the commit log and subtrans, if not already done for hot
-	 * standby.  (commit timestamps are started below, if necessary.)
+	 * Start up subtrans, if not already done for hot standby.  (commit
+	 * timestamps are started below, if necessary.)
 	 */
 	if (standbyState == STANDBY_DISABLED)
-	{
-		StartupCLOG();
 		StartupSUBTRANS(oldestActiveXID);
-	}
 
 	/*
 	 * Perform end of recovery actions for any SLRUs that need it.
-- 
2.24.3 (Apple Git-128)