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
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c index 0950e18..fa564fd 100644 --- a/contrib/auto_explain/auto_explain.c +++ b/contrib/auto_explain/auto_explain.c @@ -29,6 +29,7 @@ static bool auto_explain_log_triggers = false; static bool auto_explain_log_timing = true; static int auto_explain_log_format = EXPLAIN_FORMAT_TEXT; static bool auto_explain_log_nested_statements = false; +static double auto_explain_sample_ratio = 1; static const struct config_enum_entry format_options[] = { {"text", EXPLAIN_FORMAT_TEXT, false}, @@ -47,10 +48,14 @@ static ExecutorRun_hook_type prev_ExecutorRun = NULL; static ExecutorFinish_hook_type prev_ExecutorFinish = NULL; static ExecutorEnd_hook_type prev_ExecutorEnd = NULL; +/* Is the current query sampled, per backend */ +static bool current_query_sampled = true; + #define auto_explain_enabled() \ (auto_explain_log_min_duration >= 0 && \ (nesting_level == 0 || auto_explain_log_nested_statements)) + void _PG_init(void); void _PG_fini(void); @@ -62,6 +67,7 @@ static void explain_ExecutorFinish(QueryDesc *queryDesc); static void explain_ExecutorEnd(QueryDesc *queryDesc); + /* * Module load callback */ @@ -159,6 +165,19 @@ _PG_init(void) NULL, NULL); + DefineCustomRealVariable("auto_explain.sample_ratio", + "Percentage of queries to process.", + NULL, + &auto_explain_sample_ratio, + 1.0, + 0.0, + 1.0, + PGC_SUSET, + 0, + NULL, + NULL, + NULL); + EmitWarningsOnPlaceholders("auto_explain"); /* Install hooks. */ @@ -191,7 +210,15 @@ _PG_fini(void) static void explain_ExecutorStart(QueryDesc *queryDesc, int eflags) { - if (auto_explain_enabled()) + /* + * 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() && current_query_sampled) { /* Enable per-node instrumentation iff log_analyze is required. */ if (auto_explain_log_analyze && (eflags & EXEC_FLAG_EXPLAIN_ONLY) == 0) @@ -210,7 +237,7 @@ explain_ExecutorStart(QueryDesc *queryDesc, int eflags) else standard_ExecutorStart(queryDesc, eflags); - if (auto_explain_enabled()) + if (auto_explain_enabled() && current_query_sampled) { /* * Set up to track total elapsed time in ExecutorRun. Make sure the @@ -280,7 +307,7 @@ explain_ExecutorFinish(QueryDesc *queryDesc) static void explain_ExecutorEnd(QueryDesc *queryDesc) { - if (queryDesc->totaltime && auto_explain_enabled()) + if (queryDesc->totaltime && auto_explain_enabled() && current_query_sampled) { double msec; diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index d527208..9d40e65 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -203,6 +203,23 @@ LOAD 'auto_explain'; </para> </listitem> </varlistentry> + + <varlistentry> + <term> + <varname>auto_explain.sample_ratio</varname> (<type>real</type>) + <indexterm> + <primary><varname>auto_explain.sample_ratio</> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + <varname>auto_explain.sample_ratio</varname> causes auto_explain to only + explain X percent statements in each session. In case of nested + statements, either all will be explained or none. Only superusers can + change this setting. + </para> + </listitem> + </varlistentry> </variablelist> <para>
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers