From b8588077da95c47adff37fe2fa5ced5421c78134 Mon Sep 17 00:00:00 2001 From: Greg Nancarrow Date: Thu, 20 May 2021 15:20:37 +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 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