From: | Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: auto_explain sample rate |
Date: | 2016-02-17 00:17:35 |
Message-ID: | 56C3BC1F.1040607@dalibo.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 16/02/2016 22:51, Alvaro Herrera wrote:
> Julien Rouhaud wrote:
>
> Hijacking this macro is just too obscure:
>
>> #define auto_explain_enabled() \
>> (auto_explain_log_min_duration >= 0 && \
>> - (nesting_level == 0 || auto_explain_log_nested_statements))
>> + (nesting_level == 0 || auto_explain_log_nested_statements) && \
>> + current_query_sampled)
>
> because it then becomes hard to figure out that assigning to _sampled is
> what makes the enabled() check pass or not depending on sampling:
>
>> @@ -191,6 +211,14 @@ _PG_fini(void)
>> static void
>> explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
>> {
>> + /*
>> + * For ratio sampling, randomly choose top-level statement. Either
>> + * all nested statements will be explained or none will.
>> + */
>> + if (auto_explain_log_min_duration >= 0 && nesting_level == 0)
>> + current_query_sampled = (random() < auto_explain_sample_ratio *
>> + MAX_RANDOM_VALUE);
>> +
>> if (auto_explain_enabled())
>> {
>
> I think it's better to keep the "enabled" macro unmodified, and just add
> another conditional to the "if" test there.
>
Thanks for looking at this!
Agreed, it's too obscure. Attached v4 fixes as you said.
--
Julien Rouhaud
http://dalibo.com - http://dalibo.org
Attachment | Content-Type | Size |
---|---|---|
auto_explain_sample_rate-v4.patch | text/x-patch | 3.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jim Nasby | 2016-02-17 00:38:20 | Re: proposal: function parse_ident |
Previous Message | Bruce Momjian | 2016-02-16 23:50:41 | Re: PostgreSQL Audit Extension |