On Thu, May 27, 2021 at 10:01:49AM -0700, Andres Freund wrote:
> Why would it be intrusive? We're talking a split second here, no? More
> importantly, I don't think it's correct to release the locks at that
> point.

I have been looking at all that for the last couple of days, and
checked the code to make sure that relying on RecoveryInProgress() as
the tipping point is logically correct in terms of virtual XID,
snapshot build and KnownAssignedXids cleanup.  This stuff is tricky
enough that I may have missed something, but my impression (and
testing) is that we should be safe.

I am adding this patch to the next CF for now.  More eyes are needed.
--
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 441a9124cd..40c56d9012 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8033,13 +8033,6 @@ StartupXLOG(void)
 	/* Reload shared-memory state for prepared transactions */
 	RecoverPreparedTransactions();
 
-	/*
-	 * Shutdown the recovery environment. This must occur after
-	 * RecoverPreparedTransactions(), see notes for lock_twophase_recover()
-	 */
-	if (standbyState != STANDBY_DISABLED)
-		ShutdownRecoveryTransactionEnvironment();
-
 	/* Shut down xlogreader */
 	if (readFile >= 0)
 	{
@@ -8087,6 +8080,18 @@ StartupXLOG(void)
 	UpdateControlFile();
 	LWLockRelease(ControlFileLock);
 
+	/*
+	 * Shutdown the recovery environment. This must occur after
+	 * RecoverPreparedTransactions() (see notes in lock_twophase_recover())
+	 * and after switching SharedRecoveryState to RECOVERY_STATE_DONE so as
+	 * any session building a snapshot will not rely on KnownAssignedXids as
+	 * RecoveryInProgress() is disabled at this stage.  This is particularly
+	 * critical for prepared 2PC transactions, that may still need to be
+	 * included in snapshots once recovery has ended.
+	 */
+	if (standbyState != STANDBY_DISABLED)
+		ShutdownRecoveryTransactionEnvironment();
+
 	/*
 	 * If there were cascading standby servers connected to us, nudge any wal
 	 * sender processes to notice that we've been promoted.

Attachment: signature.asc
Description: PGP signature

Reply via email to