Date: Wed, 9 Jan 2019 10:06:37 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: A few new options for vacuumdb
Message-ID: <20190109010637.GC21835@paquier.xyz>
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>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="69pVuxX8awAiJ7fD"
Content-Disposition: inline
In-Reply-To: <FF67D651-A7AF-4E70-9401-805E3CFB4DC9@amazon.com>
Precedence: bulk


--69pVuxX8awAiJ7fD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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:
>=20
> +       <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</co=
mmand>.
> +       </para>
>=20
> 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

--69pVuxX8awAiJ7fD
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAlw1SR0ACgkQnvQgOdby
QH0ZXA/+NCTgn5SGBc6hUyp6VluooVrpJEYNcg7Jun+O6o4IPQr4IImAjA3kPP5J
rMIQW0aP3WPWJ8nRkxTXJcfBCG/gE7TQPUIrDPS5EYQKrojofOl2Kuubk73nTiVE
oDF6SPaKWXShMWvklNjY6cgTMnD5yzhGHbuAjeCrFW8EleMU+vOXpyNn+LbeZdzK
vcY4hm9UF0Dk1UTzJJnxGwWFZANoDuTt2CQYzazZwwebxmnD1qc9phgV4XcwI7zX
2pKO4GT9+7HvBDnm5FmSYduqIHZlbFoUpCZTQ9m0W99TXHhoI5iH19lVPjEG+YPQ
0TX3l33xF7znpsYYCcfXxX/7UMpPvBBV0U2nLVm/8rlLJ59hMe5m7fdYfdXEiwL5
l3Nv15kvuTBMdfzQQTDLMTCBDEtM4kmCScRTTpAU3Ehfw1DvElRrY3aeuZb07qIf
nwHgGwq8icWymyIoWaKFUOmRYdDe+v4wqr4CcRQr2/ZIjf/BSYpis1Qs5DhaumET
l4kR1LfT8Y2eZFQDVCOMudE4AsNY/3P/qM0fQREnretXLEomgB3AhDeosIZU28Tx
Cc1jp/LauF2vASEOwm1+b5Lxa652SuiMLxbjWSdhCZaRxHNhy17A0fjVoPJ1FZZ3
0AQMk3hP+Yat0xOqG62I24Z9d6T6188SetclQyBTVNPk1VArSiA=
=yatA
-----END PGP SIGNATURE-----

--69pVuxX8awAiJ7fD--