Re: idea: log_statement_sample_rate - bottom limit for sampling - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: idea: log_statement_sample_rate - bottom limit for sampling
Date
Msg-id CAFj8pRCsfOLNta1_gZmho9zfdtrMR+UXS2vWYFPg=VzhBEhSZA@mail.gmail.com
Whole thread Raw
In response to Re: idea: log_statement_sample_rate - bottom limit for sampling  (Adrien Nayrat <adrien.nayrat@anayrat.info>)
Responses Re: idea: log_statement_sample_rate - bottom limit for sampling
List pgsql-hackers


st 19. 6. 2019 v 10:49 odesílatel Adrien Nayrat <adrien.nayrat@anayrat.info> napsal:
On 6/18/19 8:29 PM, Pavel Stehule wrote:
>
>
> út 18. 6. 2019 v 14:03 odesílatel Adrien Nayrat <adrien.nayrat@anayrat.info
> <mailto:adrien.nayrat@anayrat.info>> napsal:
>
>     Hi,
>
>     I tried the patch, here my comment:
>
>     > gettext_noop("Zero effective disables sampling. "
>     >                          "-1 use sampling every time (without limit)."),
>
>     I do not agree with the zero case. In fact, sampling is disabled as soon as
>     setting is less than log_min_duration_statements. Furthermore, I think we should
>     provide a more straightforward description for users.
>
>
> You have true, but I have not a idea,how to describe it in one line. In this
> case the zero is corner case, and sampling is disabled without dependency on
> log_min_duration_statement.
>  
> I think so this design has only few useful values and ranges
>
> a) higher than log_min_duration_statement .. sampling is active with limit
> b) 0 .. for this case - other way how to effective disable sampling - no
> dependency on other
> c) -1 or negative value - sampling is allowed every time.
>
> Sure, there is range (0..log_min_duration_statement), but for this range this
> value has not sense. I think so this case cannot be mentioned in short
> description. But it should be mentioned in documentation.

Yes, it took me a while to understand :) I am ok to keep simple in GUC
description and give more information in documentation.

Maybe some like. "The zero block sampling. Negative value forces sampling without limit"


>
>
>     I changed few comments and documentation:
>
>       * As we added much more logic in this function with statement and transaction
>     sampling. And now with statement_sample_rate, it is not easy to understand the
>     logic on first look. I reword comment in check_log_duration, I hope it is more
>     straightforward.
>
>       * I am not sure if "every_time" is a good naming for the variable. In fact, if
>     duration exceeds limit we disable sampling. Maybe sampling_disabled is more
>     clear?
>
>
> For me important is following line
>
> (exceeded && (in_sample || every_time))
>
> I think so "every_time" or "always" or "every" is in this context more
> illustrative than "sampling_disabled". But my opinion is not strong in this
> case, and I have not a problem accept common opinion.

Oh, yes, that's correct. I do not have a strong opinion too. Maybe someone else
will have better idea.

the naming in this case is not hard issue, and comitter  can decide.

Regards

Pavel

--
Adrien

pgsql-hackers by date:

Previous
From: ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Subject: Re: Remove one last occurrence of "replication slave" in comments
Next
From: Robert Haas
Date:
Subject: Re: POC: Cleaning up orphaned files using undo logs