On Mon, Jan 02, 2012 at 06:41:31PM +0000, Simon Riggs wrote:
> On Mon, Jan 2, 2012 at 6:06 PM, Noah Misch <n...@leadboat.com> wrote:
> > On Mon, Jan 02, 2012 at 05:09:16PM +0000, Simon Riggs wrote:
> >> Attached patch makes SnapshotNow into an MVCC snapshot, initialised at
> >> the start of each scan iff SnapshotNow is passed as the scan's
> >> snapshot. It's fairly brief but seems to do the trick.
> >
> > That's a neat trick. ?However, if you start a new SnapshotNow scan while
> > one is
> > ongoing, the primordial scan's snapshot will change mid-stream.
>
> Do we ever do that? (and if so, Why?!? or perhaps just Where?)
I hacked up your patch a bit, as attached, to emit a WARNING for any nested
use of SnapshotNow. This made 97/127 test files fail. As one example,
RelationBuildRuleLock() does TextDatumGetCString() for every tuple of its
SnapshotNow scan. That may need a detoast, which itself runs a scan.
> We can use more complex code if required, but we'll be adding
> complexity and code into the main path that I'd like to avoid.
Agreed.
*** a/src/backend/access/heap/heapam.c
--- b/src/backend/access/heap/heapam.c
***************
*** 1201,1206 **** heap_beginscan_internal(Relation relation, Snapshot snapshot,
--- 1201,1208 ----
*/
scan->rs_pageatatime = IsMVCCSnapshot(snapshot);
+ InitSnapshotNowIfNeeded(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
***************
*** 1270,1275 **** heap_endscan(HeapScanDesc scan)
--- 1272,1279 ----
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
+ FinishSnapshotNow(scan->rs_snapshot);
+
/*
* decrement relation reference count and free scan descriptor storage
*/
***************
*** 1680,1685 **** heap_get_latest_tid(Relation relation,
--- 1684,1691 ----
if (!ItemPointerIsValid(tid))
return;
+ InitSnapshotNowIfNeeded(snapshot);
+
/*
* Since this can be called with user-supplied TID, don't trust the
input
* too much. (RelationGetNumberOfBlocks is an expensive check, so we
***************
*** 1775,1780 **** heap_get_latest_tid(Relation relation,
--- 1781,1788 ----
priorXmax = HeapTupleHeaderGetXmax(tp.t_data);
UnlockReleaseBuffer(buffer);
} /* end of loop
*/
+
+ FinishSnapshotNow(snapshot);
}
*** a/src/backend/access/index/indexam.c
--- b/src/backend/access/index/indexam.c
***************
*** 292,297 **** index_beginscan_internal(Relation indexRelation,
--- 292,299 ----
*/
RelationIncrementReferenceCount(indexRelation);
+ InitSnapshotNowIfNeeded(snapshot);
+
/*
* Tell the AM to open a scan.
*/
***************
*** 370,375 **** index_endscan(IndexScanDesc scan)
--- 372,379 ----
/* End the AM's scan */
FunctionCall1(procedure, PointerGetDatum(scan));
+ FinishSnapshotNow(scan->xs_snapshot);
+
/* Release index refcount acquired by index_beginscan */
RelationDecrementReferenceCount(scan->indexRelation);
*** a/src/backend/utils/time/snapmgr.c
--- b/src/backend/utils/time/snapmgr.c
***************
*** 205,210 **** GetLatestSnapshot(void)
--- 205,235 ----
return SecondarySnapshot;
}
+ static bool SnapshotNowActive;
+
+ /*
+ * InitSnapshotNowIfNeeded
+ * If passed snapshot is SnapshotNow then refresh it with latest
info.
+ */
+ void
+ InitSnapshotNowIfNeeded(Snapshot snap)
+ {
+ if (!IsSnapshotNow(snap))
+ return;
+
+ if (SnapshotNowActive)
+ elog(WARNING, "SnapshotNow used concurrently");
+
+ snap = GetSnapshotData(&SnapshotNowData);
+ SnapshotNowActive = true;
+ }
+
+ void FinishSnapshotNow(Snapshot snap)
+ {
+ if (IsSnapshotNow(snap))
+ SnapshotNowActive = false;
+ }
+
/*
* SnapshotSetCommandId
* Propagate CommandCounterIncrement into the static snapshots, if
set
*** a/src/backend/utils/time/tqual.c
--- b/src/backend/utils/time/tqual.c
***************
*** 67,73 ****
/* Static variables representing various special snapshot semantics */
! SnapshotData SnapshotNowData = {HeapTupleSatisfiesNow};
SnapshotData SnapshotSelfData = {HeapTupleSatisfiesSelf};
SnapshotData SnapshotAnyData = {HeapTupleSatisfiesAny};
SnapshotData SnapshotToastData = {HeapTupleSatisfiesToast};
--- 67,73 ----
/* Static variables representing various special snapshot semantics */
! SnapshotData SnapshotNowData = {HeapTupleSatisfiesMVCC};
SnapshotData SnapshotSelfData = {HeapTupleSatisfiesSelf};
SnapshotData SnapshotAnyData = {HeapTupleSatisfiesAny};
SnapshotData SnapshotToastData = {HeapTupleSatisfiesToast};
***************
*** 293,298 **** HeapTupleSatisfiesSelf(HeapTupleHeader tuple, Snapshot
snapshot, Buffer buffer)
--- 293,299 ----
return false;
}
+ #ifdef RECENTLY_DEAD_CODE
/*
* HeapTupleSatisfiesNow
* True iff heap tuple is valid "now".
***************
*** 474,479 **** HeapTupleSatisfiesNow(HeapTupleHeader tuple, Snapshot
snapshot, Buffer buffer)
--- 475,481 ----
HeapTupleHeaderGetXmax(tuple));
return false;
}
+ #endif
/*
* HeapTupleSatisfiesAny
*** a/src/include/utils/snapmgr.h
--- b/src/include/utils/snapmgr.h
***************
*** 24,29 **** extern TransactionId RecentGlobalXmin;
--- 24,31 ----
extern Snapshot GetTransactionSnapshot(void);
extern Snapshot GetLatestSnapshot(void);
+ extern void InitSnapshotNowIfNeeded(Snapshot snap);
+ extern void FinishSnapshotNow(Snapshot snap);
extern void SnapshotSetCommandId(CommandId curcid);
extern void PushActiveSnapshot(Snapshot snapshot);
*** a/src/include/utils/tqual.h
--- b/src/include/utils/tqual.h
***************
*** 40,45 **** extern PGDLLIMPORT SnapshotData SnapshotToastData;
--- 40,47 ----
/* This macro encodes the knowledge of which snapshots are MVCC-safe */
#define IsMVCCSnapshot(snapshot) \
((snapshot)->satisfies == HeapTupleSatisfiesMVCC)
+ #define IsSnapshotNow(snapshot) \
+ ((snapshot) == (&SnapshotNowData))
/*
* HeapTupleSatisfiesVisibility
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
