Re: A few new options for vacuumdb

On Wed, Jan 9, 2019 at 10:06 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Jan 08, 2019 at 06:46:11PM +0000, Bossart, Nathan wrote:
> > This was done in order to maintain the current behavior that
> > appendQualifiedRelation() gives us.  I found that skipping the
> > search_path handling here forced us to specify the schema in the
> > argument for --table in most cases.  At the very least, I could add a
> > comment here to highlight the importance of fully qualifying
> > everything in the catalog query.  What do you think?
>
> A comment sounds like a good thing.  And we really shouldn't hijack
> search_path even for one query...
>
> > Looks good to me, except for one small thing in the documentation:
> >
> > +       <para>
> > +        Disable all page-skipping behavior during processing based on
> > +        the visibility map, similarly to the option
> > +        <literal>DISABLE_PAGE_SKIPPING</literal> for <command>VACUUM</command>.
> > +       </para>
> >
> > I think the "similarly to the option" part is slightly misleading.
> > It's not just similar, it _is_ using that option in the generated
> > commands.  Perhaps we could point to the VACUUM documentation for more
> > information about this one.
>
> Sure.  If you have any suggestions, please feel free.  Adding a link
> to VACUUM documentation sounds good to me, as long as we avoid
> duplicating the description related to DISABLE_PAGE_SKIPPING on the
> VACUUM page.
>
> > Good point.  I think allowing multiple different relation size options
> > here would be confusing, too (e.g. --min-relation-size versus
> > --min-total-relation-size).  IMO pg_total_relation_size() is the way
> > to go here, as we'll most likely need to process the indexes and TOAST
> > tables, too.  If/when there is a DISABLE_INDEX_CLEANUP option for
> > VACUUM, we'd then want to use pg_table_size() when --min-relation-size
> > and --disable-index-cleanup are used together in vacuumdb.
> > Thoughts?
>
> Yes, using pg_total_relation_size() looks like the best option to me
> here as well, still this does not make me 100% comfortable from the
> user perspective.

Agreed.

Since pg_(total)_realtion_size() returns 0 for parent table the
specifying the parent table to vacuumdb with --min-relation-size
always does nothing. Maybe we will need to deal with this case when a
function returning whole partitoned table size is introduced.
Regards,

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