I think the previous patch to snapmgr.c was mistaken. Instead of fixing
a single trouble spot, we're better off fixing PushActiveSnapshot so
that any use of it that involves a snapshot that's subject to a future
command counter update should create a new copy.
This is correct because the 8.3 code used to do CopySnapshot anytime it
was setting ActiveSnapshot. So we're not disrupting any behavior here
-- we're merely restoring what was the previous customary behavior.
The attached patch implements this idea. It reverts the code changes
done in the previous patch, because they're obviously no longer
necessary. The new regression test that it added still passes with this
new patch. I will add a new one for this new problem.
(This new patch restores CopySnapshot as a static function too).
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/commands/portalcmds.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/portalcmds.c,v
retrieving revision 1.79.2.1
diff -c -p -r1.79.2.1 portalcmds.c
*** src/backend/commands/portalcmds.c 2 Oct 2009 17:58:21 -0000 1.79.2.1
--- src/backend/commands/portalcmds.c 6 Oct 2009 17:10:34 -0000
*************** PerformCursorOpen(PlannedStmt *stmt, Par
*** 120,136 ****
}
/*
- * Set up snapshot for portal. Note that we need a fresh, independent copy
- * of the snapshot because we don't want it to be modified by future
- * CommandCounterIncrement calls. We do not register it, because
- * portalmem.c will take care of that internally.
- */
- snapshot = CopySnapshot(GetActiveSnapshot());
-
- /*
* Start execution, inserting parameters if any.
*/
! PortalStart(portal, params, snapshot);
Assert(portal->strategy == PORTAL_ONE_SELECT);
--- 120,128 ----
}
/*
* Start execution, inserting parameters if any.
*/
! PortalStart(portal, params, GetActiveSnapshot());
Assert(portal->strategy == PORTAL_ONE_SELECT);
Index: src/backend/utils/time/snapmgr.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/time/snapmgr.c,v
retrieving revision 1.10.2.1
diff -c -p -r1.10.2.1 snapmgr.c
*** src/backend/utils/time/snapmgr.c 2 Oct 2009 17:58:21 -0000 1.10.2.1
--- src/backend/utils/time/snapmgr.c 6 Oct 2009 17:13:48 -0000
*************** bool FirstSnapshotSet = false;
*** 104,109 ****
--- 104,110 ----
static bool registered_serializable = false;
+ static Snapshot CopySnapshot(Snapshot snapshot);
static void FreeSnapshot(Snapshot snapshot);
static void SnapshotResetXmin(void);
*************** SnapshotSetCommandId(CommandId curcid)
*** 191,197 ****
* The copy is palloc'd in TopTransactionContext and has initial refcounts set
* to 0. The returned snapshot has the copied flag set.
*/
! Snapshot
CopySnapshot(Snapshot snapshot)
{
Snapshot newsnap;
--- 192,198 ----
* The copy is palloc'd in TopTransactionContext and has initial refcounts set
* to 0. The returned snapshot has the copied flag set.
*/
! static Snapshot
CopySnapshot(Snapshot snapshot)
{
Snapshot newsnap;
*************** FreeSnapshot(Snapshot snapshot)
*** 254,261 ****
* PushActiveSnapshot
* Set the given snapshot as the current active snapshot
*
! * If this is the first use of this snapshot, create a new long-lived copy with
! * active refcount=1. Otherwise, only increment the refcount.
*/
void
PushActiveSnapshot(Snapshot snap)
--- 255,263 ----
* PushActiveSnapshot
* Set the given snapshot as the current active snapshot
*
! * If the passed snapshot is a statically-allocated one, or it is possibly
! * subject to a future command counter update, create a new long-lived copy
! * with active refcount=1. Otherwise, only increment the refcount.
*/
void
PushActiveSnapshot(Snapshot snap)
*************** PushActiveSnapshot(Snapshot snap)
*** 265,272 ****
Assert(snap != InvalidSnapshot);
newactive = MemoryContextAlloc(TopTransactionContext, sizeof(ActiveSnapshotElt));
! /* Static snapshot? Create a persistent copy */
! newactive->as_snap = snap->copied ? snap : CopySnapshot(snap);
newactive->as_next = ActiveSnapshot;
newactive->as_level = GetCurrentTransactionNestLevel();
--- 267,278 ----
Assert(snap != InvalidSnapshot);
newactive = MemoryContextAlloc(TopTransactionContext, sizeof(ActiveSnapshotElt));
!
! if (snap == CurrentSnapshot || snap == SecondarySnapshot || !snap->copied)
! newactive->as_snap = CopySnapshot(snap);
! else
! newactive->as_snap = snap;
!
newactive->as_next = ActiveSnapshot;
newactive->as_level = GetCurrentTransactionNestLevel();
Index: src/include/utils/snapmgr.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/utils/snapmgr.h,v
retrieving revision 1.5.2.1
diff -c -p -r1.5.2.1 snapmgr.h
*** src/include/utils/snapmgr.h 2 Oct 2009 17:58:21 -0000 1.5.2.1
--- src/include/utils/snapmgr.h 6 Oct 2009 17:13:56 -0000
*************** extern TransactionId RecentGlobalXmin;
*** 26,32 ****
extern Snapshot GetTransactionSnapshot(void);
extern Snapshot GetLatestSnapshot(void);
extern void SnapshotSetCommandId(CommandId curcid);
- extern Snapshot CopySnapshot(Snapshot snapshot);
extern void PushActiveSnapshot(Snapshot snapshot);
extern void PushUpdatedSnapshot(Snapshot snapshot);
--- 26,31 ----
--
Sent via pgsql-general mailing list (pgsql-general@xxxxxxxxxxxxxx)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general
