Re: track_planning causing performance regression
Julien Rouhaud <rjuju123@gmail.com>
From: Julien Rouhaud <rjuju123@gmail.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: "Tharakan, Robins" <tharar@amazon.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2020-06-29T09:53:27Z
Lists: pgsql-hackers
On Mon, Jun 29, 2020 at 11:38 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > >> Your benchmark result seems to suggest that the cause of the problem is > >> the contention of per-query spinlock in pgss_store(). Right? > >> This lock contention is likely to happen when multiple sessions run > >> the same queries. > >> > >> One idea to reduce that lock contention is to separate per-query spinlock > >> into two; one is for planning, and the other is for execution. pgss_store() > >> determines which lock to use based on the given "kind" argument. > >> To make this idea work, also every pgss counters like shared_blks_hit > >> need to be separated into two, i.e., for planning and execution. > > > > This can probably remove some overhead, but won't it eventually hit > > the same issue when multiple connections try to plan the same query, > > given the number of different queries and very low execution runtime? > > Yes. But maybe we can expect that the idea would improve > the performance to the near same level as v12? A POC patch should be easy to do and see how much it solves this problem. However I'm not able to reproduce the issue, and IMHO unless we specifically want to be able to distinguish planner-time counters from execution-time counters, I'd prefer to disable track_planning by default than going this way, so that users with a sane usage won't have to suffer from a memory increase. > > I'm wondering if we could instead use atomics to store the counters. > > The only downside is that we won't guarantee per-row consistency > > anymore, which may be problematic. > > Yeah, we can consider more improvements against this issue. > But I'm afraid these (maybe including my idea) basically should > be items for v14... Yes, that's clearly not something I'd vote to push in v13 at this point.
Commits
-
doc: Fix description about pg_stat_statements.track_planning.
- 306c5e05e20f 13.4 landed
- e48f2afee631 14.0 landed
- 9d2a7757347c 15.0 landed
-
doc: Add note about possible performance overhead by enabling track_planning.
- da6b6ff95bca 13.0 landed
- 321fa6a4a26c 14.0 landed
-
Change default of pg_stat_statements.track_planning to off.
- 8d459762b103 13.0 landed
- d1763ea8c9c3 14.0 landed