Revert "Add log_statement_sample_rate parameter"

This reverts commit 88bdbd3f746049834ae3cc972e6e650586ec3c9d.

As committed, statement sampling used the existing duration threshold
(log_min_duration_statement) when decide which statements to sample.
The issue is that even the longest statements are subject to sampling,
and so may not end up logged. An improvement was proposed, introducing
a second duration threshold, but it would not be backwards compatible.
So we've decided to revert this feature - the separate threshold should
be part of the feature itself.

Discussion: https://postgr.es/m/CAFj8pRDS8tQ3Wviw9%3DAvODyUciPSrGeMhJi_WPE%2BEB8%2B4gLL-Q%40mail.gmail.com

5 files changed, 13 insertions, 65 deletions

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c91e3e15507..1f8d1bdb27d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5850,13 +5850,12 @@ local0.*    /var/log/postgresql
         <para>
          Causes the duration of each completed statement to be logged
          if the statement ran for at least the specified number of
-         milliseconds, modulated by <varname>log_statement_sample_rate</varname>.
-         Setting this to zero prints all statement durations.
-         <literal>-1</literal> (the default) disables logging statements due to
-         exceeding duration threshold; for example, if you set it to
-         <literal>250ms</literal>, then all SQL statements that run 250ms or
-         longer will be logged. Enabling this parameter can be helpful in
-         tracking down unoptimized queries in your applications.
+         milliseconds.  Setting this to zero prints all statement durations.
+         Minus-one (the default) disables logging statement durations.
+         For example, if you set it to <literal>250ms</literal>
+         then all SQL statements that run 250ms or longer will be
+         logged.  Enabling this parameter can be helpful in tracking down
+         unoptimized queries in your applications.
          Only superusers can change this setting.
         </para>
 
@@ -5882,27 +5881,6 @@ local0.*    /var/log/postgresql
        </listitem>
       </varlistentry>
 
-     <varlistentry id="guc-log-statement-sample-rate" xreflabel="log_statement_sample_rate">
-      <term><varname>log_statement_sample_rate</varname> (<type>real</type>)
-      <indexterm>
-       <primary><varname>log_statement_sample_rate</varname> configuration parameter</primary>
-      </indexterm>
-      </term>
-       <listitem>
-        <para>
-         Determines the fraction of statements that exceed
-         <xref linkend="guc-log-min-duration-statement"/> to be logged.
-         The default is <literal>1.0</literal>, meaning log all such
-         statements.
-         Setting this to zero disables logging by duration, same as setting
-         <varname>log_min_duration_statement</varname> to
-         <literal>-1</literal>.
-         <varname>log_statement_sample_rate</varname> is helpful when the
-         traffic is too high to log all queries.
-        </para>
-       </listitem>
-      </varlistentry>
-
      <varlistentry id="guc-log-transaction-sample-rate" xreflabel="log_transaction_sample_rate">
       <term><varname>log_transaction_sample_rate</varname> (<type>real</type>)
       <indexterm>
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f010aa884ce..c28cc370129 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2192,8 +2192,7 @@ check_log_statement(List *stmt_list)
 
 /*
  * check_log_duration
- *		Determine whether current command's duration should be logged.
- *		If log_statement_sample_rate < 1.0, log only a sample.
+ *		Determine whether current command's duration should be logged
  *		We also check if this statement in this transaction must be logged
  *		(regardless of its duration).
  *
@@ -2217,7 +2216,6 @@ check_log_duration(char *msec_str, bool was_logged)
 		int			usecs;
 		int			msecs;
 		bool		exceeded;
-		bool		in_sample;
 
 		TimestampDifference(GetCurrentStatementStartTimestamp(),
 							GetCurrentTimestamp(),
@@ -2234,17 +2232,7 @@ check_log_duration(char *msec_str, bool was_logged)
 					 (secs > log_min_duration_statement / 1000 ||
 					  secs * 1000 + msecs >= log_min_duration_statement)));
 
-		/*
-		 * Do not log if log_statement_sample_rate = 0. Log a sample if
-		 * log_statement_sample_rate <= 1 and avoid unecessary random() call
-		 * if log_statement_sample_rate = 1.
-		 */
-		if (exceeded)
-			in_sample = log_statement_sample_rate != 0 &&
-				(log_statement_sample_rate == 1 ||
-				 random() <= log_statement_sample_rate * MAX_RANDOM_VALUE);
-
-		if ((exceeded && in_sample) || log_duration || xact_is_sampled)
+		if (exceeded || log_duration || xact_is_sampled)
 		{
 			snprintf(msec_str, 32, "%ld.%03d",
 					 secs * 1000 + msecs, usecs % 1000);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 631f16f5fe7..8adb1cfeab0 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -511,7 +511,6 @@ int			log_min_messages = WARNING;
 int			client_min_messages = NOTICE;
 int			log_min_duration_statement = -1;
 int			log_temp_files = -1;
-double		log_statement_sample_rate = 1.0;
 double		log_xact_sample_rate = 0;
 int			trace_recovery_messages = LOG;
 
@@ -2705,8 +2704,7 @@ static struct config_int ConfigureNamesInt[] =
 		{"log_min_duration_statement", PGC_SUSET, LOGGING_WHEN,
 			gettext_noop("Sets the minimum execution time above which "
 						 "statements will be logged."),
-			gettext_noop("Zero prints all queries, subject to log_statement_sample_rate. "
-						 "-1 turns this feature off."),
+			gettext_noop("Zero prints all queries. -1 turns this feature off."),
 			GUC_UNIT_MS
 		},
 		&log_min_duration_statement,
@@ -3433,17 +3431,6 @@ static struct config_real ConfigureNamesReal[] =
 	},
 
 	{
-		{"log_statement_sample_rate", PGC_SUSET, LOGGING_WHEN,
-			gettext_noop("Fraction of statements exceeding log_min_duration_statement to be logged."),
-			gettext_noop("If you only want a sample, use a value between 0.0 (never "
-						 "log) and 1.0 (always log).")
-		},
-		&log_statement_sample_rate,
-		1.0, 0.0, 1.0,
-		NULL, NULL, NULL
-	},
-
-	{
 		{"log_transaction_sample_rate", PGC_SUSET, LOGGING_WHEN,
 			gettext_noop("Set the fraction of transactions to log for new transactions."),
 			gettext_noop("Logs all statements from a fraction of transactions. "
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 5ee5e09ddf6..072dfb6f0d6 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -488,14 +488,10 @@
 					#   fatal
 					#   panic (effectively off)
 
-#log_min_duration_statement = -1	# logs statements and their durations
-					# according to log_statement_sample_rate. -1 is disabled,
-					# 0 logs all statements, > 0 logs only statements running
-					# at least this number of milliseconds.
-
-#log_statement_sample_rate = 1.0	# Fraction of logged statements exceeding
-					# log_min_duration_statement to be logged.
-					# 1.0 logs all such statements, 0.0 never logs.
+#log_min_duration_statement = -1	# -1 is disabled, 0 logs all statements
+					# and their durations, > 0 logs only
+					# statements running at least this number
+					# of milliseconds
 
 #log_transaction_sample_rate = 0.0	# Fraction of transactions whose statements
 					# are logged regardless of their duration. 1.0 logs all
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index e800230ed94..a93ed77c9c3 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -253,7 +253,6 @@ extern PGDLLIMPORT int log_min_messages;
 extern PGDLLIMPORT int client_min_messages;
 extern int	log_min_duration_statement;
 extern int	log_temp_files;
-extern double log_statement_sample_rate;
 extern double log_xact_sample_rate;
 
 extern int	temp_file_limit;