Date: Fri, 11 Sep 2020 19:10:05 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: bttanakahbk <bttanakahbk@oss.nttdata.com>,
	Hamid Akhtar <hamid.akhtar@gmail.com>,
	Pavel Stehule <pavel.stehule@gmail.com>,
	Peter Geoghegan <pg@bowt.ie>, Julien Rouhaud <rjuju123@gmail.com>,
	"Tharakan, Robins" <tharar@amazon.com>,
	pgsql-hackers@postgresql.org
Subject: Re: track_planning causing performance regression
Message-ID: <20200911221005.GA6740@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <af5d442d-1abe-a3da-ee5f-437edc770fe7@oss.nttdata.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Sep-11, Fujii Masao wrote:

> Ok, so my proposed patch degrated the performance in this case :(
> This means that replacing spinlock with lwlock in pgss is not proper
> approach for the lock contention issue on pgss...
> 
> I proposed to split the spinlock for each pgss entry into two
> to reduce the lock contention, upthread. One is for planner stats,
> and the other is for executor stats. Is it worth working on
> this approach as an alternative idea? Or does anyone have any better idea?

It does seem that the excl-locked section in pgss_store is rather large.
(I admit I don't understand why would a LWLock decrease performance.)

Andres suggested in [1] to use atomics for the counters together with a
single lwlock to be used in shared mode only.  I didn't quite understand
what the lwlock is *for*, but maybe you do.

[1] https://postgr.es/m/20200629231015.qlej5b3qpfe4uijo@alap3.anarazel.de

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services