Table-level log_autovacuum_min_duration

Michael Paquier wrote:

Hi all,

Today I spent a bit of time looking at the activity of autovacuum for
one table particularly bloated. log_autovacuum_min_duration was
enabled and set to a high value but even with that I had to deal with
some spam from the jobs of other tables. It would be cool to have the
possibility to do some filtering IMO. So, what about having a relopt
to control log_autovacuum_min_duration? RelationData->rd_options is
largely accessible for a given relation in the code paths of vacuum
and analyze.

+1

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Dec 18, 2014 at 11:15 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Michael Paquier wrote:

Hi all,

Today I spent a bit of time looking at the activity of autovacuum for
one table particularly bloated. log_autovacuum_min_duration was
enabled and set to a high value but even with that I had to deal with
some spam from the jobs of other tables. It would be cool to have the
possibility to do some filtering IMO. So, what about having a relopt
to control log_autovacuum_min_duration? RelationData->rd_options is
largely accessible for a given relation in the code paths of vacuum
and analyze.

+1

As long as I got this idea in mind, I looked at the code to see how
much it would be possible to tune log_autovacuum_min_duration in the
code paths of analyze and vacuum. First, I think that it would be good
to have parameters for both parent relations and their toast relation
similarly to the other parameters...

But now, here are some things to consider if we use directly the
reloptions available with RelationData:
- If the parameters toast.autovacuum_* are not set, toast relations
inherit values from their parent relation. Looking at autovacuum.c
which is the only place where autovacuum options are located, we keep
a hash table to save the mapping toast -> parent relation. Using that
it is easy to fetch for a given toast relation the relopts of its
parent. Note this is strictly located in the autovacuum code path, so
to let vacuum and analyze now the custom value of
log_autovacuum_min_duration, with the inheritance property kept, we
would need to pass some extra values from autovacuum to the calls of
vacuum().
- It would not be possible to log autovacuum and analyze being skipped
when lock is not available because in this case Relation cannot be
safely fetched, so there are no reltoptions directly available. This
is for example this kind of message:
skipping analyze of "foo" --- lock not available

Both those things could be solved by passing a value through
VacuumStmt, like what we do when passing a value for
multixact_freeze_min_age, or with an extra argument in vacuum() for
example. Now I am not sure if it is worth it, and we could live as
well with a parameter that do not support the inheritance parent
relation -> toast, so log value set for a toast relation and its
parent share no dependency. In short that's a trade between code
simplicity and usability.

Thoughts?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Dec 20, 2014 at 9:17 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

But now, here are some things to consider if we use directly the
reloptions available with RelationData:
- If the parameters toast.autovacuum_* are not set, toast relations
inherit values from their parent relation. Looking at autovacuum.c
which is the only place where autovacuum options are located, we keep
a hash table to save the mapping toast -> parent relation. Using that
it is easy to fetch for a given toast relation the relopts of its
parent. Note this is strictly located in the autovacuum code path, so
to let vacuum and analyze now the custom value of
log_autovacuum_min_duration, with the inheritance property kept, we
would need to pass some extra values from autovacuum to the calls of
vacuum().
- It would not be possible to log autovacuum and analyze being skipped
when lock is not available because in this case Relation cannot be
safely fetched, so there are no reltoptions directly available. This
is for example this kind of message:
skipping analyze of "foo" --- lock not available

Both those things could be solved by passing a value through
VacuumStmt, like what we do when passing a value for
multixact_freeze_min_age, or with an extra argument in vacuum() for
example. Now I am not sure if it is worth it, and we could live as
well with a parameter that do not support the inheritance parent
relation -> toast, so log value set for a toast relation and its
parent share no dependency. In short that's a trade between code
simplicity and usability.

I'm not sure I follow all of the particulars of what you are asking
here, but in general I would say that you shouldn't hesitate to pass
more information down the call stack when that helps to obtain correct
behavior.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Dec 20, 2014 at 11:17 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Michael Paquier wrote:

Today I spent a bit of time looking at the activity of autovacuum for
one table particularly bloated. log_autovacuum_min_duration was
enabled and set to a high value but even with that I had to deal with
some spam from the jobs of other tables. It would be cool to have the
possibility to do some filtering IMO. So, what about having a relopt
to control log_autovacuum_min_duration? RelationData->rd_options is
largely accessible for a given relation in the code paths of vacuum
and analyze.

