v1-0001-PG13_2-Fix-parallel-worker-failed-assertion-and-coredump.patch

From 7a0d5c4e85e9ba164c701b765ae8ec669f3d39de Mon Sep 17 00:00:00 2001
From: Greg Nancarrow <gregn4422@gmail.com>
Date: Thu, 20 May 2021 13:05:42 +1000
Subject: [PATCH] Fix parallel worker failed assertion and coredump.

Currently, InitializeParallelDSM() calls GetTransactionSnapshot() and serializes
both the active_snapshot and transaction_snapshot, for parallel workers to
restore and use.
However here the transaction_snapshot is potentially a later snapshot than the
active_snapshot, and seems extraneous, as the active_snapshot already points to
a snapshot from a prior call to GetTransactionSnapshot() (just prior to plan
execution).
This patch removes the extraneous call to GetTransactionSnapshot() and modifies
the parallel DSM code to just pass the active_snapshot, and restore it in
the parallel workers as the transaction snapshot and set it as the active
snapshot.
This resolves an assertion failure in SubTransGetTopmostTransaction() that can
occur in a parallel scan, after snapshot suboverflow has occurred.

Discussion: https://postgr.es/m/002f01d748ac$eaa781a0$bff684e0$@tju.edu.cn
---
 src/backend/access/transam/parallel.c | 26 +++++++-------------------
 src/backend/utils/time/snapmgr.c      | 14 ++++++++++++++
 src/include/utils/snapmgr.h           |  1 +
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 14a8690019..9118dfe37d 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -67,7 +67,6 @@
 #define PARALLEL_KEY_LIBRARY				UINT64CONST(0xFFFFFFFFFFFF0003)
 #define PARALLEL_KEY_GUC					UINT64CONST(0xFFFFFFFFFFFF0004)
 #define PARALLEL_KEY_COMBO_CID				UINT64CONST(0xFFFFFFFFFFFF0005)
-#define PARALLEL_KEY_TRANSACTION_SNAPSHOT	UINT64CONST(0xFFFFFFFFFFFF0006)
 #define PARALLEL_KEY_ACTIVE_SNAPSHOT		UINT64CONST(0xFFFFFFFFFFFF0007)
 #define PARALLEL_KEY_TRANSACTION_STATE		UINT64CONST(0xFFFFFFFFFFFF0008)
 #define PARALLEL_KEY_ENTRYPOINT				UINT64CONST(0xFFFFFFFFFFFF0009)
@@ -205,7 +204,6 @@ InitializeParallelDSM(ParallelContext *pcxt)
 	Size		library_len = 0;
 	Size		guc_len = 0;
 	Size		combocidlen = 0;
-	Size		tsnaplen = 0;
 	Size		asnaplen = 0;
 	Size		tstatelen = 0;
 	Size		pendingsyncslen = 0;
@@ -216,7 +214,6 @@ InitializeParallelDSM(ParallelContext *pcxt)
 	int			i;
 	FixedParallelState *fps;
 	dsm_handle	session_dsm_handle = DSM_HANDLE_INVALID;
-	Snapshot	transaction_snapshot = GetTransactionSnapshot();
 	Snapshot	active_snapshot = GetActiveSnapshot();
 
 	/* We might be running in a very short-lived memory context. */
@@ -254,8 +251,6 @@ InitializeParallelDSM(ParallelContext *pcxt)
 		shm_toc_estimate_chunk(&pcxt->estimator, guc_len);
 		combocidlen = EstimateComboCIDStateSpace();
 		shm_toc_estimate_chunk(&pcxt->estimator, combocidlen);
-		tsnaplen = EstimateSnapshotSpace(transaction_snapshot);
-		shm_toc_estimate_chunk(&pcxt->estimator, tsnaplen);
 		asnaplen = EstimateSnapshotSpace(active_snapshot);
 		shm_toc_estimate_chunk(&pcxt->estimator, asnaplen);
 		tstatelen = EstimateTransactionStateSpace();
@@ -339,7 +334,6 @@ InitializeParallelDSM(ParallelContext *pcxt)
 		char	   *libraryspace;
 		char	   *gucspace;
 		char	   *combocidspace;
