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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
