Remove redundant snapshot copying from parallel leader to workers
The parallel query infrastructure copies the leader backend's active snapshot to the worker processes. But BitmapHeapScan node also had bespoken code to pass the snapshot from leader to the worker. That was redundant, so remove it. The removed code was analogous to the snapshot serialization in table_parallelscan_initialize(), but that was the wrong role model. A parallel bitmap heap scan is more like an independent non-parallel bitmap heap scan in each parallel worker as far as the table AM is concerned, because the coordination is done in nodeBitmapHeapscan.c, and the table AM doesn't need to know anything about it. This relies on the assumption that es_snapshot == GetActiveSnapshot(). That's not a new assumption, things would get weird if you used the QueryDesc's snapshot for visibility checks in the scans, but the active snapshot for evaluating quals, for example. This could use some refactoring and cleanup, but for now, just add some assertions. Reviewed-by: Dilip Kumar, Robert Haas Discussion: https://www.postgresql.org/message-id/5f3b9d59-0f43-419d-80ca-6d04c07cf61a@iki.fi
This commit is contained in:
parent
2346df6fc3
commit
84c18acaf6
|
@ -120,16 +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.
|
||||
|
|
|
@ -147,6 +147,9 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
|
|||
Assert(queryDesc != NULL);
|
||||
Assert(queryDesc->estate == NULL);
|
||||
|
||||
/* caller must ensure the query's snapshot is active */
|
||||
Assert(GetActiveSnapshot() == queryDesc->snapshot);
|
||||
|
||||
/*
|
||||
* If the transaction is read-only, we need to check if any writes are
|
||||
* planned to non-temporary tables. EXPLAIN is considered read-only.
|
||||
|
@ -319,6 +322,9 @@ standard_ExecutorRun(QueryDesc *queryDesc,
|
|||
Assert(estate != NULL);
|
||||
Assert(!(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY));
|
||||
|
||||
/* caller must ensure the query's snapshot is active */
|
||||
Assert(GetActiveSnapshot() == estate->es_snapshot);
|
||||
|
||||
/*
|
||||
* Switch into per-query memory context
|
||||
*/
|
||||
|
|
|
@ -720,6 +720,13 @@ ExecInitParallelPlan(PlanState *planstate, EState *estate,
|
|||
shm_toc_estimate_chunk(&pcxt->estimator, dsa_minsize);
|
||||
shm_toc_estimate_keys(&pcxt->estimator, 1);
|
||||
|
||||
/*
|
||||
* InitializeParallelDSM() passes the active snapshot to the parallel
|
||||
* worker, which uses it to set es_snapshot. Make sure we don't set
|
||||
* es_snapshot differently in the child.
|
||||
*/
|
||||
Assert(GetActiveSnapshot() == estate->es_snapshot);
|
||||
|
||||
/* Everyone's had a chance to ask for space, so now create the DSM. */
|
||||
InitializeParallelDSM(pcxt);
|
||||
|
||||
|
|
|
@ -721,7 +721,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
|
|||
scanstate->prefetch_iterator = NULL;
|
||||
scanstate->prefetch_pages = 0;
|
||||
scanstate->prefetch_target = 0;
|
||||
scanstate->pscan_len = 0;
|
||||
scanstate->initialized = false;
|
||||
scanstate->shared_tbmiterator = NULL;
|
||||
scanstate->shared_prefetch_iterator = NULL;
|
||||
|
@ -841,13 +840,7 @@ void
|
|||
ExecBitmapHeapEstimate(BitmapHeapScanState *node,
|
||||
ParallelContext *pcxt)
|
||||
{
|
||||
EState *estate = node->ss.ps.state;
|
||||
|
||||
node->pscan_len = add_size(offsetof(ParallelBitmapHeapState,
|
||||
phs_snapshot_data),
|
||||
EstimateSnapshotSpace(estate->es_snapshot));
|
||||
|
||||
shm_toc_estimate_chunk(&pcxt->estimator, node->pscan_len);
|
||||
shm_toc_estimate_chunk(&pcxt->estimator, sizeof(ParallelBitmapHeapState));
|
||||
shm_toc_estimate_keys(&pcxt->estimator, 1);
|
||||
}
|
||||
|
||||
|
@ -862,14 +855,13 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
|
|||
ParallelContext *pcxt)
|
||||
{
|
||||
ParallelBitmapHeapState *pstate;
|
||||
EState *estate = node->ss.ps.state;
|
||||
dsa_area *dsa = node->ss.ps.state->es_query_dsa;
|
||||
|
||||
/* If there's no DSA, there are no workers; initialize nothing. */
|
||||
if (dsa == NULL)
|
||||
return;
|
||||
|
||||
pstate = shm_toc_allocate(pcxt->toc, node->pscan_len);
|
||||
pstate = shm_toc_allocate(pcxt->toc, sizeof(ParallelBitmapHeapState));
|
||||
|
||||
pstate->tbmiterator = 0;
|
||||
pstate->prefetch_iterator = 0;
|
||||
|
@ -881,7 +873,6 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
|
|||
pstate->state = BM_INITIAL;
|
||||
|
||||
ConditionVariableInit(&pstate->cv);
|
||||
SerializeSnapshot(estate->es_snapshot, pstate->phs_snapshot_data);
|
||||
|
||||
shm_toc_insert(pcxt->toc, node->ss.ps.plan->plan_node_id, pstate);
|
||||
node->pstate = pstate;
|
||||
|
@ -927,13 +918,9 @@ 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);
|
||||
}
|
||||
|
|
|
@ -1038,11 +1038,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.
|
||||
*/
|
||||
|
|
|
@ -1689,7 +1689,6 @@ typedef enum
|
|||
* prefetch_target current target prefetch distance
|
||||
* state current state of the TIDBitmap
|
||||
* cv conditional wait variable
|
||||
* phs_snapshot_data snapshot data shared to workers
|
||||
* ----------------
|
||||
*/
|
||||
typedef struct ParallelBitmapHeapState
|
||||
|
@ -1701,7 +1700,6 @@ typedef struct ParallelBitmapHeapState
|
|||
int prefetch_target;
|
||||
SharedBitmapState state;
|
||||
ConditionVariable cv;
|
||||
char phs_snapshot_data[FLEXIBLE_ARRAY_MEMBER];
|
||||
} ParallelBitmapHeapState;
|
||||
|
||||
/* ----------------
|
||||
|
@ -1721,7 +1719,6 @@ typedef struct ParallelBitmapHeapState
|
|||
* prefetch_pages # pages prefetch iterator is ahead of current
|
||||
* prefetch_target current target prefetch distance
|
||||
* prefetch_maximum maximum value for prefetch_target
|
||||
* pscan_len size of the shared memory for parallel bitmap
|
||||
* initialized is node is ready to iterate
|
||||
* shared_tbmiterator shared iterator
|
||||
* shared_prefetch_iterator shared iterator for prefetching
|
||||
|
@ -1745,7 +1742,6 @@ typedef struct BitmapHeapScanState
|
|||
int prefetch_pages;
|
||||
int prefetch_target;
|
||||
int prefetch_maximum;
|
||||
Size pscan_len;
|
||||
bool initialized;
|
||||
TBMSharedIterator *shared_tbmiterator;
|
||||
TBMSharedIterator *shared_prefetch_iterator;
|
||||
|
|
Loading…
Reference in New Issue