-		char	   *tsnapspace;
 		char	   *asnapspace;
 		char	   *tstatespace;
 		char	   *pendingsyncsspace;
@@ -366,11 +360,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
 		SerializeComboCIDState(combocidlen, combocidspace);
 		shm_toc_insert(pcxt->toc, PARALLEL_KEY_COMBO_CID, combocidspace);
 
-		/* Serialize transaction snapshot and active snapshot. */
-		tsnapspace = shm_toc_allocate(pcxt->toc, tsnaplen);
-		SerializeSnapshot(transaction_snapshot, tsnapspace);
-		shm_toc_insert(pcxt->toc, PARALLEL_KEY_TRANSACTION_SNAPSHOT,
-					   tsnapspace);
+		/* Serialize active snapshot. */
 		asnapspace = shm_toc_allocate(pcxt->toc, asnaplen);
 		SerializeSnapshot(active_snapshot, asnapspace);
 		shm_toc_insert(pcxt->toc, PARALLEL_KEY_ACTIVE_SNAPSHOT, asnapspace);
@@ -1251,7 +1241,6 @@ ParallelWorkerMain(Datum main_arg)
 	parallel_worker_main_type entrypt;
 	char	   *gucspace;
 	char	   *combocidspace;
-	char	   *tsnapspace;
 	char	   *asnapspace;
 	char	   *tstatespace;
 	char	   *pendingsyncsspace;
@@ -1407,14 +1396,13 @@ ParallelWorkerMain(Datum main_arg)
 		shm_toc_lookup(toc, PARALLEL_KEY_SESSION_DSM, false);
 	AttachSession(*(dsm_handle *) session_dsm_handle_space);
 
-	/* Restore transaction snapshot. */
-	tsnapspace = shm_toc_lookup(toc, PARALLEL_KEY_TRANSACTION_SNAPSHOT, false);
-	RestoreTransactionSnapshot(RestoreSnapshot(tsnapspace),
-							   fps->parallel_master_pgproc);
-
-	/* Restore active snapshot. */
+	/*
+	 * Restore the snapshot, install it as the transaction snapshot and set it
+	 * as the active snapshot.
+	 */
 	asnapspace = shm_toc_lookup(toc, PARALLEL_KEY_ACTIVE_SNAPSHOT, false);
-	PushActiveSnapshot(RestoreSnapshot(asnapspace));
+	RestoreTxnSnapshotAndSetAsActive(RestoreSnapshot(asnapspace),
+							   fps->parallel_master_pgproc);
 
 	/*
 	 * We've changed which tuples we can see, and must therefore invalidate
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 5e3b31ba90..c5861c1af3 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -2228,6 +2228,20 @@ RestoreTransactionSnapshot(Snapshot snapshot, void *master_pgproc)
 	SetTransactionSnapshot(snapshot, NULL, InvalidPid, master_pgproc);
 }
 
+/*
+ * Install a restored snapshot as the transaction snapshot, and set it as
+ * the active snapshot.
+ *
+ * The second argument is of type void * so that snapmgr.h need not include
+ * the declaration for PGPROC.
+ */
+void
+RestoreTxnSnapshotAndSetAsActive(Snapshot snapshot, void *master_pgproc)
+{
+	RestoreTransactionSnapshot(snapshot, master_pgproc);
+	PushActiveSnapshot(CurrentSnapshot);
+}
+
 /*
  * XidInMVCCSnapshot
  *		Is the given XID still-in-progress according to the snapshot?
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index ad7c15dc1a..3b6d9c77a7 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -154,5 +154,6 @@ extern Size EstimateSnapshotSpace(Snapshot snapshot);
 extern void SerializeSnapshot(Snapshot snapshot, char *start_address);
 extern Snapshot RestoreSnapshot(char *start_address);
 extern void RestoreTransactionSnapshot(Snapshot snapshot, void *master_pgproc);
+extern void RestoreTxnSnapshotAndSetAsActive(Snapshot snapshot, void *master_pgproc);
 
 #endif							/* SNAPMGR_H */
-- 
2.27.0