On Mon, 2010-04-19 at 10:55 -0400, Tom Lane wrote:
> Simon Riggs <si...@2ndquadrant.com> writes:
> > On Mon, 2010-04-19 at 17:44 +0300, Heikki Linnakangas wrote:
> >>> Choices are
> >>> 1. Check RecoveryInProgress() once outside of lock, plus wild rumour of
> >>> Murphy
> >>> 2. Check RecoveryInProgress() before and after holding lock
> >>> 3. Check RecoveryInProgress() while holding lock
> >>
> >> 4. Check RecoveryInProgress() once outside of lock, and scan the
> >> ProcArray anyway, just in case. That's what we did before this patch.
> >> Document that takenDuringRecovery == true means that the snapshot was
> >> most likely taken during recovery, but there is some race conditions
> >> where takenDuringRecovery is true even though the snapshot was taken
> >> just after recovery finished. AFAICS all of the other current uses of
> >> takenDuringRecovery work fine with that.
>
> > Checking RecoveryInProgress() is much cheaper than scanning the whole
> > ProcArray, so (4) is definitely worse than 1-3.
>
> If the lock we're talking about is an LWLock, #3 is okay. If it's a
> spinlock, not so much.
Just committed the following patch to implement option #3.
We test RecoveryInProgress() after the LWLockAcquire rather than before.
--
Simon Riggs www.2ndQuadrant.com
*** a/src/backend/storage/ipc/procarray.c
--- b/src/backend/storage/ipc/procarray.c
***************
*** 1074,1081 **** GetSnapshotData(Snapshot snapshot)
errmsg("out of memory")));
}
- snapshot->takenDuringRecovery = RecoveryInProgress();
-
/*
* It is sufficient to get shared lock on ProcArrayLock, even if we are
* going to set MyProc->xmin.
--- 1074,1079 ----
***************
*** 1091,1098 **** GetSnapshotData(Snapshot snapshot)
globalxmin = xmin = xmax;
/*
! * If in recovery get any known assigned xids.
*/
if (!snapshot->takenDuringRecovery)
{
/*
--- 1089,1103 ----
globalxmin = xmin = xmax;
/*
! * If we're in recovery then snapshot data comes from a different place,
! * so decide which route we take before grab the lock. It is possible
! * for recovery to end before we finish taking snapshot, and for newly
! * assigned transaction ids to be added to the procarray. Xmax cannot
! * change while we hold ProcArrayLock, so those newly added transaction
! * ids would be filtered away, so we need not be concerned about them.
*/
+ snapshot->takenDuringRecovery = RecoveryInProgress();
+
if (!snapshot->takenDuringRecovery)
{
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
