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