From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: vignesh C <vignesh21@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: add PROCESS_MAIN to VACUUM
Date: Wed, 1 Mar 2023 09:26:37 -0800
Message-ID: <20230301172637.GA1456203@nathanxps13> (raw)
In-Reply-To: <Y/7xVESfLgw3E/F/@paquier.xyz>
References: <20221230000028.GA435655@nathanxps13>
<20230107050724.GA914213@nathanxps13>
<CALDaNm1bKSNG9Jc1mrtk2ZWsVdvTQw+XOnbB3yL_Xf57oAdg5Q@mail.gmail.com>
<20230119190807.GA4062411@nathanxps13>
<Y/7xVESfLgw3E/F/@paquier.xyz>
Thanks for taking a look.
On Wed, Mar 01, 2023 at 03:31:48PM +0900, Michael Paquier wrote:
> PROCESS_TOAST has that:
> /* sanity check for PROCESS_TOAST */
> if ((params->options & VACOPT_FULL) != 0 &&
> (params->options & VACOPT_PROCESS_TOAST) == 0)
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("PROCESS_TOAST required with VACUUM FULL")));
> [...]
> - if (params->options & VACOPT_FULL)
> + if (params->options & VACOPT_FULL &&
> + params->options & VACOPT_PROCESS_MAIN)
> {
>
> Shouldn't we apply the same rule for PROCESS_MAIN? One of the
> regression tests added means that FULL takes priority over
> PROCESS_MAIN=FALSE, which is a bit confusing IMO.
I don't think so. We disallow FULL without PROCESS_TOAST because there
presently isn't a way to VACUUM FULL the main relation without rebuilding
its TOAST table. However, FULL without PROCESS_MAIN can be used to run
VACUUM FULL on only the TOAST table.
> @@ -190,6 +190,7 @@ typedef struct VacAttrStats
> #define VACOPT_DISABLE_PAGE_SKIPPING 0x80 /* don't skip any pages */
> #define VACOPT_SKIP_DATABASE_STATS 0x100 /* skip vac_update_datfrozenxid() */
> #define VACOPT_ONLY_DATABASE_STATS 0x200 /* only vac_update_datfrozenxid() */
> +#define VACOPT_PROCESS_MAIN 0x400 /* process main relation */
>
> Perhaps the options had better be reorganized so as PROCESS_MAIN is
> just before PROCESS_TOAST?
Sure.
> +-- PROCESS_MAIN option
> +VACUUM (PROCESS_MAIN FALSE) vactst;
> +VACUUM (PROCESS_MAIN FALSE, PROCESS_TOAST FALSE) vactst;
> +VACUUM (PROCESS_MAIN FALSE, FULL) vactst;
>
> Thinking a bit here. This set of tests does not make sure that the
> main relation and/or the toast relation have been actually processed.
> pg_stat_user_tables does not track what's happening on the toast
> relations. So... What about adding some tests in 100_vacuumdb.pl
> that rely on vacuumdb --verbose and check the logs produced? We
> should make sure that the toast or the main relation are processed,
> by tracking, for example, logs like vacuuming "schema.table". When
> FULL is involved, we may want to track the changes on relfilenodes
> depending on what's wanted.
That seems like a good idea.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
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: nathandbossart@gmail.com, michael@paquier.xyz, vignesh21@gmail.com
Subject: Re: add PROCESS_MAIN to VACUUM
In-Reply-To: <20230301172637.GA1456203@nathanxps13>
* 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