On Mon, Feb 9, 2015 at 3:47 PM, Naoya Anzai
<anzai-naoya@mxu.nes.nec.co.jp> wrote:
>>> Feature test
>>> ====================
> (snip)
>> I thought about using a
>> special value like -2 to define the behavior you are mentioning here,
>> aka with "-2" disable custom value and GUC parameter but I don't think
>> it is worth adding as that's an ugly 3 line of code of this type:
>> if (log_min_duration == -2)
>> enforce_log_min = -1;
>
> I discussed about this use case with my co-workers, who said
> "that code is not good", then we concluded that it was actually
> a rare case. If such a case sometimes happens in fact, I (or someone)
> will suggest a different way from this patch to handle this case.
>
> We know there is a practical workaround. :)
OK, done.
>>> Coding review
>>> ====================
>>> I think description of log_autovacuum_min_duration in reloptions.c
>>> (line:215) should be like other parameters (match with guc.c). So
>>> it should be "Sets the minimum execution time above which autovacuum
>>> actions will be logged." but not "Log autovacuum execution for
>>> given threshold".
>>
>>What about that then?
>>"Minimum execution time above which autovacuum actions will be logged"
>
> That's roughly match. For matching with guc.c, you might be better to
> add "Sets the" to that discription.
OK, done this way...
>>And I forgot to change VacuumStmt for the ANALYZE portion in gram.y...
>>Patch updated attached.
>
> Sorry, I could not correctly express my opinion to you. I mean
> "log_autovacuum_min_duration" is used only by AutoVacuum, Any VACUUM
> queries (including VACUUM VERBOSE) never use this parameter. So,
> when someone executes Manual Vacuum, "log_min_duration" always should
> be initialized to -1.
>
> ANALYZE should also be the same.
>
> In other words, it is not necessary to initialize "log_min_duration"
> to zero when perform the VACUUM(or ANALYZE) VERBOSE. "VERBOSE" is
> provided only for changing the log level of Manual VACUUM from
> DEBUG2 to INFO.
Well, I see your point but this is not completely true: we could as
well rely entirely on this parameter instead of VACOPT_VERBOSE to
determine if autovacuum, a vacuum or an analyze are in verbose mode,
and remove VACOPT_VERBOSE, but I can imagine people complaining if
VACOPT_VERBOSE is removed. So let's set it up unconditionally to -1 in
gram.y for now. However if people think that it is fine to remove
VACOPT_VERBOSE, we could use exclusively this parameter in VacuumStmt.
Or even add sanity checks at the top of vacuum() to ensure that
VACOPT_VERBOSE is set only when log_min_duration is positive.
Additional opinions on this matter are welcome.
--
Michael
