Re: Log statement sample - take two

2019-11-06 Thread Adrien Nayrat
On 11/6/19 7:16 PM, Tomas Vondra wrote:
> Pushed, with some minor tweaks and rewording to the documentation.
> 
> The first bit, documenting the log_transaction_sample_rate as PG_SUSET,
> got backpatched to 12, where it was introduced.
> 
> regards
> 

Thanks Tomas!

-- 
Adrien





Re: Log statement sample - take two

2019-11-06 Thread Tomas Vondra

Pushed, with some minor tweaks and rewording to the documentation.

The first bit, documenting the log_transaction_sample_rate as PG_SUSET,
got backpatched to 12, where it was introduced.

regards

--
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services 





Re: Log statement sample - take two

2019-11-04 Thread Adrien Nayrat
On 11/4/19 2:08 AM, Tomas Vondra wrote:
> 
> Seems fine to me, mostly. I think the docs should explain how
> log_min_duration_statement interacts with log_min_duration_sample.
> Attached is a patch doing that, by adding one para to each GUC, along
> with some minor rewordings. I think the docs are mixing "sampling"
> vs. "logging" and "durations" vs. "statements" not sure.

Thanks for the rewording, it's clearer now.

> 
> I also think the two new sampling GUCs (log_min_duration_sample and
> log_statement_sample_rate) should be next to each other. We're not
> ordering the GUCs alphabetically anyway.

+1

> 
> I plan to make those changes and push in a couple days.
> 

Thanks!




Re: Log statement sample - take two

2019-11-03 Thread Tomas Vondra

On Sat, Oct 19, 2019 at 05:02:01PM +0200, Adrien Nayrat wrote:

Hello,

This patch propose a new way to sample statement to logs.

As a reminder, this feature was committed in PG12[1] then reverted[2] after the
proposition of log_statement_sample_limit[3]

The first implementation added a new GUC to sample statement logged by
log_min_duration_statement. Then, we wanted to add the ability to log all
statement whose duration exceed log_statement_sample_limit.

This was confusing because log_min_duration behaves as a minimum to enable
sampling. While log_statement_sample_limit behave as maximum to disable it.[4]

Tomas Vondra proposed to use two minimum thresholds:


1) log_min_duration_sample - enables sampling of commands, using the
existing GUC log_statement_sample_rate

2) log_min_duration_statement - logs all commands exceeding this


This patch implement this idea.

PS: I notice I forgot to mention "Only superusers can change this setting" in
the log_transaction_sample_rate documentation. It attached a second patch to fix
this.



Seems fine to me, mostly. I think the docs should explain how
log_min_duration_statement interacts with log_min_duration_sample.
Attached is a patch doing that, by adding one para to each GUC, along
with some minor rewordings. I think the docs are mixing "sampling"
vs. "logging" and "durations" vs. "statements" not sure.

I also think the two new sampling GUCs (log_min_duration_sample and
log_statement_sample_rate) should be next to each other. We're not
ordering the GUCs alphabetically anyway.

I plan to make those changes and push in a couple days.


regards

--
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services 
>From 185caaf3b1956300d87bdf48cdd91a7f89b515c3 Mon Sep 17 00:00:00 2001
From: Tomas Vondra 
Date: Mon, 4 Nov 2019 02:00:26 +0100
Subject: [PATCH 1/2] fix log_min_transaction_sample_rate docs

---
 doc/src/sgml/config.sgml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 0191ec84b1..48d7939d2d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5988,6 +5988,7 @@ local0.*/var/log/postgresql
  logs all statements for all transactions.
  log_transaction_sample_rate is helpful to track a
  sample of transaction.
+ Only superusers can change this setting.
 

 
-- 
2.21.0

>From ca4a8b558742f247e3c9e5d388dacd4ef918e154 Mon Sep 17 00:00:00 2001
From: Tomas Vondra 
Date: Mon, 4 Nov 2019 01:57:45 +0100
Subject: [PATCH 2/2] log_min_duration_sample rework

---
 doc/src/sgml/config.sgml  | 85 +++
 src/backend/tcop/postgres.c   | 40 ++---
 src/backend/utils/misc/guc.c  | 27 +-
 src/backend/utils/misc/postgresql.conf.sample | 11 +++
 src/include/utils/guc.h   |  2 +
 5 files changed, 153 insertions(+), 12 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 48d7939d2d..e5372d933c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5930,6 +5930,55 @@ local0.*/var/log/postgresql
   
  
 
