From d780f07ac1ea97e2d3cf906cc1c9d59d6b21c5e2 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 2 Dec 2005 20:03:42 +0000 Subject: [PATCH] Adjust scan plan nodes to avoid getting an extra AccessShareLock on a relation if it's already been locked by execMain.c as either a result relation or a FOR UPDATE/SHARE relation. This avoids an extra trip to the shared lock manager state. Per my suggestion yesterday. --- src/backend/executor/execMain.c | 21 ++--- src/backend/executor/execUtils.c | 92 +++++++++++++++++++++- src/backend/executor/nodeBitmapHeapscan.c | 26 ++---- src/backend/executor/nodeBitmapIndexscan.c | 6 +- src/backend/executor/nodeIndexscan.c | 21 ++--- src/backend/executor/nodeSeqscan.c | 24 ++---- src/backend/executor/nodeTidscan.c | 21 +---- src/include/executor/executor.h | 5 +- src/include/nodes/execnodes.h | 11 ++- 9 files changed, 135 insertions(+), 92 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 4228a8f996..22ba7880ae 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -26,7 +26,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.261 2005/11/22 18:17:10 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.262 2005/12/02 20:03:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -52,13 +52,6 @@ #include "utils/memutils.h" -typedef struct execRowMark -{ - Relation relation; - Index rti; - char resname[32]; -} execRowMark; - typedef struct evalPlanQual { Index rti; @@ -567,10 +560,10 @@ InitPlan(QueryDesc *queryDesc, bool explainOnly) Index rti = lfirst_int(l); Oid relid = getrelid(rti, rangeTable); Relation relation; - execRowMark *erm; + ExecRowMark *erm; relation = heap_open(relid, RowShareLock); - erm = (execRowMark *) palloc(sizeof(execRowMark)); + erm = (ExecRowMark *) palloc(sizeof(ExecRowMark)); erm->relation = relation; erm->rti = rti; snprintf(erm->resname, sizeof(erm->resname), "ctid%u", rti); @@ -1020,7 +1013,7 @@ ExecEndPlan(PlanState *planstate, EState *estate) */ foreach(l, estate->es_rowMarks) { - execRowMark *erm = lfirst(l); + ExecRowMark *erm = lfirst(l); heap_close(erm->relation, NoLock); } @@ -1165,7 +1158,7 @@ lnext: ; lmark: ; foreach(l, estate->es_rowMarks) { - execRowMark *erm = lfirst(l); + ExecRowMark *erm = lfirst(l); HeapTupleData tuple; Buffer buffer; ItemPointerData update_ctid; @@ -1859,9 +1852,9 @@ EvalPlanQual(EState *estate, Index rti, relation = NULL; foreach(l, estate->es_rowMarks) { - if (((execRowMark *) lfirst(l))->rti == rti) + if (((ExecRowMark *) lfirst(l))->rti == rti) { - relation = ((execRowMark *) lfirst(l))->relation; + relation = ((ExecRowMark *) lfirst(l))->relation; break; } } diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 5221624fc7..c814b97147 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.129 2005/11/23 20:27:57 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.130 2005/12/02 20:03:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -24,6 +24,9 @@ * ExecAssignResultType * etc * + * ExecOpenScanRelation Common code for scan node init routines. + * ExecCloseScanRelation + * * ExecOpenIndices \ * ExecCloseIndices | referenced by InitPlan, EndPlan, * ExecInsertIndexTuples / ExecInsert, ExecUpdate @@ -45,6 +48,7 @@ #include "catalog/pg_index.h" #include "executor/execdebug.h" #include "miscadmin.h" +#include "parser/parsetree.h" #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/memutils.h" @@ -684,6 +688,90 @@ ExecAssignScanTypeFromOuterPlan(ScanState *scanstate) } +/* ---------------------------------------------------------------- + * Scan node support + * ---------------------------------------------------------------- + */ + +/* ---------------------------------------------------------------- + * ExecOpenScanRelation + * + * Open the heap relation to be scanned by a base-level scan plan node. + * This should be called during the node's ExecInit routine. + * + * By default, this acquires AccessShareLock on the relation. However, + * if the relation was already locked by InitPlan, we don't need to acquire + * any additional lock. This saves trips to the shared lock manager. + * ---------------------------------------------------------------- + */ +Relation +ExecOpenScanRelation(EState *estate, Index scanrelid) +{ + RangeTblEntry *rtentry; + Oid reloid; + LOCKMODE lockmode; + ResultRelInfo *resultRelInfos; + int i; + + /* + * First determine the lock type we need. Scan to see if target relation + * is either a result relation or a FOR UPDATE/FOR SHARE relation. + */ + lockmode = AccessShareLock; + resultRelInfos = estate->es_result_relations; + for (i = 0; i < estate->es_num_result_relations; i++) + { + if (resultRelInfos[i].ri_RangeTableIndex == scanrelid) + { + lockmode = NoLock; + break; + } + } + + if (lockmode == AccessShareLock) + { + ListCell *l; + + foreach(l, estate->es_rowMarks) + { + ExecRowMark *erm = lfirst(l); + + if (erm->rti == scanrelid) + { + lockmode = NoLock; + break; + } + } + } + + /* OK, open the relation and acquire lock as needed */ + rtentry = rt_fetch(scanrelid, estate->es_range_table); + reloid = rtentry->relid; + + return heap_open(reloid, lockmode); +} + +/* ---------------------------------------------------------------- + * ExecCloseScanRelation + * + * Close the heap relation scanned by a base-level scan plan node. + * This should be called during the node's ExecEnd routine. + * + * Currently, we do not release the lock acquired by ExecOpenScanRelation. + * This lock should be held till end of transaction. (There is a faction + * that considers this too much locking, however.) + * + * If we did want to release the lock, we'd have to repeat the logic in + * ExecOpenScanRelation in order to figure out what to release. + * ---------------------------------------------------------------- + */ +void +ExecCloseScanRelation(Relation scanrel) +{ + heap_close(scanrel, NoLock); +} + + /* ---------------------------------------------------------------- * ExecInsertIndexTuples support * ---------------------------------------------------------------- @@ -760,7 +848,7 @@ ExecOpenIndices(ResultRelInfo *resultRelInfo) * * If the index AM is not safe for concurrent updates, obtain an * exclusive lock on the index to lock out other updaters as well as - * readers (index_beginscan places AccessShareLock). + * readers (index_beginscan places AccessShareLock on the index). * * If there are multiple not-concurrent-safe indexes, all backends * must lock the indexes in the same order or we will get deadlocks diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 37bd42d9d7..959b559d1b 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -21,7 +21,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.7 2005/12/02 01:29:55 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.8 2005/12/02 20:03:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -141,9 +141,9 @@ BitmapHeapNext(BitmapHeapScanState *node) /* * Ignore any claimed entries past what we think is the end of the - * relation. (This is probably not necessary given that we got - * AccessShareLock before performing any of the indexscans, but - * let's be safe.) + * relation. (This is probably not necessary given that we got at + * least AccessShareLock on the table before performing any of the + * indexscans, but let's be safe.) */ if (tbmres->blockno >= scan->rs_nblocks) { @@ -448,13 +448,8 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) /* * close the heap relation. - * - * Currently, we do not release the AccessShareLock acquired by - * ExecInitBitmapHeapScan. This lock should be held till end of - * transaction. (There is a faction that considers this too much locking, - * however.) */ - heap_close(relation, NoLock); + ExecCloseScanRelation(relation); } /* ---------------------------------------------------------------- @@ -467,9 +462,6 @@ BitmapHeapScanState * ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate) { BitmapHeapScanState *scanstate; - RangeTblEntry *rtentry; - Index relid; - Oid reloid; Relation currentRelation; /* @@ -519,13 +511,9 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate) CXT1_printf("ExecInitBitmapHeapScan: context is %d\n", CurrentMemoryContext); /* - * open the base relation and acquire AccessShareLock on it. + * open the base relation and acquire appropriate lock on it. */ - relid = node->scan.scanrelid; - rtentry = rt_fetch(relid, estate->es_range_table); - reloid = rtentry->relid; - - currentRelation = heap_open(reloid, AccessShareLock); + currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid); scanstate->ss.ss_currentRelation = currentRelation; diff --git a/src/backend/executor/nodeBitmapIndexscan.c b/src/backend/executor/nodeBitmapIndexscan.c index 46e8c14652..4217de3952 100644 --- a/src/backend/executor/nodeBitmapIndexscan.c +++ b/src/backend/executor/nodeBitmapIndexscan.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.12 2005/11/25 19:47:49 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.13 2005/12/02 20:03:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -286,8 +286,8 @@ ExecInitBitmapIndexScan(BitmapIndexScan *node, EState *estate) /* * We do not open or lock the base relation here. We assume that an - * ancestor BitmapHeapScan node is holding AccessShareLock on the heap - * relation throughout the execution of the plan tree. + * ancestor BitmapHeapScan node is holding AccessShareLock (or better) + * on the heap relation throughout the execution of the plan tree. */ indexstate->ss.ss_currentRelation = NULL; diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index f34a14c859..4beecfbd57 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.107 2005/11/25 19:47:49 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.108 2005/12/02 20:03:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -421,12 +421,8 @@ ExecEndIndexScan(IndexScanState *node) /* * close the heap relation. - * - * Currently, we do not release the AccessShareLock acquired by - * ExecInitIndexScan. This lock should be held till end of transaction. - * (There is a faction that considers this too much locking, however.) */ - heap_close(relation, NoLock); + ExecCloseScanRelation(relation); } /* ---------------------------------------------------------------- @@ -464,9 +460,6 @@ IndexScanState * ExecInitIndexScan(IndexScan *node, EState *estate) { IndexScanState *indexstate; - RangeTblEntry *rtentry; - Index relid; - Oid reloid; Relation currentRelation; /* @@ -551,13 +544,9 @@ ExecInitIndexScan(IndexScan *node, EState *estate) } /* - * open the base relation and acquire AccessShareLock on it. + * open the base relation and acquire appropriate lock on it. */ - relid = node->scan.scanrelid; - rtentry = rt_fetch(relid, estate->es_range_table); - reloid = rtentry->relid; - - currentRelation = heap_open(reloid, AccessShareLock); + currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid); indexstate->ss.ss_currentRelation = currentRelation; indexstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */ @@ -570,7 +559,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate) /* * open the index relation and initialize relation and scan descriptors. * Note we acquire no locks here; the index machinery does its own locks - * and unlocks. (We rely on having AccessShareLock on the parent table to + * and unlocks. (We rely on having a lock on the parent table to * ensure the index won't go away!) */ indexstate->iss_RelationDesc = index_open(node->indexid); diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c index 391fdf7e91..5e8ba95ee2 100644 --- a/src/backend/executor/nodeSeqscan.c +++ b/src/backend/executor/nodeSeqscan.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/nodeSeqscan.c,v 1.55 2005/11/25 04:24:48 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/nodeSeqscan.c,v 1.56 2005/12/02 20:03:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -141,25 +141,15 @@ ExecSeqScan(SeqScanState *node) static void InitScanRelation(SeqScanState *node, EState *estate) { - Index relid; - List *rangeTable; - RangeTblEntry *rtentry; - Oid reloid; Relation currentRelation; HeapScanDesc currentScanDesc; /* * get the relation object id from the relid'th entry in the range table, - * open that relation and initialize the scan state. - * - * We acquire AccessShareLock for the duration of the scan. + * open that relation and acquire appropriate lock on it. */ - relid = ((SeqScan *) node->ps.plan)->scanrelid; - rangeTable = estate->es_range_table; - rtentry = rt_fetch(relid, rangeTable); - reloid = rtentry->relid; - - currentRelation = heap_open(reloid, AccessShareLock); + currentRelation = ExecOpenScanRelation(estate, + ((SeqScan *) node->ps.plan)->scanrelid); currentScanDesc = heap_beginscan(currentRelation, estate->es_snapshot, @@ -281,12 +271,8 @@ ExecEndSeqScan(SeqScanState *node) /* * close the heap relation. - * - * Currently, we do not release the AccessShareLock acquired by - * InitScanRelation. This lock should be held till end of transaction. - * (There is a faction that considers this too much locking, however.) */ - heap_close(relation, NoLock); + ExecCloseScanRelation(relation); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index ba4b7f3bca..e6d61c78a6 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/nodeTidscan.c,v 1.45 2005/11/26 22:14:56 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/nodeTidscan.c,v 1.46 2005/12/02 20:03:41 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -421,12 +421,8 @@ ExecEndTidScan(TidScanState *node) /* * close the heap relation. - * - * Currently, we do not release the AccessShareLock acquired by - * ExecInitTidScan. This lock should be held till end of transaction. - * (There is a faction that considers this too much locking, however.) */ - heap_close(node->ss.ss_currentRelation, NoLock); + ExecCloseScanRelation(node->ss.ss_currentRelation); } /* ---------------------------------------------------------------- @@ -472,9 +468,6 @@ TidScanState * ExecInitTidScan(TidScan *node, EState *estate) { TidScanState *tidstate; - RangeTblEntry *rtentry; - Oid relid; - Oid reloid; Relation currentRelation; /* @@ -521,15 +514,9 @@ ExecInitTidScan(TidScan *node, EState *estate) tidstate->tss_TidPtr = -1; /* - * open the base relation - * - * We acquire AccessShareLock for the duration of the scan. + * open the base relation and acquire appropriate lock on it. */ - relid = node->scan.scanrelid; - rtentry = rt_fetch(relid, estate->es_range_table); - reloid = rtentry->relid; - - currentRelation = heap_open(reloid, AccessShareLock); + currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid); tidstate->ss.ss_currentRelation = currentRelation; tidstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 39c76c60aa..f1d4e37c41 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.121 2005/11/23 20:27:58 tgl Exp $ + * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.122 2005/12/02 20:03:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -230,6 +230,9 @@ extern void ExecAssignScanType(ScanState *scanstate, TupleDesc tupDesc, bool shouldFree); extern void ExecAssignScanTypeFromOuterPlan(ScanState *scanstate); +extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid); +extern void ExecCloseScanRelation(Relation scanrel); + extern void ExecOpenIndices(ResultRelInfo *resultRelInfo); extern void ExecCloseIndices(ResultRelInfo *resultRelInfo); extern void ExecInsertIndexTuples(TupleTableSlot *slot, ItemPointer tupleid, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 7371f95007..062985aacf 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.145 2005/11/28 23:46:03 tgl Exp $ + * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.146 2005/12/02 20:03:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -346,6 +346,15 @@ typedef struct EState } EState; +/* es_rowMarks is a list of these structs: */ +typedef struct ExecRowMark +{ + Relation relation; /* opened and RowShareLock'd relation */ + Index rti; /* its range table index */ + char resname[32]; /* name for its ctid junk attribute */ +} ExecRowMark; + + /* ---------------------------------------------------------------- * Tuple Hash Tables *