From f01e3d3a414f909762ab840719aeb8ae291aab5a Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 29 Jun 2011 21:43:53 +0300 Subject: [PATCH] Move the PredicateLockRelation() call from nodeSeqscan.c to heapam.c. It's more consistent that way, since all the other PredicateLock* calls are made in various heapam.c and index AM functions. The call in nodeSeqscan.c was unnecessarily aggressive anyway, there's no need to try to lock the relation every time a tuple is fetched, it's enough to do it once. This has the user-visible effect that if a seq scan is initialized in the executor, but never executed, we now acquire the predicate lock on the heap relation anyway. We could avoid that by taking the lock on the first heap_getnext() call instead, but it doesn't seem worth the trouble given that it feels more natural to do it in heap_beginscan(). Also, remove the retail PredicateLockTuple() calls from heap_getnext(). In a seqscan, started with heap_begin(), we're holding a whole-relation predicate lock on the heap so there's no need to lock the tuples individually. Kevin Grittner and me --- src/backend/access/heap/heapam.c | 21 ++++++++++++++------- src/backend/executor/nodeSeqscan.c | 6 ------ src/include/access/relscan.h | 1 - 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index b947c11f7d..450bb15ff8 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -479,8 +479,6 @@ heapgettup(HeapScanDesc scan, if (valid) { - if (!scan->rs_relpredicatelocked) - PredicateLockTuple(scan->rs_rd, tuple, snapshot); LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK); return; } @@ -748,16 +746,12 @@ heapgettup_pagemode(HeapScanDesc scan, nkeys, key, valid); if (valid) { - if (!scan->rs_relpredicatelocked) - PredicateLockTuple(scan->rs_rd, tuple, scan->rs_snapshot); scan->rs_cindex = lineindex; return; } } else { - if (!scan->rs_relpredicatelocked) - PredicateLockTuple(scan->rs_rd, tuple, scan->rs_snapshot); scan->rs_cindex = lineindex; return; } @@ -1224,13 +1218,26 @@ heap_beginscan_internal(Relation relation, Snapshot snapshot, scan->rs_strategy = NULL; /* set in initscan */ scan->rs_allow_strat = allow_strat; scan->rs_allow_sync = allow_sync; - scan->rs_relpredicatelocked = false; /* * we can use page-at-a-time mode if it's an MVCC-safe snapshot */ scan->rs_pageatatime = IsMVCCSnapshot(snapshot); + /* + * For a seqscan in a serializable transaction, acquire a predicate lock + * on the entire relation. This is required not only to lock all the + * matching tuples, but also to conflict with new insertions into the + * table. In an indexscan, we take page locks on the index pages covering + * the range specified in the scan qual, but in a heap scan there is + * nothing more fine-grained to lock. A bitmap scan is a different story, + * there we have already scanned the index and locked the index pages + * covering the predicate. But in that case we still have to lock any + * matching heap tuples. + */ + if (!is_bitmapscan) + PredicateLockRelation(relation, snapshot); + /* we only need to set this up once */ scan->rs_ctup.t_tableOid = RelationGetRelid(relation); diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c index f356874b44..0f3438d063 100644 --- a/src/backend/executor/nodeSeqscan.c +++ b/src/backend/executor/nodeSeqscan.c @@ -28,7 +28,6 @@ #include "access/relscan.h" #include "executor/execdebug.h" #include "executor/nodeSeqscan.h" -#include "storage/predicate.h" static void InitScanRelation(SeqScanState *node, EState *estate); static TupleTableSlot *SeqNext(SeqScanState *node); @@ -106,16 +105,11 @@ SeqRecheck(SeqScanState *node, TupleTableSlot *slot) * tuple. * We call the ExecScan() routine and pass it the appropriate * access method functions. - * For serializable transactions, we first acquire a predicate - * lock on the entire relation. * ---------------------------------------------------------------- */ TupleTableSlot * ExecSeqScan(SeqScanState *node) { - PredicateLockRelation(node->ss_currentRelation, - node->ss_currentScanDesc->rs_snapshot); - node->ss_currentScanDesc->rs_relpredicatelocked = true; return ExecScan((ScanState *) node, (ExecScanAccessMtd) SeqNext, (ExecScanRecheckMtd) SeqRecheck); diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h index 7663033723..ee60ca82ae 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -35,7 +35,6 @@ typedef struct HeapScanDescData BlockNumber rs_startblock; /* block # to start at */ BufferAccessStrategy rs_strategy; /* access strategy for reads */ bool rs_syncscan; /* report location to syncscan logic? */ - bool rs_relpredicatelocked; /* predicate lock on relation exists */ /* scan current state */ bool rs_inited; /* false = scan not init'd yet */ -- 2.40.0