Re: auto_explain sample rate

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.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services