On 05/07/2015 18:22, Julien Rouhaud wrote:
> On 03/06/2015 15:00, Craig Ringer wrote:
>>
>>
>> On 3 June 2015 at 20:04, Andres Freund <and...@anarazel.de
>> <mailto:and...@anarazel.de>> wrote:
>>
>>     On 2015-06-03 18:54:24 +0800, Craig Ringer wrote:
>>     > OK, here we go.
>>
>>     Hm. Wouldn't random sampling be better than what you do? If your queries
>>     have a pattern to them (e.g. you always issue the same 10 queries in
>>     succession), this will possibly only show a subset of the queries.
>>
>>     I think a formulation in fraction (i.e. a float between 0 and 1) will
>>     also be easier to understand.
>>
>>
>> Could be, yeah. I was thinking about the cost of generating a random
>> each time, but it's going to vanish in the noise compared to the rest of
>> the costs in query execution.
>>
> 
> Hello, I've just reviewed the patch.
> 
> I'm not sure if there's a consensus on the sample rate format.  FWIW, I
> also think a fraction would be easier to understand.  Any news about
> generating a random at each call to avoid the query pattern problem ?
> 
> The patch applies without error.  I wonder if there's any reason for
> using pg_lrand48() instead of random(), as there's a port for random()
> if the system lacks it.
> 
> After some quick checks, I found that auto_explain_sample_counter is
> always initialized with the same value.  After some digging, it seems
> that pg_lrand48() always returns the same values in the same order, at
> least on my computer.  Have I missed something?
> 

Well, I obviously missed that pg_srand48() is only used if the system
lacks random/srandom, sorry for the noise.  So yes, random() must be
used instead of pg_lrand48().

I'm attaching a new version of the patch fixing this issue just in case.


> Otherwise, after replacing the pg_lrand48() call with a random(), it
> works just fine.
> 
>> ---
>>  Craig Ringer                   http://www.2ndQuadrant.com/
>>  PostgreSQL Development, 24x7 Support, Training & Services
> 
> 


-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org
*** a/contrib/auto_explain/auto_explain.c
--- b/contrib/auto_explain/auto_explain.c
***************
*** 29,34 **** static bool auto_explain_log_triggers = false;
--- 29,35 ----
  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 int  auto_explain_sample_ratio = 1;
  
  static const struct config_enum_entry format_options[] = {
  	{"text", EXPLAIN_FORMAT_TEXT, false},
***************
*** 47,55 **** static ExecutorRun_hook_type prev_ExecutorRun = NULL;
  static ExecutorFinish_hook_type prev_ExecutorFinish = NULL;
  static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
  
  #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);
--- 48,61 ----
  static ExecutorFinish_hook_type prev_ExecutorFinish = NULL;
  static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
  
+ /* per-backend counter used for ratio sampling */
+ static int  auto_explain_sample_counter = 0;
+ 
  #define auto_explain_enabled() \
  	(auto_explain_log_min_duration >= 0 && \
! 	 (nesting_level == 0 || auto_explain_log_nested_statements)) && \
! 	 (auto_explain_sample_ratio == 1 || auto_explain_sample_counter == 0)
! 
  
  void		_PG_init(void);
  void		_PG_fini(void);
***************
*** 61,66 **** static void explain_ExecutorRun(QueryDesc *queryDesc,
--- 67,85 ----
  static void explain_ExecutorFinish(QueryDesc *queryDesc);
  static void explain_ExecutorEnd(QueryDesc *queryDesc);
  
+ static void
+ auto_explain_sample_ratio_assign_hook(int newval, void *extra)
+ {
+ 	if (auto_explain_sample_ratio != newval)
+ 	{
+ 		/* Schedule a counter reset when the sample ratio changed */
+ 		auto_explain_sample_counter = -1;
+ 	}
+ 
+ 	auto_explain_sample_ratio = newval;
+ }
+ 
+ 
  
  /*
   * Module load callback
***************
*** 159,164 **** _PG_init(void)
--- 178,195 ----
  							 NULL,
  							 NULL);
  
+ 	DefineCustomIntVariable("auto_explain.sample_ratio",
+ 		"Only explain one in approx. every sample_ratio queries, or 1 for all",
+ 							NULL,
+ 							&auto_explain_sample_ratio,
+ 							1,
+ 							1, INT_MAX - 1,
+ 							PGC_SUSET,
+ 							0,
+ 							NULL,
+ 							auto_explain_sample_ratio_assign_hook,
+ 							NULL);
+ 
  	EmitWarningsOnPlaceholders("auto_explain");
  
  	/* Install hooks. */
***************
*** 191,196 **** _PG_fini(void)
--- 222,250 ----
  static void
  explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
  {
+ 	/*
+ 	 * For ratio sampling, only increment the counter for top-level
+ 	 * statements. Either all nested statements will be explained
+ 	 * or none will, because we need to know at ExecutorEnd hook time
+ 	 * whether or not we explained any given statement.
+ 	 */
+ 	if (nesting_level == 0 && auto_explain_sample_ratio > 1)
+ 	{
+ 		if (auto_explain_sample_counter == -1)
+ 		{
+ 			/*
+ 			 * First time the hook ran in this backend, seed the counter. This
+ 			 * might be zero and cause the first statement to be explained.
+ 			 */
+ 			auto_explain_sample_counter = random() % auto_explain_sample_ratio;
+ 		}
+ 		else if ((++auto_explain_sample_counter) == auto_explain_sample_ratio)
+ 		{
+ 			/* Wrap the counter and explain this statement */
+ 			auto_explain_sample_counter = 0;
+ 		}
+ 	}
+ 
  	if (auto_explain_enabled())
  	{
  		/* Enable per-node instrumentation iff log_analyze is required. */
*** a/doc/src/sgml/auto-explain.sgml
--- b/doc/src/sgml/auto-explain.sgml
***************
*** 203,208 **** LOAD 'auto_explain';
--- 203,226 ----
       </para>
      </listitem>
     </varlistentry>
+ 
+    <varlistentry>
+     <term>
+      <varname>auto_explain.sample_ratio</varname> (<type>integer</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 every n'th statement in each session.  The number of statements
+       before the first is explained is between 1 and n, randomized to ensure
+       that short sessions still sometimes get explained. 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

Reply via email to