Hi, I tried the patch, here my comment:
> gettext_noop("Zero effective disables sampling. "
> "-1 use sampling every time (without limit)."),
I do not agree with the zero case. In fact, sampling is disabled as soon as
setting is less than log_min_duration_statements. Furthermore, I think we should
provide a more straightforward description for users.
I changed few comments and documentation:
* As we added much more logic in this function with statement and transaction
sampling. And now with statement_sample_rate, it is not easy to understand the
logic on first look. I reword comment in check_log_duration, I hope it is more
straightforward.
* I am not sure if "every_time" is a good naming for the variable. In fact, if
duration exceeds limit we disable sampling. Maybe sampling_disabled is more
clear?
* I propose to add some words in log_min_duration_statement and
log_statement_sample_rate documentation.
* Rephrased log_statement_sample_limit documentation, I hope it help
understanding.
Patch attached.
Regards,
--
Adrien
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 84341a30e5..ad965a084c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5850,7 +5850,8 @@ 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>.
+ milliseconds, modulated by <varname>log_statement_sample_rate</varname>
+ and <varname>log_statement_sample_limit</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
@@ -5894,6 +5895,8 @@ local0.* /var/log/postgresql
<xref linkend="guc-log-min-duration-statement"/> to be logged.
The default is <literal>1.0</literal>, meaning log all such
statements.
+ <varname>log_statement_sample_limit</varname> allows to disable sampling
+ if duration exceeds the limit.
Setting this to zero disables logging by duration, same as setting
<varname>log_min_duration_statement</varname> to
<literal>-1</literal>.
@@ -5903,6 +5906,26 @@ local0.* /var/log/postgresql
</listitem>
</varlistentry>
+ <varlistentry id="guc-log-statement-sample-limit" xreflabel="log_statement_sample_limit">
+ <term><varname>log_statement_sample_limit</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>log_statement_sample_limit</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Disable effect of <varname>log_statement_sample_rate</varname> when
+ statements last longer than the specified limit in milliseconds.
+ Otherwise, statements that last longer than <varname>log_min_duration_statement</varname>
+ but less than <varname>log_statement_sample_limit</varname> are subject
+ to log sampling according to <varname>log_statement_sample_rate</varname>.
+ When <varname>log_statement_sample_limit</varname> is less or equal to
+ <varname>log_min_duration_statement</varname> then log sampling is
+ never applied.
+ </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 44a59e1d4f..50b3f839da 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2192,10 +2192,15 @@ 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.
- * We also check if this statement in this transaction must be logged
- * (regardless of its duration).
+ *
+ * Determine whether current command's duration should be logged. There is also
+ * sampling mechanism following these rules:
+ * - If log_statement_sample_rate < 1.0, log only a sample.
+ * - If log_statement_sample_limit >= 0 and duration exceeds this limit,
+ * sampling is ignored, so the command is logged.
+ *
+ * If xact_is_sampled is true (due to log_transaction_sample_rate), it means
+ * the whole transaction is logged (regardless of its duration).
*
* Returns:
* 0 if no logging is needed
@@ -2217,6 +2222,7 @@ check_log_duration(char *msec_str, bool was_logged)
int usecs;
int msecs;
bool exceeded;
+ bool every_time;
bool in_sample;
TimestampDifference(GetCurrentStatementStartTimestamp(),
@@ -2234,6 +2240,14 @@ check_log_duration(char *msec_str, bool was_logged)
(secs > log_min_duration_statement / 1000 ||
secs * 1000 + msecs >= log_min_duration_statement)));
+ /*
+ * When log_statement_sample_limit is exceeded, then query is
+ * logged every time.
+ */
+ every_time = (log_statement_sample_limit >= 0 &&
+ (secs > log_statement_sample_limit / 1000 ||
+ secs * 1000 + msecs >= log_statement_sample_limit));
+
/*
* Do not log if log_statement_sample_rate = 0. Log a sample if
* log_statement_sample_rate <= 1 and avoid unnecessary random() call
@@ -2245,7 +2259,7 @@ check_log_duration(char *msec_str, bool was_logged)
(log_statement_sample_rate == 1 ||
random() <= log_statement_sample_rate * MAX_RANDOM_VALUE);
- if ((exceeded && in_sample) || log_duration || xact_is_sampled)
+ if ((exceeded && (in_sample || every_time)) || 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 1208eb9a68..b0c7d9df0e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -510,6 +510,7 @@ int log_min_error_statement = ERROR;
int log_min_messages = WARNING;
int client_min_messages = NOTICE;
int log_min_duration_statement = -1;
+int log_statement_sample_limit = -1;
int log_temp_files = -1;
double log_statement_sample_rate = 1.0;
double log_xact_sample_rate = 0;
@@ -2715,6 +2716,19 @@ static struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"log_statement_sample_limit", PGC_SUSET, LOGGING_WHEN,
+ gettext_noop("Sets the maximum execution time for sampling "
+ "logged statements."),
+ gettext_noop("Zero effective disables sampling. "
+ "-1 use sampling every time (without limit)."),
+ GUC_UNIT_MS
+ },
+ &log_statement_sample_limit,
+ -1, -1, INT_MAX,
+ NULL, NULL, NULL
+ },
+
{
{"log_autovacuum_min_duration", PGC_SIGHUP, LOGGING_WHAT,
gettext_noop("Sets the minimum execution time above which "
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index e709177c37..8f0c2c59d4 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -252,6 +252,7 @@ extern int log_min_error_statement;
extern PGDLLIMPORT int log_min_messages;
extern PGDLLIMPORT int client_min_messages;
extern int log_min_duration_statement;
+extern int log_statement_sample_limit;
extern int log_temp_files;
extern double log_statement_sample_rate;
extern double log_xact_sample_rate;
signature.asc
Description: OpenPGP digital signature
