Message-ID: <4ACA6EDE.4020407@enterprisedb.com>
Date: Tue, 06 Oct 2009 01:10:38 +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 <pgsql-hackers@postgresql.org>
Subject: Re: Hot Standby on git
References: <1253958559.4449.833.camel@ebony.2ndQuadrant>	
	<4ABF4522.6060601@enterprisedb.com>
	<1254743455.4691.153.camel@ebony.2ndQuadrant>
In-Reply-To: <1254743455.4691.153.camel@ebony.2ndQuadrant>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Simon Riggs wrote:
> We discussed briefly your change 
> 0011-Replace-per-proc-counters-of-loggable-locks-with-per.patch.
> 
> I don't see how that helps at all. The objective of lock counters was to
> know if we can skip acquiring an LWlock on all lock partitions. This
> change keeps the lock counters yet acquires the locks we were trying to
> avoid. This change needs some justification since it is not a bug fix.

Well, the original code was buggy. But more to the point, it's a lot
simpler this way, I don't see any reason why the counters should be
per-process, meaning that they need to be exposed in the pgproc structs
or procarray.c.

The point is to avoid the seqscan of the lock hash table. I presumed
that's the expensive part in GetRunningTransactionLocks().

Tom Lane wrote:
> [ scratches head ... ]  Why is hot standby messing with this sort of
> thing at all?  It sounds like a performance optimization that should
> be considered separately, and *later*.

Yeah, I too considered just ripping it out. Simon is worried that
locking all the lock partitions and scanning the locks table can take a
long time. We do that in the master, while holding both ProcArrayLock
and XidGenLock in exclusive mode (hmm, why is shared not enough?), so
there is some grounds for worry. OTOH, it's only done once per checkpoint.

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