OK, instead of only words, attached is a patch adding relopts called
log_autovacuum_min_duration and toast.log_autovacuum_min_duration to
control the logging of autovacuum at relation-level. The default value
of those parameters is -1, meaning that in this case the global
log_autovacuum_min_duration is used to control the logs. The patch has
finished by being simpler than I though first by using VacuumStmt to
pass the relopts from autovacuum to the code paths of analyze and
vacuum. Note that this uses the same mechanisms as the other relopts,
meaning that the toast relations use the values of their parent tables
if it is defined.

I am adding it to the next CF.
--
Michael

I wrote:

I am adding it to the next CF.

Patch 2 attached. I forgot the update of copyfuncs.c and equalfuncs.c.
--
Michael

The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, failed
Spec compliant: tested, failed
Documentation: tested, failed

Hi,

I'm Naoya Anzai.
I've been working as a PostgreSQL Support Engineer for 6 years.
I am a newcomer of reviewing, and My programming skill is not so high.
But the following members also participate in this review. (We say
"Two heads are better than one." :))

Akira Kurosawa <kurosawa-akira@mxc.nes.nec.co.jp>
Taiki Kondo <kondo-taiki@mxt.nes.nec.co.jp>
Huong Dangminh <dangminh-huong@mxm.nes.nec.co.jp>

So I believe reviewing patches is not difficult for us.

This is a review of Table-level log_autovacuum_min_duration patch:
/messages/by-id/CAB7nPqTBQsbEgvb8cOH01K7ARGYs9KBuV8dr+aqGonFVb8koAQ@mail.gmail.com

Submission review
====================
The patch applies cleanly to HEAD, and it works fine on Fedora
release 20.
There is no regression test,but I think it is no problem
because other parameter also is not tested.

Usability review
====================
pg_dump/pg_restore support is OK.
I think this feature is a nice idea and I also want this feature.

Feature test
====================
I executed following commands after setting
"log_autovacuum_min_duration" to 1000 in the GUC. ("bar" table is
already created with no options.)

CREATE TABLE foo ( ... ) WITH ( log_autovacuum_min_duration = 0 );
ALTER TABLE bar SET (log_autovacuum_min_duration = 0 );

Then, only in "foo" and "bar" table, autovacuum log was printed out
even if elapsed time of autovacuum is lesser than 1000ms. This
behavior was expected and there was no crash or failed assertion.
So it looked good for me. But, I executed following command, in
"buzz" table, autovacuum log was printed out if elapsed time is
more than 1000ms.

CREATE TABLE buzz ( ... ) WITH ( log_autovacuum_min_duration = -1 );
^^

I expect autovacuum log is NOT printed out even if elapsed time is
more than 1000ms in this case. My thought is wrong, isn't it? In my
opinion, there is an use case that autovacuum log is always printed
out in all tables excepting specified tables. I think there is a
person who wants to use it like this case, but I (or he) can NOT use
in this situation.

How about your opinion?

Performance review
====================
Not reviewed from this point of view.

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

There is no new function which is defined in this patch, and
modified contents are not related to OS-dependent contents, so I
think it will work fine on Windows/BSD etc.

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.

Regards,
Naoya Anzai (NEC Solution Innovators,Ltd.)

The new status of this patch is: Waiting on Author

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, failed
Spec compliant: tested, failed
Documentation: tested, failed

I'm sorry, I just sent it by mistake.
All of them have passed.

---
Naoya Anzai

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Feb 6, 2015 at 4:50 AM, Naoya Anzai
<anzai-naoya@mxu.nes.nec.co.jp> wrote:

The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, failed
Spec compliant: tested, failed
Documentation: tested, failed

I'm sorry, I just sent it by mistake.
All of them have passed.

That's fine. I think you used the UI on the commit fest app, and it is
not intuitive that you need to check those boxes at first sight when
using it for the first time.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Feb 6, 2015 at 4:39 AM, Naoya Anzai wrote:

I've been working as a PostgreSQL Support Engineer for 6 years.

Thanks for your input Anzai-san.

I am a newcomer of reviewing, and My programming skill is not so high.
But the following members also participate in this review. (We say
"Two heads are better than one." :))

FWIW, any review for any kind of patches are always welcome, either
knowing or not the code you are looking at for a given patch. It is
even encouraged to look at new areas when possible.

