On 11/19/18 2:52 PM, Dmitry Dolgov wrote: >> On Mon, Nov 19, 2018 at 2:40 PM Tomas Vondra <tomas.von...@2ndquadrant.com> >> wrote: >> >> On 11/19/18 2:57 AM, Michael Paquier wrote: >>> On Sun, Nov 18, 2018 at 12:18:33PM +0100, Dmitry Dolgov wrote: >>>> Since it's hard to come up with a concise name that will mention sampling >>>> rate >>>> in the context of min_duration_statement, I think it's fine to name this >>>> configuration "log_sample_rate", as long as it's dependency from >>>> log_min_duration_statements is clearly explained in the documentation. >>> >>> log_sample_rate looks fine to me as a name. >> >> That seems far too short to me - the name should indicate it applies to >> statement logging. I'd say log_statement_sample_rate is better. > > I agree, sounds reasonable. >
Thanks for your comments. Here is the updated patch. I fixed a warning for missing parentheses in this expression: if ((exceeded && in_sample) || log_duration) It passed make check_world and make docs
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index d1dab35559..0e21b6a95f 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -5247,6 +5247,26 @@ 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> + Causes logging only a fraction of the statements when <xref + linkend="guc-log-min-duration-statement"/> is used. The default is + <literal>1</literal>, meaning log all statements longer than + <varname>log_min_duration_statement</varname>. Setting this to zero + disables logging, same as setting + <varname>log_min_duration_statement</varname> to + <literal>-1</literal>. Using <varname>log_statement_sample_rate</varname> is + helpful when the traffic is too high to log all queries. + </para> + </listitem> + </varlistentry> + </variablelist> <para> diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index a3b9757565..6777eebde1 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -2201,7 +2201,8 @@ check_log_statement(List *stmt_list) /* * check_log_duration - * Determine whether current command's duration should be logged + * Determine whether current command's duration should be logged. + * If log_statement_sample_rate < 1.0, log only a sample. * * Returns: * 0 if no logging is needed @@ -2223,6 +2224,7 @@ check_log_duration(char *msec_str, bool was_logged) int usecs; int msecs; bool exceeded; + bool in_sample; TimestampDifference(GetCurrentStatementStartTimestamp(), GetCurrentTimestamp(), @@ -2239,7 +2241,16 @@ check_log_duration(char *msec_str, bool was_logged) (secs > log_min_duration_statement / 1000 || secs * 1000 + msecs >= log_min_duration_statement))); - if (exceeded || log_duration) + /* + * 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. + */ + 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) { 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 0ec3ff0fd6..452bdd7484 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -471,6 +471,7 @@ 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; int trace_recovery_messages = LOG; int temp_file_limit = -1; @@ -3272,6 +3273,17 @@ static struct config_real ConfigureNamesReal[] = NULL, NULL, NULL }, + { + {"log_statement_sample_rate", PGC_SUSET, LOGGING_WHEN, + gettext_noop("Fraction of statements to log."), + gettext_noop("1.0 logs all statements."), + NULL + }, + &log_statement_sample_rate, + 1.0, 0.0, 1.0, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, 0.0, 0.0, 0.0, NULL, NULL, NULL diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 3038fe627b..798888829e 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -441,6 +441,8 @@ # statements running at least this number # of milliseconds +#log_statement_sample_rate = 1 # Fraction of logged statements. 1 means log + # all statements. # - What to Log - diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index df2e556b02..4c5cabcc5f 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -251,6 +251,7 @@ 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 int temp_file_limit;