+ 
+  log_min_duration_sample (integer)
+  
+   log_min_duration_sample configuration 
parameter
+  
+  
+   
+
+ Allows to sample the logging of the duration of each completed
+ statement if the statement ran for at least the specified amount of
+ time. If this value is specified without units, it is taken as 
milliseconds.
+ Setting this to zero samples all statement durations.
+ Minus-one (the default) disables sampling statement durations.
+ For example, if you set it to 250ms
+ then all SQL statements that run 250ms or longer will be considered
+ for sampling, with sample rate is controlled by . 
+ Enabling this parameter can be helpful when the traffic too high to
+ sample all queries.
+ Only superusers can change this setting.
+
+
+
+ This option has lower priority than ,
+ which means setting this to a value higher than 
+ means the sample rate is ignored and all queries will be logged.
+
+
+
+ For clients using extended query protocol, durations of the Parse,
+ Bind, and Execute steps are logged independently.
+
+
+   
+
+ When using this option together with
+ ,
+ the text of statements that are logged because of
+ log_statement will not be repeated in the
+ duration log message.
+ If you are not using syslog, it is 
recommended
+ that you log the PID or session ID using
+ 
+ so that you can link the statement message to the later
+ duration message using the process ID or session ID.
+
+   
+   
+ 
+
 

Log statement sample - take two

2019-10-19 Thread Adrien Nayrat
Hello,

This patch propose a new way to sample statement to logs.

As a reminder, this feature was committed in PG12[1] then reverted[2] after the
proposition of log_statement_sample_limit[3]

The first implementation added a new GUC to sample statement logged by
log_min_duration_statement. Then, we wanted to add the ability to log all
statement whose duration exceed log_statement_sample_limit.

This was confusing because log_min_duration behaves as a minimum to enable
sampling. While log_statement_sample_limit behave as maximum to disable it.[4]

Tomas Vondra proposed to use two minimum thresholds:

> 1) log_min_duration_sample - enables sampling of commands, using the
> existing GUC log_statement_sample_rate
> 
> 2) log_min_duration_statement - logs all commands exceeding this 

This patch implement this idea.

PS: I notice I forgot to mention "Only superusers can change this setting" in
the log_transaction_sample_rate documentation. It attached a second patch to fix
this.


Regards,

1:
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=799e220346f1387e823a4dbdc3b1c8c3cdc5c3e0
2:
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=75506195da81d75597a4025b72f8367e6c45f60d
3:
https://www.postgresql.org/message-id/CAFj8pRDS8tQ3Wviw9%3DAvODyUciPSrGeMhJi_WPE%2BEB8%2B4gLL-Q%40mail.gmail.com
4:
https://www.postgresql.org/message-id/20190727221948.irg6sfqh57dynoc7%40development

-- 
Adrien NAYRAT

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f69b04db05..ef25cf1e96 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5985,6 +5985,7 @@ local0.*/var/log/postgresql
  logs all statements for all transactions.
  log_transaction_sample_rate is helpful to track a
  sample of transaction.
+ Only superusers can change this setting.
 

 
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 886632ff43..f69b04db05 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5855,6 +5855,49 @@ local0.*/var/log/postgresql
   
  
 
+ 
+  log_min_duration_sample (integer)
+  
+   log_min_duration_sample configuration parameter
+  
+  
+   
+
+ Allows to sample the logging of the duration of each completed
+ statement if the statement ran for at least the specified duration
+ (default in milliseconds). Setting this to zero sample all statement
+ durations. Minus-one (the default) disables sampling statement durations.
+ Sampling is controlled by .
+ For example, if you set it to 250ms
+ then all SQL statements that run 250ms or longer will be
+ sampled to logs. Enabling this parameter can be helpful when the traffic
+ is too high to log all queries.
+ Only superusers can change this setting.
+
+
+
+ For clients using extended query protocol, durations of the Parse,
+ Bind, and Execute steps are logged independently.
+
+
+   
+
+ When using this option together with
+ ,
+ the text of statements that are logged because of
+ log_statement will not be repeated in the
+ duration log message.
+ If you are not using syslog, it is recommended
+ that you log the PID or session ID using
+ 
+ so that you can link the statement message to the later
+ duration message using the process ID or session ID.
+
+   
+   
+ 
+
+
  
   log_min_duration_statement (integer)
   
@@ -5894,7 +5937,37 @@ local0.*/var/log/postgresql
 


-  
+ 
+
+ 
+  log_statement_sample_rate (real)
+  
+   log_statement_sample_rate configuration parameter
+  
+  
+   
+
+ Determines the fraction of statements that exceed
+  to be logged.
+ This is a statistical parameter, for example 0.5
+ means there is statiscally one in two chances to log the statement.
+ The default is 1.0, meaning log all such
+ statements.
+ Setting this to zero disables sample logging, same as setting
+ log_min_duration_sample to
+ -1.
+ log_statement_sample_rate is helpful when the
+ traffic is too high to log all queries.
+ Only superusers can change this setting.
+
+   
+
+ Like all statement-logging options, this option can add significant
+ overhead.
+
+   
+   
+ 
 
  
   log_transaction_sample_rate (real)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e8d8e6f828..6cdc4f3349 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2230,12 +2230,15 @@ check_log_statement(List *stmt_list)
 int
 check_log_duration(char *msec_str, bool was_logged)
 {
-	if (log_duration ||