Feature test
====================
[blah]
CREATE TABLE buzz ( ... ) WITH ( log_autovacuum_min_duration = -1 );
I expect autovacuum log is NOT printed out even if elapsed time is
more than 1000ms in this case. My thought is wrong, isn't it? In my
opinion, there is an use case that autovacuum log is always printed
out in all tables excepting specified tables. I think there is a
person who wants to use it like this case, but I (or he) can NOT use
in this situation.

How about your opinion?

The patch is working as expected. Similarly to the other parameters
like vacuum_cost_delay, a value of -1 is the default behavior, meaning
that the underlying GUC parameter is used. 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;
And you can actually get what you are looking for by setting
min_duration through ALTER TABLE to a very high value, like say 2e9 to
suppress the autovacuum output of a set of tables out of the rest.

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"

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

On Fri, Feb 6, 2015 at 11:14 AM, Michael Paquier wrote:

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

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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

An updated patch is attached, I think that previous version broke value
assignment of log_min_duration in table_recheck_autovac: the value in
reltoptions should be used only if it is positive. At the same time I wrote
some more documentation about the fact that a toast table will use the
value of its parent table if nothing is set.
--
Michael

Hi, Michael-san

An updated patch is attached,

I'm sorry for confusing you.

I think you don't have to implement this code to disable this
feature with using value "-2".Because this use case is a rare case,
and there is a practical workaround using huge value like "2e9".
(You suggested "2e9" to me, didn't you? :) ) So, please remove this code.

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.

I understand your point at last. :)

You mean that ...
Log_autovacuum_min_duration assumes a role of VACOPT_VERBOSE.
Even if this parameter never use currently for manual vacuum,
log_autovacuum_min_duration should be set zero(anytime output)
when we executes "VACUUM(or ANALYZE) VERBOSE".
Is my understanding correct? If so,it sounds logical.

If we can abolish VERBOSE option,
I think it's ideal that we will prepare a new parameter like
a log_min_duration_vacuum(and log_min_duration_analyze) which
integrating "VERBOSE feature" and "log_autovacuum_min_duration".

Regards,
---
Naoya Anzai

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Feb 12, 2015 at 5:44 PM, Naoya Anzai <anzai-naoya@mxu.nes.nec.co.jp>
wrote:

Hi, Michael-san

An updated patch is attached,

I'm sorry for confusing you.

I think you don't have to implement this code to disable this
feature with using value "-2".Because this use case is a rare case,
and there is a practical workaround using huge value like "2e9".
(You suggested "2e9" to me, didn't you? :) ) So, please remove this code.

I will clean up the code.

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.

I understand your point at last. :)

You mean that ...
Log_autovacuum_min_duration assumes a role of VACOPT_VERBOSE.
Even if this parameter never use currently for manual vacuum,
log_autovacuum_min_duration should be set zero(anytime output)
when we executes "VACUUM(or ANALYZE) VERBOSE".
Is my understanding correct? If so,it sounds logical.

Yup, that's my opinion. Now I don't know if people would mind to remove
VACOPT_VERBOSE and replace the control it does by log_min_duration in
VacuumStmt. At least both things are overlapping, and log_min_duration
offers more options than the plain VACOPT_VERBOSE.

If we can abolish VERBOSE option,
I think it's ideal that we will prepare a new parameter like
a log_min_duration_vacuum(and log_min_duration_analyze) which
integrating "VERBOSE feature" and "log_autovacuum_min_duration".

What I think you are proposing here is a VERBOSE option that hypothetically
gets activated if a manual VACUUM takes more than a certain amount
specified by those parameters. I doubt this would be useful. In any case
this is unrelated to this patch.
--
Michael

You mean that ...
Log_autovacuum_min_duration assumes a role of VACOPT_VERBOSE.
Even if this parameter never use currently for manual vacuum,
log_autovacuum_min_duration should be set zero(anytime output)
when we executes "VACUUM(or ANALYZE) VERBOSE".
Is my understanding correct? If so,it sounds logical.

Yup, that's my opinion. Now I don't know if people would mind to remove
VACOPT_VERBOSE and replace the control it does by log_min_duration in
VacuumStmt. At least both things are overlapping, and log_min_duration
offers more options than the plain VACOPT_VERBOSE.

OK. I agree with you.
Please re-implement as you are thinking.

