On Sat, Jan 5, 2019 at 8:50 AM Bossart, Nathan <bossa...@amazon.com> wrote: > > On 12/21/18, 11:14 AM, "Bossart, Nathan" <bossa...@amazon.com> wrote: > > On 12/21/18, 10:51 AM, "Robert Haas" <robertmh...@gmail.com> wrote: > >> On Thu, Dec 20, 2018 at 11:48 AM Bossart, Nathan <bossa...@amazon.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
