On 1/23/19 3:12 AM, Kuroda, Hayato wrote:
Dear Adrien,
Hello Kuroda-san
---- config.sgml ----
You must document the behavior when users change the parameter during a
transaction.
あやしい・・・
Agreed, I added a wording.
---- postgres.c ----
I give you three comments.
/* flag for logging statements in this transaction */
I think "a" or the plural form should be used instead of "this"
Fixed
* xact_is_sampled is left at the end of a transaction.
Should the parameter be set to false at the lowest layer of the transaction
system?
I understand it is unnecessary for the functionality, but it have more symmetry.
Yes, it is not necessary. I wonder what is more important : keep some
kind of symmetry or avoid unnecessary code (which can be source of mistake)?
* check_log_duration should be used only when postgres check the duration.
But I'm not sure a new function such as check_is_sampled is needed because A
processing in new function will be as almost same as check_log_duration.
I agree, I asked myself the same question and I preferred to keep code
simple.
Here is 4th patch.
Thanks!
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index b6f5822b84..29b64c51b3 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5780,6 +5780,25 @@ local0.* /var/log/postgresql
</listitem>
</varlistentry>
+ <varlistentry id="guc-transaction-sample-rate" xreflabel="log_transaction_sample_rate">
+ <term><varname>log_transaction_sample_rate</varname> (<type>real</type>)
+ <indexterm>
+ <primary><varname>log_transaction_sample_rate</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Set the fraction of transactions whose statements are logged. It applies
+ to each new transaction regardless of their duration. The default is
+ <literal>0</literal>, meaning do not log statements from this transaction.
+ Setting this to <literal>1</literal> logs all statements for all transactions.
+ Logging can be disabled inside a transaction by setting this to 0.
+ <varname>log_transaction_sample_rate</varname> is helpful to track a
+ sample of transaction.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 7c3a9c1e89..1a52c10c39 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1821,6 +1821,9 @@ StartTransaction(void)
s->state = TRANS_START;
s->transactionId = InvalidTransactionId; /* until assigned */
+ /* Determine if we log statements in this transaction */
+ xact_is_sampled = random() <= log_xact_sample_rate * MAX_RANDOM_VALUE;
+
/*
* initialize current transaction state fields
*
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e773f20d9f..a11667834e 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -100,7 +100,8 @@ int max_stack_depth = 100;
/* wait N seconds to allow attach from a debugger */
int PostAuthDelay = 0;
-
+/* flag for logging statements in a transaction */
+bool xact_is_sampled = false;
/* ----------------
* private variables
@@ -2203,6 +2204,8 @@ 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).
*
* Returns:
* 0 if no logging is needed
@@ -2218,7 +2221,8 @@ check_log_statement(List *stmt_list)
int
check_log_duration(char *msec_str, bool was_logged)
{
- if (log_duration || log_min_duration_statement >= 0)
+ if (log_duration || log_min_duration_statement >= 0 ||
+ (xact_is_sampled && log_xact_sample_rate > 0))
{
long secs;
int usecs;
@@ -2252,11 +2256,12 @@ 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)
+ if ((exceeded && in_sample) || log_duration ||
+ (xact_is_sampled && log_xact_sample_rate > 0))
{
snprintf(msec_str, 32, "%ld.%03d",
secs * 1000 + msecs, usecs % 1000);
- if (exceeded && !was_logged)
+ if ((exceeded || xact_is_sampled) && !was_logged)
return 2;
else
return 1;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c216ed0922..06a5e668aa 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -494,6 +494,7 @@ 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;
int temp_file_limit = -1;
@@ -3347,13 +3348,25 @@ static struct config_real ConfigureNamesReal[] =
{"log_statement_sample_rate", PGC_SUSET, LOGGING_WHEN,
gettext_noop("Fraction of statements over log_min_duration_statement to log."),
gettext_noop("If you only want a sample, use a value between 0 (never "
- "log) and 1.0 (always log).")
+ "log) and 1 (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. "
+ "Use a value between 0 (never log) and 1 (log all "
+ "statements for all transactions).")
+ },
+ &log_xact_sample_rate,
+ 0.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 a21865a77f..07e590ea20 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -491,6 +491,10 @@
# log_min_duration_statement. 1.0 logs all statements,
# 0 never logs.
+#log_transaction_sample_rate = 0 # Fraction of transactions whose statements
+ # are logged regardless of their duration. 1.0 logs all
+ # statements from all transactions, 0 never logs.
+
# - What to Log -
#debug_print_parse = off
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index ed21a13896..1f7f0d28bc 100644
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -54,6 +54,9 @@ extern PGDLLIMPORT int XactIsoLevel;
extern bool DefaultXactReadOnly;
extern bool XactReadOnly;
+/* flag for logging statements in this transaction */
+extern bool xact_is_sampled;
+
/*
* Xact is deferrable -- only meaningful (currently) for read only
* SERIALIZABLE transactions
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c07e7b945e..cb67fbd907 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -252,6 +252,7 @@ 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;
