Skip to content

Commit a98871b

Browse files
tglsfdctglsfdc
committed
Tweak indexscan machinery to avoid taking an AccessShareLock on an index
if we already have a stronger lock due to the index's table being the update target table of the query. Same optimization I applied earlier at the table level. There doesn't seem to be much interest in the more radical idea of not locking indexes at all, so do what we can ...
1 parent 1cf6514 commit a98871b

File tree

12 files changed

+95
-35
lines changed

12 files changed

+95
-35
lines changed

‎src/backend/access/heap/tuptoaster.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.56 2005/11/22 18:17:06 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.57 2005/12/03 05:50:59 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1142,8 +1142,8 @@ toast_delete_datum(Relation rel, Datum value)
11421142
/*
11431143
* Find the chunks by index
11441144
*/
1145-
toastscan = index_beginscan(toastrel, toastidx, SnapshotToast,
1146-
1, &toastkey);
1145+
toastscan = index_beginscan(toastrel, toastidx, true,
1146+
SnapshotToast, 1, &toastkey);
11471147
while ((toasttup = index_getnext(toastscan, ForwardScanDirection)) != NULL)
11481148
{
11491149
/*
@@ -1219,8 +1219,8 @@ toast_fetch_datum(varattrib *attr)
12191219
*/
12201220
nextidx = 0;
12211221

1222-
toastscan = index_beginscan(toastrel, toastidx, SnapshotToast,
1223-
1, &toastkey);
1222+
toastscan = index_beginscan(toastrel, toastidx, true,
1223+
SnapshotToast, 1, &toastkey);
12241224
while ((ttup = index_getnext(toastscan, ForwardScanDirection)) != NULL)
12251225
{
12261226
/*
@@ -1394,8 +1394,8 @@ toast_fetch_datum_slice(varattrib *attr, int32 sliceoffset, int32 length)
13941394
* The index is on (valueid, chunkidx) so they will come in order
13951395
*/
13961396
nextidx = startchunk;
1397-
toastscan = index_beginscan(toastrel, toastidx, SnapshotToast,
1398-
nscankeys, toastkey);
1397+
toastscan = index_beginscan(toastrel, toastidx, true,
1398+
SnapshotToast, nscankeys, toastkey);
13991399
while ((ttup = index_getnext(toastscan, ForwardScanDirection)) != NULL)
14001400
{
14011401
/*

‎src/backend/access/index/genam.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.51 2005/11/22 18:17:06 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.52 2005/12/03 05:51:00 tgl Exp $
1212
*
1313
* NOTES
1414
* many of the old access method routines have been turned into
@@ -86,7 +86,8 @@ RelationGetIndexScan(Relation indexRelation,
8686
else
8787
scan->keyData = NULL;
8888

89-
scan->is_multiscan = false; /* caller may change this */
89+
scan->is_multiscan = false; /* caller may change this */
90+
scan->have_lock = false; /* ditto */
9091
scan->kill_prior_tuple = false;
9192
scan->ignore_killed_tuples = true; /* default setting */
9293
scan->keys_are_unique = false; /* may be set by index AM */
@@ -211,8 +212,8 @@ systable_beginscan(Relation heapRelation,
211212
key[i].sk_attno = i + 1;
212213
}
213214

214-
sysscan->iscan = index_beginscan(heapRelation, irel, snapshot,
215-
nkeys, key);
215+
sysscan->iscan = index_beginscan(heapRelation, irel, true,
216+
snapshot, nkeys, key);
216217
sysscan->scan = NULL;
217218
}
218219
else

‎src/backend/access/index/indexam.c

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.86 2005/10/15 02:49:09 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.87 2005/12/03 05:51:00 tgl Exp $
1212
*
1313
* INTERFACE ROUTINES
1414
* index_open - open an index relation by relation OID
@@ -111,6 +111,7 @@ do { \
111111
} while(0)
112112

113113
static IndexScanDesc index_beginscan_internal(Relation indexRelation,
114+
bool need_index_lock,
114115
int nkeys, ScanKey key);
115116

116117

@@ -229,16 +230,23 @@ index_insert(Relation indexRelation,
229230
* heapRelation link (nor the snapshot). However, the caller had better
230231
* be holding some kind of lock on the heap relation in any case, to ensure
231232
* no one deletes it (or the index) out from under us.
233+
*
234+
* Most callers should pass need_index_lock = true to cause the index code
235+
* to take AccessShareLock on the index for the duration of the scan. But
236+
* if it is known that a lock is already held on the index, pass false to
237+
* skip taking an unnecessary lock.
232238
*/
233239
IndexScanDesc
234240
index_beginscan(Relation heapRelation,
235241
Relation indexRelation,
242+
bool need_index_lock,
236243
Snapshot snapshot,
237244
int nkeys, ScanKey key)
238245
{
239246
IndexScanDesc scan;
240247

241-
scan = index_beginscan_internal(indexRelation, nkeys, key);
248+
scan = index_beginscan_internal(indexRelation, need_index_lock,
249+
nkeys, key);
242250

243251
/*
244252
* Save additional parameters into the scandesc. Everything else was set
@@ -259,12 +267,14 @@ index_beginscan(Relation heapRelation,
259267
*/
260268
IndexScanDesc
261269
index_beginscan_multi(Relation indexRelation,
270+
bool need_index_lock,
262271
Snapshot snapshot,
263272
int nkeys, ScanKey key)
264273
{
265274
IndexScanDesc scan;
266275

267-
scan = index_beginscan_internal(indexRelation, nkeys, key);
276+
scan = index_beginscan_internal(indexRelation, need_index_lock,
277+
nkeys, key);
268278

269279
/*
270280
* Save additional parameters into the scandesc. Everything else was set
@@ -281,6 +291,7 @@ index_beginscan_multi(Relation indexRelation,
281291
*/
282292
static IndexScanDesc
283293
index_beginscan_internal(Relation indexRelation,
294+
bool need_index_lock,
284295
int nkeys, ScanKey key)
285296
{
286297
IndexScanDesc scan;
@@ -291,13 +302,15 @@ index_beginscan_internal(Relation indexRelation,
291302
RelationIncrementReferenceCount(indexRelation);
292303

293304
/*
294-
* Acquire AccessShareLock for the duration of the scan
305+
* Acquire AccessShareLock for the duration of the scan, unless caller
306+
* says it already has lock on the index.
295307
*
296308
* Note: we could get an SI inval message here and consequently have to
297309
* rebuild the relcache entry. The refcount increment above ensures that
298310
* we will rebuild it and not just flush it...
299311
*/
300-
LockRelation(indexRelation, AccessShareLock);
312+
if (need_index_lock)
313+
LockRelation(indexRelation, AccessShareLock);
301314

302315
/*
303316
* LockRelation can clean rd_aminfo structure, so fill procedure after
@@ -315,6 +328,9 @@ index_beginscan_internal(Relation indexRelation,
315328
Int32GetDatum(nkeys),
316329
PointerGetDatum(key)));
317330

331+
/* Save flag to tell index_endscan whether to release lock */
332+
scan->have_lock = need_index_lock;
333+
318334
return scan;
319335
}
320336

@@ -380,7 +396,8 @@ index_endscan(IndexScanDesc scan)
380396

381397
/* Release index lock and refcount acquired by index_beginscan */
382398

383-
UnlockRelation(scan->indexRelation, AccessShareLock);
399+
if (scan->have_lock)
400+
UnlockRelation(scan->indexRelation, AccessShareLock);
384401

385402
RelationDecrementReferenceCount(scan->indexRelation);
386403

‎src/backend/catalog/catalog.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/catalog/catalog.c,v 1.64 2005/10/15 02:49:12 momjian Exp $
12+
* $PostgreSQL: pgsql/src/backend/catalog/catalog.c,v 1.65 2005/12/03 05:51:00 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -309,7 +309,8 @@ GetNewOidWithIndex(Relation relation, Relation indexrel)
309309
ObjectIdGetDatum(newOid));
310310

311311
/* see notes above about using SnapshotDirty */
312-
scan = index_beginscan(relation, indexrel, SnapshotDirty, 1, &key);
312+
scan = index_beginscan(relation, indexrel, true,
313+
SnapshotDirty, 1, &key);
313314

314315
collides = HeapTupleIsValid(index_getnext(scan, ForwardScanDirection));
315316

‎src/backend/commands/cluster.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
*
1313
* IDENTIFICATION
14-
* $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.142 2005/11/22 18:17:08 momjian Exp $
14+
* $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.143 2005/12/03 05:51:01 tgl Exp $
1515
*
1616
*-------------------------------------------------------------------------
1717
*/
@@ -643,7 +643,8 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex)
643643
* Scan through the OldHeap on the OldIndex and copy each tuple into the
644644
* NewHeap.
645645
*/
646-
scan = index_beginscan(OldHeap, OldIndex, SnapshotNow, 0, (ScanKey) NULL);
646+
scan = index_beginscan(OldHeap, OldIndex, true,
647+
SnapshotNow, 0, (ScanKey) NULL);
647648

648649
while ((tuple = index_getnext(scan, ForwardScanDirection)) != NULL)
649650
{

‎src/backend/executor/execUtils.c

Lines changed: 23 additions & 1 deletion
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.130 2005/12/02 20:03:40 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.131 2005/12/03 05:51:01 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -693,6 +693,28 @@ ExecAssignScanTypeFromOuterPlan(ScanState *scanstate)
693693
* ----------------------------------------------------------------
694694
*/
695695

696+
/* ----------------------------------------------------------------
697+
* ExecRelationIsTargetRelation
698+
*
699+
* Detect whether a relation (identified by rangetable index)
700+
* is one of the target relations of the query.
701+
* ----------------------------------------------------------------
702+
*/
703+
bool
704+
ExecRelationIsTargetRelation(EState *estate, Index scanrelid)
705+
{
706+
ResultRelInfo *resultRelInfos;
707+
int i;
708+
709+
resultRelInfos = estate->es_result_relations;
710+
for (i = 0; i < estate->es_num_result_relations; i++)
711+
{
712+
if (resultRelInfos[i].ri_RangeTableIndex == scanrelid)
713+
return true;
714+
}
715+
return false;
716+
}
717+
696718
/* ----------------------------------------------------------------
697719
* ExecOpenScanRelation
698720
*

‎src/backend/executor/nodeBitmapIndexscan.c

Lines changed: 10 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.13 2005/12/02 20:03:40 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.14 2005/12/03 05:51:01 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -214,6 +214,7 @@ BitmapIndexScanState *
214214
ExecInitBitmapIndexScan(BitmapIndexScan *node, EState *estate)
215215
{
216216
BitmapIndexScanState *indexstate;
217+
bool relistarget;
217218

218219
/*
219220
* create state structure
@@ -294,13 +295,19 @@ ExecInitBitmapIndexScan(BitmapIndexScan *node, EState *estate)
294295
indexstate->ss.ss_currentScanDesc = NULL;
295296

296297
/*
297-
* open the index relation and initialize relation and scan descriptors.
298+
* Open the index relation and initialize relation and scan descriptors.
298299
* Note we acquire no locks here; the index machinery does its own locks
299-
* and unlocks.
300+
* and unlocks. (We rely on having a lock on the parent table to
301+
* ensure the index won't go away!) Furthermore, if the parent table
302+
* is one of the target relations of the query, then InitPlan already
303+
* opened and write-locked the index, so we can tell the index machinery
304+
* not to bother getting an extra lock.
300305
*/
301306
indexstate->biss_RelationDesc = index_open(node->indexid);
307+
relistarget = ExecRelationIsTargetRelation(estate, node->scan.scanrelid);
302308
indexstate->biss_ScanDesc =
303309
index_beginscan_multi(indexstate->biss_RelationDesc,
310+
!relistarget,
304311
estate->es_snapshot,
305312
indexstate->biss_NumScanKeys,
306313
indexstate->biss_ScanKeys);

‎src/backend/executor/nodeIndexscan.c

Lines changed: 9 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/nodeIndexscan.c,v 1.108 2005/12/02 20:03:40 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.109 2005/12/03 05:51:02 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -461,6 +461,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate)
461461
{
462462
IndexScanState *indexstate;
463463
Relation currentRelation;
464+
bool relistarget;
464465

465466
/*
466467
* create state structure
@@ -557,14 +558,19 @@ ExecInitIndexScan(IndexScan *node, EState *estate)
557558
ExecAssignScanType(&indexstate->ss, RelationGetDescr(currentRelation), false);
558559

559560
/*
560-
* open the index relation and initialize relation and scan descriptors.
561+
* Open the index relation and initialize relation and scan descriptors.
561562
* Note we acquire no locks here; the index machinery does its own locks
562563
* and unlocks. (We rely on having a lock on the parent table to
563-
* ensure the index won't go away!)
564+
* ensure the index won't go away!) Furthermore, if the parent table
565+
* is one of the target relations of the query, then InitPlan already
566+
* opened and write-locked the index, so we can tell the index machinery
567+
* not to bother getting an extra lock.
564568
*/
565569
indexstate->iss_RelationDesc = index_open(node->indexid);
570+
relistarget = ExecRelationIsTargetRelation(estate, node->scan.scanrelid);
566571
indexstate->iss_ScanDesc = index_beginscan(currentRelation,
567572
indexstate->iss_RelationDesc,
573+
!relistarget,
568574
estate->es_snapshot,
569575
indexstate->iss_NumScanKeys,
570576
indexstate->iss_ScanKeys);

‎src/backend/storage/large_object/inv_api.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.113 2005/10/15 02:49:26 momjian Exp $
12+
* $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.114 2005/12/03 05:51:02 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -299,7 +299,7 @@ inv_getsize(LargeObjectDesc *obj_desc)
299299
BTEqualStrategyNumber, F_OIDEQ,
300300
ObjectIdGetDatum(obj_desc->id));
301301

302-
sd = index_beginscan(lo_heap_r, lo_index_r,
302+
sd = index_beginscan(lo_heap_r, lo_index_r, true,
303303
obj_desc->snapshot, 1, skey);
304304

305305
/*
@@ -410,7 +410,7 @@ inv_read(LargeObjectDesc *obj_desc, char *buf, int nbytes)
410410
BTGreaterEqualStrategyNumber, F_INT4GE,
411411
Int32GetDatum(pageno));
412412

413-
sd = index_beginscan(lo_heap_r, lo_index_r,
413+
sd = index_beginscan(lo_heap_r, lo_index_r, true,
414414
obj_desc->snapshot, 2, skey);
415415

416416
while ((tuple = index_getnext(sd, ForwardScanDirection)) != NULL)
@@ -526,7 +526,7 @@ inv_write(LargeObjectDesc *obj_desc, char *buf, int nbytes)
526526
BTGreaterEqualStrategyNumber, F_INT4GE,
527527
Int32GetDatum(pageno));
528528

529-
sd = index_beginscan(lo_heap_r, lo_index_r,
529+
sd = index_beginscan(lo_heap_r, lo_index_r, false /* got lock */,
530530
obj_desc->snapshot, 2, skey);
531531

532532
oldtuple = NULL;

‎src/include/access/genam.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/access/genam.h,v 1.53 2005/10/15 02:49:42 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/access/genam.h,v 1.54 2005/12/03 05:51:03 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -79,9 +79,11 @@ extern bool index_insert(Relation indexRelation,
7979

8080
extern IndexScanDesc index_beginscan(Relation heapRelation,
8181
Relation indexRelation,
82+
bool need_index_lock,
8283
Snapshot snapshot,
8384
int nkeys, ScanKey key);
8485
extern IndexScanDesc index_beginscan_multi(Relation indexRelation,
86+
bool need_index_lock,
8587
Snapshot snapshot,
8688
int nkeys, ScanKey key);
8789
extern void index_rescan(IndexScanDesc scan, ScanKey key);

0 commit comments

Comments
 (0)