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
signature.asc
Description: OpenPGP digital signature
