Re: Add MAIN_RELATION_CLEANUP and SECONDARY_RELATION_CLEANUP options to VACUUM

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: "k(dot)jamison(at)fujitsu(dot)com" <k(dot)jamison(at)fujitsu(dot)com>
Cc: "'Bossart, Nathan'" <bossartn(at)amazon(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add MAIN_RELATION_CLEANUP and SECONDARY_RELATION_CLEANUP options to VACUUM
Date: 2020-08-03 06:47:13
Message-ID: 20200803064713.GM3317@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 14, 2020 at 05:34:01AM +0000, k(dot)jamison(at)fujitsu(dot)com wrote:
> I've also confirmed those through regression + tap test in my own env
> and they've passed. I'll look into deeply again if I find problems.

+ VACOPT_TOAST_CLEANUP = 1 << 6, /* process TOAST table, if any */
+ VACOPT_DISABLE_PAGE_SKIPPING = 1 << 7, /* don't skip any pages */
+ VACOPT_MAIN_REL_CLEANUP = 1 << 8 /* process main relation */
} VacuumOption;

Do we actually need this much complication in the option set? It is
possible to vacuum directly a toast table by passing directly its
relation name, with pg_toast as schema, so you can already vacuum a
toast relation without the main part. And I would guess that users
caring about the toast table specifically would know already how to do
that, even if it requires a simple script and a query on pg_class.
Now there is a second part, where we'd like to vacuum the main
relation but not its toast table. My feeling by looking at this patch
today is that we could just make VACOPT_SKIPTOAST an option available
at user-level, and support all the cases discussed on this thread.
And we have already all the code in place to support that in the
backend for autovacuum as relations are processed individually,
without their toast tables if they have one.

> I think this follows the similar course of previously added VACUUM and
> vacuummdb options (for using and skipping truncate, index cleanup, etc.),
> so the patch seems almost plausible enough for me.

-static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params);
+static bool vacuum_rel(Oid relid,
+ RangeVar *relation,
+ VacuumParams *params,
+ bool processing_toast_table);

Not much a fan of the addition of this parameter on this routine to
track down if the call should process a toast relation or not.
Couldn't you just prevent the call to vacuum_rel() to happen at all?
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2020-08-03 06:53:15 Re: Make autovacuum sort tables in descending order of xid_age
Previous Message Noah Misch 2020-08-03 06:30:50 Re: public schema default ACL