From 6cfa8fee46a83936789062eedc37d0c06b59dc46 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 12 Feb 2024 18:50:29 -0500 Subject: [PATCH v2 03/13] BitmapHeapScan begin scan after bitmap setup There is no reason for table_beginscan_bm() to begin the actual scan of the underlying table in ExecInitBitmapHeapScan(). We can begin the underlying table scan after the index scan has been completed and the bitmap built. The one use of the scan descriptor during initialization was ExecBitmapHeapInitializeWorker(), which set the scan descriptor snapshot with one from an array in the parallel state. This overwrote the snapshot set in table_beginscan_bm(). By saving that worker snapshot as a member in the BitmapHeapScanState during initialization, it can be restored in table_beginscan_bm() after returning from the table AM specific begin scan function. --- src/backend/access/table/tableam.c | 11 ------ src/backend/executor/nodeBitmapHeapscan.c | 43 +++++++++++++++++------ src/include/access/tableam.h | 10 ++---- src/include/nodes/execnodes.h | 2 ++ 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index 6ed8cca05a1..e78d793f69c 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -120,17 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key) NULL, flags); } -void -table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot) -{ - Assert(IsMVCCSnapshot(snapshot)); - - RegisterSnapshot(snapshot); - scan->rs_snapshot = snapshot; - scan->rs_flags |= SO_TEMP_SNAPSHOT; -} - - /* ---------------------------------------------------------------------------- * Parallel table scan related functions. * ---------------------------------------------------------------------------- diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 76382c91fd7..be08bd785ae 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -191,6 +191,30 @@ BitmapHeapNext(BitmapHeapScanState *node) } #endif /* USE_PREFETCH */ } + + if (!scan) + { + Snapshot snapshot = node->ss.ps.state->es_snapshot; + uint32 extra_flags = 0; + + /* + * Parallel workers must use the snapshot initialized by the + * parallel leader. + */ + if (node->worker_snapshot) + { + snapshot = node->worker_snapshot; + extra_flags |= SO_TEMP_SNAPSHOT; + } + + scan = node->ss.ss_currentScanDesc = table_beginscan_bm( + node->ss.ss_currentRelation, + snapshot, + 0, + NULL, + extra_flags); + } + node->initialized = true; } @@ -614,7 +638,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node) PlanState *outerPlan = outerPlanState(node); /* rescan to release any page pin */ - table_rescan(node->ss.ss_currentScanDesc, NULL); + if (node->ss.ss_currentScanDesc) + table_rescan(node->ss.ss_currentScanDesc, NULL); /* release bitmaps and buffers if any */ if (node->tbmiterator) @@ -691,7 +716,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) /* * close heap scan */ - table_endscan(scanDesc); + if (scanDesc) + table_endscan(scanDesc); + } /* ---------------------------------------------------------------- @@ -740,6 +767,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) scanstate->shared_prefetch_iterator = NULL; scanstate->pstate = NULL; scanstate->can_skip_fetch = false; + scanstate->worker_snapshot = NULL; /* * Miscellaneous initialization @@ -788,11 +816,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) scanstate->ss.ss_currentRelation = currentRelation; - scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation, - estate->es_snapshot, - 0, - NULL); - /* * all done. */ @@ -931,13 +954,13 @@ ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node, ParallelWorkerContext *pwcxt) { ParallelBitmapHeapState *pstate; - Snapshot snapshot; Assert(node->ss.ps.state->es_query_dsa != NULL); pstate = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false); node->pstate = pstate; - snapshot = RestoreSnapshot(pstate->phs_snapshot_data); - table_scan_update_snapshot(node->ss.ss_currentScanDesc, snapshot); + node->worker_snapshot = RestoreSnapshot(pstate->phs_snapshot_data); + Assert(IsMVCCSnapshot(node->worker_snapshot)); + RegisterSnapshot(node->worker_snapshot); } diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 4d495216f07..8ef6b5ca25b 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -939,9 +939,10 @@ table_beginscan_strat(Relation rel, Snapshot snapshot, */ static inline TableScanDesc table_beginscan_bm(Relation rel, Snapshot snapshot, - int nkeys, struct ScanKeyData *key) + int nkeys, struct ScanKeyData *key, + uint32 extra_flags) { - uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE; + uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags; return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags); } @@ -1033,11 +1034,6 @@ table_rescan_set_params(TableScanDesc scan, struct ScanKeyData *key, allow_pagemode); } -/* - * Update snapshot used by the scan. - */ -extern void table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot); - /* * Return next tuple from `scan`, store in slot. */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 444a5f0fd57..00c75fb10e2 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1726,6 +1726,7 @@ typedef struct ParallelBitmapHeapState * shared_tbmiterator shared iterator * shared_prefetch_iterator shared iterator for prefetching * pstate shared state for parallel bitmap scan + * worker_snapshot snapshot for parallel worker * ---------------- */ typedef struct BitmapHeapScanState @@ -1750,6 +1751,7 @@ typedef struct BitmapHeapScanState TBMSharedIterator *shared_tbmiterator; TBMSharedIterator *shared_prefetch_iterator; ParallelBitmapHeapState *pstate; + Snapshot worker_snapshot; } BitmapHeapScanState; /* ---------------- -- 2.37.2