From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
Cc: | "Tharakan, Robins" <tharar(at)amazon(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: track_planning causing performance regression |
Date: | 2020-06-29 11:55:53 |
Message-ID: | CAOBaU_awdntO+cofrvDJarKhTC6_43A4Kmy2ohLMwqSNmY7k+w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jun 29, 2020 at 1:14 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
> On 2020/06/29 18:56, Fujii Masao wrote:
> >
> >
> > On 2020/06/29 18:53, Julien Rouhaud wrote:
> >> On Mon, Jun 29, 2020 at 11:38 AM Fujii Masao
> >> <masao(dot)fujii(at)oss(dot)nttdata(dot)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.
> >
> > Agreed. +1 to change that default to off.
>
> Attached patch does this.
Patch looks good to me.
> I also add the following into the description about each *_plan_time column
> in the docs. IMO this is helpful for users when they see that those columns
> report zero by default and try to understand why.
>
> (if <varname>pg_stat_statements.track_planning</varname> is enabled, otherwise zero)
+1
Do you intend to wait for other input before pushing? FWIW I'm still
not convinced that the exposed problem is representative of any
realistic workload. I of course entirely agree with the other
documentation changes.
From | Date | Subject | |
---|---|---|---|
Next Message | Dmitry Dolgov | 2020-06-29 12:07:09 | Re: Index Skip Scan (new UniqueKeys) |
Previous Message | Robert Haas | 2020-06-29 11:52:46 | Re: tar-related code in PostgreSQL |