If we can abolish VERBOSE option,
I think it's ideal that we will prepare a new parameter like
a log_min_duration_vacuum(and log_min_duration_analyze) which
integrating "VERBOSE feature" and "log_autovacuum_min_duration".

What I think you are proposing here is a VERBOSE option that hypothetically
gets activated if a manual VACUUM takes more than a certain amount
specified by those parameters. I doubt this would be useful. In any case
this is unrelated to this patch.

I suspect manual vacuum often executes as "semi-auto vacuum"
by job-scheduler, cron, etc in actual maintenance cases.
Whether auto or manual, I think that's important to output
their logs in the same mechanism.

Sorry, I seem to have wandered from the subject.

Naoya

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Feb 13, 2015 at 10:16 AM, Naoya Anzai
<anzai-naoya@mxu.nes.nec.co.jp> wrote:

You mean that ...
Log_autovacuum_min_duration assumes a role of VACOPT_VERBOSE.
Even if this parameter never use currently for manual vacuum,
log_autovacuum_min_duration should be set zero(anytime output)
when we executes "VACUUM(or ANALYZE) VERBOSE".
Is my understanding correct? If so,it sounds logical.

Yup, that's my opinion. Now I don't know if people would mind to remove
VACOPT_VERBOSE and replace the control it does by log_min_duration in
VacuumStmt. At least both things are overlapping, and log_min_duration
offers more options than the plain VACOPT_VERBOSE.

OK. I agree with you.
Please re-implement as you are thinking.

OK will do that today.

If we can abolish VERBOSE option,
I think it's ideal that we will prepare a new parameter like
a log_min_duration_vacuum(and log_min_duration_analyze) which
integrating "VERBOSE feature" and "log_autovacuum_min_duration".

What I think you are proposing here is a VERBOSE option that hypothetically
gets activated if a manual VACUUM takes more than a certain amount
specified by those parameters. I doubt this would be useful. In any case
this is unrelated to this patch.

I suspect manual vacuum often executes as "semi-auto vacuum"
by job-scheduler, cron, etc in actual maintenance cases.
Whether auto or manual, I think that's important to output
their logs in the same mechanism.

Sorry, I seem to have wandered from the subject.

No problem. That's a constructive discussion :)
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Feb 13, 2015 at 10:16 AM, Naoya Anzai
<anzai-naoya@mxu.nes.nec.co.jp> wrote:

You mean that ...
Log_autovacuum_min_duration assumes a role of VACOPT_VERBOSE.
Even if this parameter never use currently for manual vacuum,
log_autovacuum_min_duration should be set zero(anytime output)
when we executes "VACUUM(or ANALYZE) VERBOSE".
Is my understanding correct? If so,it sounds logical.

Yup, that's my opinion. Now I don't know if people would mind to remove
VACOPT_VERBOSE and replace the control it does by log_min_duration in
VacuumStmt. At least both things are overlapping, and log_min_duration
offers more options than the plain VACOPT_VERBOSE.

OK. I agree with you.
Please re-implement as you are thinking.

Thanks. Attached is an updated patch will all those things implemented.

If we can abolish VERBOSE option,
I think it's ideal that we will prepare a new parameter like
a log_min_duration_vacuum(and log_min_duration_analyze) which
integrating "VERBOSE feature" and "log_autovacuum_min_duration".

What I think you are proposing here is a VERBOSE option that hypothetically
gets activated if a manual VACUUM takes more than a certain amount
specified by those parameters. I doubt this would be useful. In any case
this is unrelated to this patch.

I suspect manual vacuum often executes as "semi-auto vacuum"
by job-scheduler, cron, etc in actual maintenance cases.
Whether auto or manual, I think that's important to output
their logs in the same mechanism.

Sorry, I seem to have wandered from the subject.

This patch is a step in this direction as it enables any backend-side
code to set up VacuumStmt with a custom timestamp value to output logs
after a given timing, for example in background workers. For the
client-side of things, I am unsure if we'd actually want it, we should
always VERBOSE when this option is invoked through a query, and not
add any hypothetical condition on it...

Btw, after hacking on this it happens that we cannot completely remove
VACOPT_VERBOSE as gram.y needs to take into account options with
parenthesis :)
But we can limit its use to the query parser only, similarly for VACOPT_FREEZE.
--
Michael

Hi, Michael

I found that definition of VERBOSE and log_autovacuum is not
pretty match. For example, "VERBOSE" can output logs of
scanning indices and scanning detail of analyze, but
log_autovacuum can't output them.

