From: Michael Paquier <michael@paquier.xyz>
To: Bossart, Nathan <bossartn@amazon.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: A few new options for vacuumdb
Date: Wed, 9 Jan 2019 10:06:37 +0900
Message-ID: <20190109010637.GC21835@paquier.xyz> (raw)
In-Reply-To: <FF67D651-A7AF-4E70-9401-805E3CFB4DC9@amazon.com>
References: <FFE5373C-E26A-495B-B5C8-911EC4A41C5E@amazon.com>
<20181220020522.GM27104@paquier.xyz>
<7B4ACE8E-5D81-416B-B984-17E60F7ABABA@amazon.com>
<CA+TgmoaQ+h-gNAn3LL3Sn=_JVOGg8QDiRY1V6t4ECqPVWFTH=g@mail.gmail.com>
<33F59F9F-78A6-4ECE-BFE0-B1E4AAF40094@amazon.com>
<579F2B3A-4EA5-4032-AAD9-4AEE9219A4AB@amazon.com>
<CAD21AoDHsFx_Rnp-AB2UT2ORc3_PmCx18-wfrA_38i038D5WZQ@mail.gmail.com>
<20190108020300.GH22498@paquier.xyz>
<FF67D651-A7AF-4E70-9401-805E3CFB4DC9@amazon.com>
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.
> I've realized that I forgot to add links to the XID/MXID wraparound
> documentation like you suggested a while back. I'll make sure that
> gets included in the next patch set, too.
Nice, thanks.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: michael@paquier.xyz, bossartn@amazon.com, sawada.mshk@gmail.com, robertmhaas@gmail.com
Subject: Re: A few new options for vacuumdb
In-Reply-To: <20190109010637.GC21835@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox