Re: A few new options for vacuumdb

On Sat, Jan 5, 2019 at 8:50 AM Bossart, Nathan <bossartn(at)amazon(dot)com> wrote:
>
> On 12/21/18, 11:14 AM, "Bossart, Nathan" <bossartn(at)amazon(dot)com> wrote:
> > On 12/21/18, 10:51 AM, "Robert Haas" <robertmhaas(at)gmail(dot)com> wrote:
> >> On Thu, Dec 20, 2018 at 11:48 AM Bossart, Nathan <bossartn(at)amazon(dot)com> wrote:
> >>> Either way, we'll still have to decide whether to fail or to silently
> >>> skip the option if you do something like specify --min-mxid-age for a
> >>> 9.4 server.
> >>
> >> +1 for fail.
> >
> > Sounds good. I'll keep all the version checks and will fail for
> > unsupported options in the next patch set.
>
> Here's an updated set of patches with the following changes:
>
> - 0002 adds the parenthesized syntax for ANALYZE.
> - 0003 adds DISABLE_PAGE_SKIPPING for VACUUM.
> - 0003 also ensures SKIP_LOCKED is applied for --analyze-only.
> - 0004 alters vacuumdb to always use the catalog query to discover
> the list of tables to process.
> - 0003, 0005, and 0006 now fail in vacuum_one_database() if a
> specified option is not available on the server version.
> - If tables are listed along with --min-xid-age, --min-mxid-age, or
> --min-relation-size, each table is processed only if it satisfies
> the provided options.
>
> 0004 introduces a slight change to existing behavior. Currently, if
> you specify a missing table, vacuumdb will process each table until
> it reaches the nonexistent one, at which point it will fail. After
> 0004 is applied, vacuumdb will fail during the catalog query, and no
> tables will be processed. I considered avoiding this change in
> behavior by doing an additional catalog lookup for each specified
> table to see whether it satisfies --min-xid-age, etc. However, this
> introduced quite a bit more complexity and increased the number of
> catalog queries needed.
>
> Nathan
>

0002 and 0003 are merged and posted by Michael-san and it looks good
to me, so I've looked at the 0001, 0004, 0005 and 0006 patches. Here
is a few review comments.

-----
Even if all tables are filtered by --min-relation-size, min-mxid-age
or min-xid-age the following message appeared.

$ vacuumdb --verbose --min-relation-size 1000000 postgres
vacuumdb: vacuuming database "postgres"

Since no tables are processed in this case isn't is better not to
output this message by moving the message after checking if ntup == 0?

-----
You use pg_total_relation_size() to check the relation size but it
might be better to use pg_relation_size() instead. The
pg_total_relation_size() includes the all index size but I think it's
common based on my experience that we check only the table size
(including toast table) to do vacuum it or not.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center