Please see following sequences.

1. execute these queries.

DROP TABLE t1;
CREATE TABLE t1(id integer primary key,name text);
ALTER TABLE t1 SET (log_autovacuum_min_duration=0);
ALTER TABLE t1 ALTER COLUMN name SET STORAGE EXTERNAL;
INSERT INTO t1 SELECT GENERATE_SERIES(1,100),repeat('a',3000);
UPDATE t1 SET name='update';

2. after a while, output the following logs by autovacuum movement.
(For your convenience, I put the "### TYPE ###" prefix on each logs.)

### VERBOSE ###INFO: vacuuming "public.t1"
### VERBOSE ###INFO: scanned index "t1_pkey" to remove 33 row versions
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: "t1": removed 33 row versions in 1 pages
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: index "t1_pkey" now contains 100 row versions in 2 pages
### VERBOSE ###DETAIL: 33 index row versions were removed.
### VERBOSE ### 0 index pages have been deleted, 0 are currently reusable.
### VERBOSE ### CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: "t1": found 100 removable, 100 nonremovable row versions in 2 out of 2 pages
### VERBOSE ###DETAIL: 0 dead row versions cannot be removed yet.
### VERBOSE ### There were 0 unused item pointers.
### VERBOSE ### Skipped 0 pages due to buffer pins.
### VERBOSE ### 0 pages are entirely empty.
### VERBOSE ### CPU 0.00s/0.00u sec elapsed 0.00 sec.
### LOG_AVAC ###LOG: automatic vacuum of table "naoya.public.t1": index scans: 1
### LOG_AVAC ### pages: 0 removed, 2 remain, 0 skipped due to pins
### LOG_AVAC ### tuples: 100 removed, 100 remain, 0 are dead but not yet removable
### LOG_AVAC ### buffer usage: 47 hits, 4 misses, 4 dirtied
### LOG_AVAC ### avg read rate: 14.192 MB/s, avg write rate: 14.192 MB/s
### LOG_AVAC ### system usage: CPU 0.00s/0.00u sec elapsed 0.00 sec
### VERBOSE ###INFO: analyzing "public.t1"
### VERBOSE ###INFO: "t1": scanned 2 of 2 pages, containing 100 live rows and 0 dead rows; 100 rows in sample, 100 estimated total rows
### LOG_AVAC ###LOG: automatic analyze of table "naoya.public.t1" system usage: CPU 0.00s/0.00u sec elapsed 0.04 sec
### VERBOSE ###INFO: vacuuming "pg_toast.pg_toast_72882"
### VERBOSE ###INFO: scanned index "pg_toast_72882_index" to remove 200 row versions
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: "pg_toast_72882": removed 200 row versions in 50 pages
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: index "pg_toast_72882_index" now contains 0 row versions in 2 pages
### VERBOSE ###DETAIL: 200 index row versions were removed.
### VERBOSE ### 0 index pages have been deleted, 0 are currently reusable.
### VERBOSE ### CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: "pg_toast_72882": found 200 removable, 0 nonremovable row versions in 50 out of 50 pages
### VERBOSE ###DETAIL: 0 dead row versions cannot be removed yet.
### VERBOSE ### There were 0 unused item pointers.
### VERBOSE ### Skipped 0 pages due to buffer pins.
### VERBOSE ### 0 pages are entirely empty.
### VERBOSE ### CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: "pg_toast_72882": truncated 50 to 0 pages
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.03 sec.
### LOG_AVAC ###LOG: automatic vacuum of table "naoya.pg_toast.pg_toast_72882": index scans: 1
### LOG_AVAC ### pages: 50 removed, 0 remain, 0 skipped due to pins
### LOG_AVAC ### tuples: 200 removed, 0 remain, 0 are dead but not yet removable
### LOG_AVAC ### buffer usage: 223 hits, 2 misses, 1 dirtied
### LOG_AVAC ### avg read rate: 0.457 MB/s, avg write rate: 0.228 MB/s
### LOG_AVAC ### system usage: CPU 0.00s/0.00u sec elapsed 0.03 sec

I think VACOPT_VERBOSE should not be easily replaced to log_min_duration>=0.

And, in this v7 patch looks that VERBOSE log is always output
in INFO-Level when log_autovacuum_min_duration is set to 0.
Is this your point?

Regards,
---
Naoya

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers