Re: Add MAIN_RELATION_CLEANUP and SECONDARY_RELATION_CLEANUP options to VACUUM

From: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>, "k(dot)jamison(at)fujitsu(dot)com" <k(dot)jamison(at)fujitsu(dot)com>
Cc: 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-05 00:56:48
Message-ID: FD08FEE4-CA01-424F-823F-0D7426EDB3D4@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 8/2/20, 11:47 PM, "Michael Paquier" <michael(at)paquier(dot)xyz> wrote:
> + 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.

My main motive for adding the MAIN_RELATION_CLEANUP option is to allow
table owners to easily vacuum only a relation's TOAST table. Roles do
not have access to the pg_toast schema by default, so they might be
restricted from vacuuming their TOAST tables directly.

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

I think it would be possible to skip calling vacuum_rel() from
expand_vacuum_rel()/get_all_vacuum_rels() as appropriate, but when I
looked into that approach originally, I was concerned that it would
add complexity to the lookups and ownership checks (especially the
partition lookup logic). The main tradeoffs of the approach I went
with are that we still create a transaction for the main relation and
that we still lock the main relation.

Nathan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2020-08-05 01:04:28 Re: FailedAssertion("pd_idx == pinfo->nparts", File: "execPartition.c", Line: 1689)
Previous Message Amit Langote 2020-08-05 00:53:44 Re: FailedAssertion("pd_idx == pinfo->nparts", File: "execPartition.c", Line: 1689)