Message-ID: <4BCC1644.70008@enterprisedb.com>
Date: Mon, 19 Apr 2010 11:37:24 +0300
From: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
Organization: EnterpriseDB
User-Agent: Mozilla-Thunderbird 2.0.0.22 (X11/20090706)
MIME-Version: 1.0
To: Simon Riggs <simon@2ndQuadrant.com>
CC: pgsql-hackers@postgresql.org
Subject: Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby
	by avoiding loop
References: <20100418180608.16F067541D1@cvs.postgresql.org>	
	<4BCC07F1.6040405@enterprisedb.com>
	<1271664332.8305.17430.camel@ebony>
In-Reply-To: <1271664332.8305.17430.camel@ebony>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Simon Riggs wrote:
> On Mon, 2010-04-19 at 10:36 +0300, Heikki Linnakangas wrote:
>> Simon Riggs wrote:
>>> Log Message:
>>> -----------
>>> Tune GetSnapshotData() during Hot Standby by avoiding loop
>>> through normal backends. Makes code clearer also, since we
>>> avoid various Assert()s. Performance of snapshots taken
>>> during recovery no longer depends upon number of read-only
>>> backends.
>> I think there's a little race condition there.
>> snapshot->takenDuringRecovery is set before acquiring ProcArrayLock, so
>> it's possible that by the time we acquire the lock, we're no longer in
>> recovery. So this can happen:
>>
>> 1. Backend A starts to take a snapshot, while we're still in recovery.
>> takenDuringRecovery is assigned true.
>> 2. Recovery ends, and a normal transaction X begins in backend B.
>> 3. A skips scanning ProcArray because takenDuringRecovery is true.
>>
>> The snapshot doesn't include X, so any changes done in that transaction
>> will not be visible to the snapshot while the transaction is still
>> running, but will be after it commits.
>>
>> Of course, it's highly improbable for 2. to happen, but it's there.
> 
> The order of events is as you say, though I don't see the problem. The
> new xids will be beyond xmax and would be filtered out even if we did
> scan the procs, so they will be treated as running, which they are. Xmax
> will not have advanced since that relies on completed transactions, not
> started ones.

Good point. However, it is theoretically possible that yet another
transaction starts and commits in that same window, bumping xmax.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com