Skip to content

Commit d780f07

Browse files
tglsfdctglsfdc
committed
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.
1 parent 5ab2598 commit d780f07

File tree

9 files changed

+135
-92
lines changed

9 files changed

+135
-92
lines changed

‎src/backend/executor/execMain.c

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
*
2828
* IDENTIFICATION
29-
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.261 2005/11/22 18:17:10 momjian Exp $
29+
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.262 2005/12/02 20:03:40 tgl Exp $
3030
*
3131
*-------------------------------------------------------------------------
3232
*/
@@ -52,13 +52,6 @@
5252
#include "utils/memutils.h"
5353

5454

55-
typedef struct execRowMark
56-
{
57-
Relation relation;
58-
Index rti;
59-
char resname[32];
60-
} execRowMark;
61-
6255
typedef struct evalPlanQual
6356
{
6457
Index rti;
@@ -567,10 +560,10 @@ InitPlan(QueryDesc *queryDesc, bool explainOnly)
567560
Index rti = lfirst_int(l);
568561
Oid relid = getrelid(rti, rangeTable);
569562
Relation relation;
570-
execRowMark *erm;
563+
ExecRowMark *erm;
571564

572565
relation = heap_open(relid, RowShareLock);
573-
erm = (execRowMark *) palloc(sizeof(execRowMark));
566+
erm = (ExecRowMark *) palloc(sizeof(ExecRowMark));
574567
erm->relation = relation;
575568
erm->rti = rti;
576569
snprintf(erm->resname, sizeof(erm->resname), "ctid%u", rti);
@@ -1020,7 +1013,7 @@ ExecEndPlan(PlanState *planstate, EState *estate)
10201013
*/
10211014
foreach(l, estate->es_rowMarks)
10221015
{
1023-
execRowMark *erm = lfirst(l);
1016+
ExecRowMark *erm = lfirst(l);
10241017

10251018
heap_close(erm->relation, NoLock);
10261019
}
@@ -1165,7 +1158,7 @@ lnext: ;
11651158
lmark: ;
11661159
foreach(l, estate->es_rowMarks)
11671160
{
1168-
execRowMark *erm = lfirst(l);
1161+
ExecRowMark *erm = lfirst(l);
11691162
HeapTupleData tuple;
11701163
Buffer buffer;
11711164
ItemPointerData update_ctid;
@@ -1859,9 +1852,9 @@ EvalPlanQual(EState *estate, Index rti,
18591852
relation = NULL;
18601853
foreach(l, estate->es_rowMarks)
18611854
{
1862-
if (((execRowMark *) lfirst(l))->rti == rti)
1855+
if (((ExecRowMark *) lfirst(l))->rti == rti)
18631856
{
1864-
relation = ((execRowMark *) lfirst(l))->relation;
1857+
relation = ((ExecRowMark *) lfirst(l))->relation;
18651858
break;
18661859
}
18671860
}

‎src/backend/executor/execUtils.c

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.129 2005/11/23 20:27:57 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.130 2005/12/02 20:03:40 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -24,6 +24,9 @@
2424
* ExecAssignResultType
2525
* etc
2626
*
27+
* ExecOpenScanRelation Common code for scan node init routines.
28+
* ExecCloseScanRelation
29+
*
2730
* ExecOpenIndices \
2831
* ExecCloseIndices | referenced by InitPlan, EndPlan,
2932
* ExecInsertIndexTuples / ExecInsert, ExecUpdate
@@ -45,6 +48,7 @@
4548
#include "catalog/pg_index.h"
4649
#include "executor/execdebug.h"
4750
#include "miscadmin.h"
51+
#include "parser/parsetree.h"
4852
#include "utils/builtins.h"
4953
#include "utils/fmgroids.h"
5054
#include "utils/memutils.h"
@@ -684,6 +688,90 @@ ExecAssignScanTypeFromOuterPlan(ScanState *scanstate)
684688
}
685689

686690

691+
/* ----------------------------------------------------------------
692+
* Scan node support
693+
* ----------------------------------------------------------------
694+
*/
695+
696+
/* ----------------------------------------------------------------
697+
* ExecOpenScanRelation
698+
*
699+
* Open the heap relation to be scanned by a base-level scan plan node.
700+
* This should be called during the node's ExecInit routine.
701+
*
702+
* By default, this acquires AccessShareLock on the relation. However,
703+
* if the relation was already locked by InitPlan, we don't need to acquire
704+
* any additional lock. This saves trips to the shared lock manager.
705+
* ----------------------------------------------------------------
706+
*/
707+
Relation
708+
ExecOpenScanRelation(EState *estate, Index scanrelid)
709+
{
710+
RangeTblEntry *rtentry;
711+
Oid reloid;
712+
LOCKMODE lockmode;
713+
ResultRelInfo *resultRelInfos;
714+
int i;
715+
716+
/*
717+
* First determine the lock type we need. Scan to see if target relation
718+
* is either a result relation or a FOR UPDATE/FOR SHARE relation.
719+
*/
720+
lockmode = AccessShareLock;
721+
resultRelInfos = estate->es_result_relations;
722+
for (i = 0; i < estate->es_num_result_relations; i++)
723+
{
724+
if (resultRelInfos[i].ri_RangeTableIndex == scanrelid)
725+
{
726+
lockmode = NoLock;
727+
break;
728+
}
729+
}
730+
731+
if (lockmode == AccessShareLock)
732+
{
733+
ListCell *l;
734+
735+
foreach(l, estate->es_rowMarks)
736+
{
737+
ExecRowMark *erm = lfirst(l);
738+
739+
if (erm->rti == scanrelid)
740+
{
741+
lockmode = NoLock;
742+
break;
743+
}
744+
}
745+
}
746+
747+
/* OK, open the relation and acquire lock as needed */
748+
rtentry = rt_fetch(scanrelid, estate->es_range_table);
749+
reloid = rtentry->relid;
750+
751+
return heap_open(reloid, lockmode);
752+
}
753+
754+
/* ----------------------------------------------------------------
755+
* ExecCloseScanRelation
756+
*
757+
* Close the heap relation scanned by a base-level scan plan node.
758+
* This should be called during the node's ExecEnd routine.
759+
*
760+
* Currently, we do not release the lock acquired by ExecOpenScanRelation.
761+
* This lock should be held till end of transaction. (There is a faction
762+
* that considers this too much locking, however.)
763+
*
764+
* If we did want to release the lock, we'd have to repeat the logic in
765+
* ExecOpenScanRelation in order to figure out what to release.
766+
* ----------------------------------------------------------------
767+
*/
768+
void
769+
ExecCloseScanRelation(Relation scanrel)
770+
{
771+
heap_close(scanrel, NoLock);
772+
}
773+
774+
687775
/* ----------------------------------------------------------------
688776
* ExecInsertIndexTuples support
689777
* ----------------------------------------------------------------
@@ -760,7 +848,7 @@ ExecOpenIndices(ResultRelInfo *resultRelInfo)
760848
*
761849
* If the index AM is not safe for concurrent updates, obtain an
762850
* exclusive lock on the index to lock out other updaters as well as
763-
* readers (index_beginscan places AccessShareLock).
851+
* readers (index_beginscan places AccessShareLock on the index).
764852
*
765853
* If there are multiple not-concurrent-safe indexes, all backends
766854
* must lock the indexes in the same order or we will get deadlocks

‎src/backend/executor/nodeBitmapHeapscan.c

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
*
2222
*
2323
* IDENTIFICATION
24-
* $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.7 2005/12/02 01:29:55 tgl Exp $
24+
* $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.8 2005/12/02 20:03:40 tgl Exp $
2525
*
2626
*-------------------------------------------------------------------------
2727
*/
@@ -141,9 +141,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
141141

142142
/*
143143
* Ignore any claimed entries past what we think is the end of the
144-
* relation. (This is probably not necessary given that we got
145-
* AccessShareLock before performing any of the indexscans, but
146-
* let's be safe.)
144+
* relation. (This is probably not necessary given that we got at
145+
* least AccessShareLock on the table before performing any of the
146+
* indexscans, but let's be safe.)
147147
*/
148148
if (tbmres->blockno >= scan->rs_nblocks)
149149
{
@@ -448,13 +448,8 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
448448

449449
/*
450450
* close the heap relation.
451-
*
452-
* Currently, we do not release the AccessShareLock acquired by
453-
* ExecInitBitmapHeapScan. This lock should be held till end of
454-
* transaction. (There is a faction that considers this too much locking,
455-
* however.)
456451
*/
457-
heap_close(relation, NoLock);
452+
ExecCloseScanRelation(relation);
458453
}
459454

460455
/* ----------------------------------------------------------------
@@ -467,9 +462,6 @@ BitmapHeapScanState *
467462
ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate)
468463
{
469464
BitmapHeapScanState *scanstate;
470-
RangeTblEntry *rtentry;
471-
Index relid;
472-
Oid reloid;
473465
Relation currentRelation;
474466

475467
/*
@@ -519,13 +511,9 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate)
519511
CXT1_printf("ExecInitBitmapHeapScan: context is %d\n", CurrentMemoryContext);
520512

521513
/*
522-
* open the base relation and acquire AccessShareLock on it.
514+
* open the base relation and acquire appropriate lock on it.
523515
*/
524-
relid = node->scan.scanrelid;
525-
rtentry = rt_fetch(relid, estate->es_range_table);
526-
reloid = rtentry->relid;
527-
528-
currentRelation = heap_open(reloid, AccessShareLock);
516+
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
529517

530518
scanstate->ss.ss_currentRelation = currentRelation;
531519

‎src/backend/executor/nodeBitmapIndexscan.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.12 2005/11/25 19:47:49 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.13 2005/12/02 20:03:40 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -286,8 +286,8 @@ ExecInitBitmapIndexScan(BitmapIndexScan *node, EState *estate)
286286

287287
/*
288288
* We do not open or lock the base relation here. We assume that an
289-
* ancestor BitmapHeapScan node is holding AccessShareLock on the heap
290-
* relation throughout the execution of the plan tree.
289+
* ancestor BitmapHeapScan node is holding AccessShareLock (or better)
290+
* on the heap relation throughout the execution of the plan tree.
291291
*/
292292

293293
indexstate->ss.ss_currentRelation = NULL;

‎src/backend/executor/nodeIndexscan.c

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.107 2005/11/25 19:47:49 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.108 2005/12/02 20:03:40 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -421,12 +421,8 @@ ExecEndIndexScan(IndexScanState *node)
421421

422422
/*
423423
* close the heap relation.
424-
*
425-
* Currently, we do not release the AccessShareLock acquired by
426-
* ExecInitIndexScan. This lock should be held till end of transaction.
427-
* (There is a faction that considers this too much locking, however.)
428424
*/
429-
heap_close(relation, NoLock);
425+
ExecCloseScanRelation(relation);
430426
}
431427

432428
/* ----------------------------------------------------------------
@@ -464,9 +460,6 @@ IndexScanState *
464460
ExecInitIndexScan(IndexScan *node, EState *estate)
465461
{
466462
IndexScanState *indexstate;
467-
RangeTblEntry *rtentry;
468-
Index relid;
469-
Oid reloid;
470463
Relation currentRelation;
471464

472465
/*
@@ -551,13 +544,9 @@ ExecInitIndexScan(IndexScan *node, EState *estate)
551544
}
552545

553546
/*
554-
* open the base relation and acquire AccessShareLock on it.
547+
* open the base relation and acquire appropriate lock on it.
555548
*/
556-
relid = node->scan.scanrelid;
557-
rtentry = rt_fetch(relid, estate->es_range_table);
558-
reloid = rtentry->relid;
559-
560-
currentRelation = heap_open(reloid, AccessShareLock);
549+
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
561550

562551
indexstate->ss.ss_currentRelation = currentRelation;
563552
indexstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */
@@ -570,7 +559,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate)
570559
/*
571560
* open the index relation and initialize relation and scan descriptors.
572561
* Note we acquire no locks here; the index machinery does its own locks
573-
* and unlocks. (We rely on having AccessShareLock on the parent table to
562+
* and unlocks. (We rely on having a lock on the parent table to
574563
* ensure the index won't go away!)
575564
*/
576565
indexstate->iss_RelationDesc = index_open(node->indexid);

‎src/backend/executor/nodeSeqscan.c

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeSeqscan.c,v 1.55 2005/11/25 04:24:48 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeSeqscan.c,v 1.56 2005/12/02 20:03:40 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -141,25 +141,15 @@ ExecSeqScan(SeqScanState *node)
141141
static void
142142
InitScanRelation(SeqScanState *node, EState *estate)
143143
{
144-
Index relid;
145-
List *rangeTable;
146-
RangeTblEntry *rtentry;
147-
Oid reloid;
148144
Relation currentRelation;
149145
HeapScanDesc currentScanDesc;
150146

151147
/*
152148
* get the relation object id from the relid'th entry in the range table,
153-
* open that relation and initialize the scan state.
154-
*
155-
* We acquire AccessShareLock for the duration of the scan.
149+
* open that relation and acquire appropriate lock on it.
156150
*/
157-
relid = ((SeqScan *) node->ps.plan)->scanrelid;
158-
rangeTable = estate->es_range_table;
159-
rtentry = rt_fetch(relid, rangeTable);
160-
reloid = rtentry->relid;
161-
162-
currentRelation = heap_open(reloid, AccessShareLock);
151+
currentRelation = ExecOpenScanRelation(estate,
152+
((SeqScan *) node->ps.plan)->scanrelid);
163153

164154
currentScanDesc = heap_beginscan(currentRelation,
165155
estate->es_snapshot,
@@ -281,12 +271,8 @@ ExecEndSeqScan(SeqScanState *node)
281271

282272
/*
283273
* close the heap relation.
284-
*
285-
* Currently, we do not release the AccessShareLock acquired by
286-
* InitScanRelation. This lock should be held till end of transaction.
287-
* (There is a faction that considers this too much locking, however.)
288274
*/
289-
heap_close(relation, NoLock);
275+
ExecCloseScanRelation(relation);
290276
}
291277

292278
/* ----------------------------------------------------------------

0 commit comments

Comments
 (0)