Date: Thu, 20 Dec 2018 11:05:22 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: A few new options for vacuumdb
Message-ID: <20181220020522.GM27104@paquier.xyz>
References: <FFE5373C-E26A-495B-B5C8-911EC4A41C5E@amazon.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="31zvzas5NXT9fief"
Content-Disposition: inline
In-Reply-To: <FFE5373C-E26A-495B-B5C8-911EC4A41C5E@amazon.com>
Precedence: bulk


--31zvzas5NXT9fief
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Dec 19, 2018 at 08:50:10PM +0000, Bossart, Nathan wrote:
> If an option is specified for a server version that is not supported,
> the option is silently ignored.  For example, SKIP_LOCKED was only
> added to VACUUM and ANALYZE for v12.  Alternatively, I think we could
> fail in vacuum_one_database() if an unsupported option is specified.
> Some of these options will work on all currently supported versions,
> so I am curious what others think about skipping some of these version
> checks altogether.

prepare_vacuum_command() already handles that by ignoring silently
unsupported options (see the case of SKIP_LOCKED).  So why not doing the
same?

> It does not seem clear whether the user wants us to process mytable
> only if it is at least 1 GB, or if we should process mytable in
> addition to any other relations over 1 GB.  Either way, I think trying
> to support these combinations of options adds more complexity than it
> is worth.

It seems to me that a combination of both options means that the listed
table should be processed only if its minimum size is 1GB.  If multiple
tables are specified with --table, then only those reaching 1GB would be
processed.  So this restriction can go away.  The same applies for the
proposed --min-xid-age and --min-mxid-age.

+       <para>
+        Only execute the vacuum or analyze commands on tables with a multixact
+        ID age of at least <replaceable
class="parameter">mxid_age</replaceable>.
+       </para>
Adding a link to explain the multixact business may be helpful, say
vacuum-for-multixact-wraparound.  Same comment for XID.

> 0001 is a minor fix that is somewhat separate from these new options,
> although the new options will make the edge case it aims to fix much
> easier to reach.  When the catalogs are queried in parallel mode to
> get the list of tables to process, we currently assume that at least
> one table will be returned.  If no tables are found, the tables
> variable will stay as NULL, which leads to database-wide VACUUM or
> ANALYZE commands.  Since there are currently no user-configurable
> options available for this catalog query, this case is likely
> exceptionally rare.  However, with the new options, it is much easier
> to inadvertently filter out all relations.

Agreed.  No need to visibly bother about that in back-branches.

There is an argument about also adding DISABLE_PAGE_SKIPPING.
--
Michael

--31zvzas5NXT9fief
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAlwa+OIACgkQnvQgOdby
QH0HtA//SYrFy5vOrUY7hnW2VNNpSuIVdrEEHKRjNTuPcBWSY84L209XuCKMG/9U
+icL8bXWfC8xswtwOQ7IUSKs2rBqpwqzbCmUDbuse8ZG78UFOdbp1zYWaGMutAkN
nL/G45K4lJvtN3QJbNG4XbbaWPj53at5BWs7C0gPjKbd3PQCP3JiaziW3wrL9TAW
QK/zqCIseY0MNH0qZLj1G1uvoUJEOaZXT9AEyJUh0j3DsETF36SwuZ7QwjKijWLu
67FM/mIU5VAoS0KS+72exQAOacNRCjOS1NkVIfRbQGFrpHuIoKcf5Hg2FfC9vb7D
oZDFxVFA65ZCrtmUSRqc+8wPBc5/GlYE6FUaXeWWoPpe2r7z1/ZnzVXW/4CXUKsk
oxIaGVl/DyAW1Zf1zdvxMqWKM+jdke2BpvXR8y7qfACfXt1GIuWLG8SmWNVwqXX3
9AE/gu9cqnZ7XIeHr4HOO3QqVAEzFxCrMsByBNScIPsFmGvK0mCu7rXgUOxB8Iev
1AZFXq5YB0SMXrw+/0jWBBqNqhDcpggDtL80Gl3H/OhUt++srLiXI89x97PKQZHb
hVT9N84McuGhCcLHvYUZprtxOiNH+nNHTYdGU4bXXklIrm7ZPbWNoDNVNdet+QGJ
gu8g2xkRgwthxfq2BEqtSh9kzOjncEmDkYeriSAHy7TVHgveHUc=
=HoQU
-----END PGP SIGNATURE-----

--31zvzas5NXT9fief--