From 10c49d68e89d87b396ccf88c7fd10044869bfe33 Mon Sep 17 00:00:00 2001 From: Greg Nancarrow Date: Fri, 23 Jul 2021 15:52:13 +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. This patch needs more work, to address issues raised by Robert Haas (such as handling the cases of REPEATABLE READ and SERIALIZABLE transaction isolation levels). 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 3550ef13ba..2736d3824e 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); @@ -1252,7 +1242,6 @@ ParallelWorkerMain(Datum main_arg) parallel_worker_main_type entrypt; char *gucspace; char *combocidspace; - char *tsnapspace; char *asnapspace; char *tstatespace; char *pendingsyncsspace; @@ -1408,14 +1397,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_leader_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_leader_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 2968c7f7b7..b38f48b71d 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -2228,6 +2228,20 @@ RestoreTransactionSnapshot(Snapshot snapshot, void *source_pgproc) SetTransactionSnapshot(snapshot, NULL, InvalidPid, source_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 *source_pgproc) +{ + RestoreTransactionSnapshot(snapshot, source_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 44539fe15a..df31e28887 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -173,5 +173,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 *source_pgproc); +extern void RestoreTxnSnapshotAndSetAsActive(Snapshot snapshot, void *source_pgproc); #endif /* SNAPMGR_H */ -- 2.27.0