Re: Table-level log_autovacuum_min_duration

From: Naoya Anzai <anzai-naoya(at)mxu(dot)nes(dot)nec(dot)co(dot)jp>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Akio Iwaasa <iwaasa(at)mxs(dot)nes(dot)nec(dot)co(dot)jp>
Subject: Re: Table-level log_autovacuum_min_duration
Date: 2015-02-09 06:47:12
Message-ID: 116262CF971C844FB6E793F8809B51C6E361A1@BPXM02GP.gisp.nec.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for your reply.

>> 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 disscussed 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. :)

>> 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.

>> Architecture review
>> ====================
>> About the modification of gram.y.
>>
>> I think it is not good that log_min_duration is initialized to
>> zeros in gram.y when "FREEZE" option is set. Because any VACUUM
>> queries never use this parameter. I think log_min_duration always
>> should be initialized to -1.
>
>Looking at this patch this morning, actually I think that my last
>patch as well as your suggestion are both wrong. To put it in short
>words, log_min_duration should be set only if VACOPT_VERBOSE is
>defined in query declaration. So I changed this portion of the patch
>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.

Regards,
-----
Naoya.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2015-02-09 07:12:36 Re: RangeType internal use
Previous Message Fujii Masao 2015-02-09 06:18:04 Re: [REVIEW] Re: Compression of full-page-writes