RE: minimizing pg_stat_statements performance overhead

From: Raymond Martin <ramarti(at)microsoft(dot)com>
To: legrand legrand <legrand_legrand(at)hotmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: RE: minimizing pg_stat_statements performance overhead
Date: 2019-04-04 00:19:28
Message-ID: BN8PR21MB1217D3F1BAF98ECB9795CC9FB1500@BN8PR21MB1217.namprd21.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

From: Robert Haas
>>
>> One thing that needs some thought here is what happens if the value of
>> pgss_enabled() changes. For example we don't want a situation where
>> if the value changes from off to on between one stage of processing
>> and another, the server crashes.
>>
>> I don't know whether that's a risk here or not; it's just something to
>> think about.
This is definitely an important consideration for this change. A hook could
have the implicit assumption that a query ID is always generated.

From: PAscal
> Hi, here is a simple test where I commented that line in pgss_post_parse_analyze
> to force return; (as if pgss_enabled() was disabled) but kept pgss_enabled() every where else
>
> /* Safety check... */
> // if (!pgss || !pgss_hash || !pgss_enabled())
> return;
>
> This works without crash as you can see here after:

In theory, the rest of the hooks look solid.
As mentioned above, I think the major concern would be a hook that depends
on a variable generated in pgss_post_parse_analyze. Two hooks
(pgss_ExecutorStart, pgss_ExecutorEnd) depend on the query ID
generated from pgss_post_parse_analyze. Fortunately, both of these
functions already check for query ID before doing work.

I really appreciate you putting this change into practice.
Great to see your results align with mine.
Thanks Pascal!!!

--
Raymond Martin
ramarti(at)microsoft(dot)com
Azure Database for PostgreSQL

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2019-04-04 00:25:12 Re: shared-memory based stats collector
Previous Message Kyotaro HORIGUCHI 2019-04-04 00:17:43 Re: New vacuum option to do only freezing