Re: auto_explain sample rate

On Thu, Mar 10, 2016 at 10:07 PM, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:

> On 10/03/16 20:59, Julien Rouhaud wrote:
>
>> On 10/03/2016 04:37, Petr Jelinek wrote:
>>
>>> On 17/02/16 01:17, Julien Rouhaud wrote:
>>>
>>>>
>>>> Agreed, it's too obscure. Attached v4 fixes as you said.
>>>>
>>>>
>>> Seems to be simple enough patch and works. However I would like
>>> documentation to say that the range is 0 to 1 and represents fraction of
>>> the queries sampled, because right now both the GUC description and the
>>> documentation say it's in percent but that's not really true as percent
>>> is 0 to 100.
>>>
>>>
>> Agreed. v5 attached fixes that.
>>
>>
> Great, I will test it once more (just because when I don't bugs suddenly
> appear out of nowhere) and mark it ready for committer.
>
>
Coming back to the previous discussions about random() - AFAICT this patch
will introduce the random() call always (in explain_ExecutorStart):

+ if (auto_explain_log_min_duration >= 0 && nesting_level == 0)
+ current_query_sampled = (random() < auto_explain_sample_ratio *
+ MAX_RANDOM_VALUE);

Not sure what the overhead is, but wouldn't it be better to include a check
for current_query_sampled>0 in the if part of that statement? Regardless
of performance, that feels cleaner to me. Or am I missing something?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/