Log statement sample - take two

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

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

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!

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

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