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