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