Re: idea: log_statement_sample_rate - bottom limit for sampling

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