Re: add PROCESS_MAIN to VACUUM

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: add PROCESS_MAIN to VACUUM
Date: 2023-03-01 06:31:48
Message-ID: Y/7xVESfLgw3E/F/@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 19, 2023 at 11:08:07AM -0800, Nathan Bossart wrote:
> rebased

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.

@@ -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?

+-- 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.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2023-03-01 06:35:58 Re: LOG: invalid record length at <LSN> : wanted 24, got 0
Previous Message Amit Kapila 2023-03-01 06:21:49 Re: pg_upgrade and logical replication