Re: [HACKERS] Planning counters in pg_stat_statements - Mailing list pgsql-hackers

From Thomas Munro
Subject Re: [HACKERS] Planning counters in pg_stat_statements
Date
Msg-id CAEepm=2j+GmL3cxNbDatgMU+TAuK18xbwfxzOnP7VYdEHAHF0A@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Planning counters in pg_stat_statements  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] Planning counters in pg_stat_statements
List pgsql-hackers
On Sat, Jan 13, 2018 at 2:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
>> Haribabu Kommi <kommi.haribabu@gmail.com> writes:
>>> I checked the latest patch and it is working fine and I don't have any
>>> further comments. Marked the patch as "ready for committer".
>
>> I started to look at this patch,

Thanks!

> ... looking further, I'm really seriously unhappy about this bit:
>
> @@ -943,9 +1006,16 @@ pgss_ExecutorEnd(QueryDesc *queryDesc)
> ...
> +
> +               /*
> +                * Clear planning_time, so that we only count it once for each
> +                * replanning of a prepared statement.
> +                */
> +               queryDesc->plannedstmt->planning_time = 0;
>         }
>
> What we have here is pgss_ExecutorEnd stomping on the plan cache.
> I do *not* find that acceptable.  At the very least, it ruins the
> theory that this field is a shared resource.

The idea was that planning_time is work space for extensions to do
whatever they like with, but objection noted.

> What we could/should do instead, I think, is have pgss_planner_hook
> make its own pgss_store() call to log the planning time results
> (which would mean we don't need the added PlannedStmt field at all).
> That would increase the overhead of this feature, which might mean
> that it'd be worth having a pg_stat_statements GUC to enable it.
> But it'd be a whole lot cleaner.

Ok.  It seems a bit strange to have to go through the locking twice,
but I don't have a better idea.  First attempt seems to be working.
I'll post an updated patch in a couple of days when I'm back from
travelling and have a tidier version with a new GUC and have thought
about the nested_level question.

-- 
Thomas Munro
http://www.enterprisedb.com


pgsql-hackers by date:

Previous
From: hubert depesz lubaczewski
Date:
Subject: Re: Is there a "right" way to test if a database is empty?
Next
From: Michael Paquier
Date:
Subject: Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least9.5)?