On 7/28/19 12:19 AM, Tomas Vondra wrote:
> Hi,
>
> I've started reviewing this patch, thinking that maybe I could get it
> committed as it's marked as RFC. In general I agree with having this
> fuature, but I think we need to rethink the GUC because the current
> approach is just confusing.
>
> The way the current patch works is that we have three GUCs:
>
> log_min_duration_statement
> log_statement_sample_limit
> log_statement_sample_rate
>
> and it essentially works like this:
>
> - If the duration exceeds log_min_duration_statement, we start sampling
> the commands with log_statement_sample rate.
>
> - If the duration exceeds log_statement_sample_limit, we just log the
> command every time (i.e. we disable sampling, using sample rate 1.0).
>
> IMO that's bound to be confusing for users, because one threshold
> behaves as minimum while the other behaves as maximum.
I agree, it took me a while to understand how it behave with the three GUC. That
why I tried to enrich documentation, but it may mean that the functionality is
not properly implemented.
>
>
> What I think we should do instead is 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
>
>
> I think this is going to be much easier for users to understand.
+1, I like this idea.
I don't really have an opinion if we have to revert the whole feature or try to
fix it for v12. But it is true it is a late to fix it.
Furthermore, users who really want this feature in v12 can use an extension for
that purpose [1].
1: I made this extension with same kind of features :
https://github.com/anayrat/pg_sampletolog
