From: Simon Riggs <simon@2ndQuadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop
Date: Mon, 19 Apr 2010 19:17:56 +0100
Message-ID: <1271701076.8305.19751.camel@ebony> (raw)
In-Reply-To: <14068.1271688932@sss.pgh.pa.us>
References: <20100418180608.16F067541D1@cvs.postgresql.org>
<4BCC07F1.6040405@enterprisedb.com>
<1271664332.8305.17430.camel@ebony>
<4BCC1644.70008@enterprisedb.com>
<1271667922.8305.17650.camel@ebony>
<v2y603c8f071004190455gf18633fbx1a666c17c998b1a1@mail.gmail.com>
<13439.1271687068@sss.pgh.pa.us>
<1271687489.8305.18879.camel@ebony>
<4BCC6C37.9090703@enterprisedb.com>
<1271688445.8305.18966.camel@ebony>
<14068.1271688932@sss.pgh.pa.us>
On Mon, 2010-04-19 at 10:55 -0400, Tom Lane wrote:
> Simon Riggs <simon@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
Attachments:switcheroo.patch (text/x-patch, 1.2K) download
*** 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)
{
/*
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: simon@2ndQuadrant.com, tgl@sss.pgh.pa.us, heikki.linnakangas@enterprisedb.com, robertmhaas@gmail.com
Subject: Re: Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop
In-Reply-To: <1271701076.8305.19751.camel@ebony>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox