From: Nathan Bossart <nathandbossart@gmail.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: vignesh C <vignesh21@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: add PROCESS_MAIN to VACUUM
Date: Mon, 6 Mar 2023 13:13:37 -0800
Message-ID: <20230306211337.GA3076909@nathanxps13> (raw)
In-Reply-To: <CAAKRu_ZJMFrO_tspzoX43h9NyEFS1nS438MoS3Yv7JxK3=jFPw@mail.gmail.com>
References: <20230119190807.GA4062411@nathanxps13>
<Y/7xVESfLgw3E/F/@paquier.xyz>
<20230301172637.GA1456203@nathanxps13>
<ZAA5vt7jUTRpPDmL@paquier.xyz>
<20230302065359.GA2140777@nathanxps13>
<ZAWbkiZSdFZOTS7F@paquier.xyz>
<20230306173723.GA2934573@nathanxps13>
<20230306194009.5cn6sp3wjotd36nu@liskov>
<20230306202734.GA3033389@nathanxps13>
<CAAKRu_ZJMFrO_tspzoX43h9NyEFS1nS438MoS3Yv7JxK3=jFPw@mail.gmail.com>
On Mon, Mar 06, 2023 at 03:48:28PM -0500, Melanie Plageman wrote:
> On Mon, Mar 6, 2023 at 3:27 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> On Mon, Mar 06, 2023 at 02:40:09PM -0500, Melanie Plageman wrote:
>> > I noticed in vacuum_rel() in vacuum.c where table_relation_vacuum() is
>> > called, 4211fbd84 changes the else into an else if [1]. I understand
>> > after reading the commit and re-reading the code why that is now, but I
>> > was initially confused. I was thinking it might be nice to have a
>> > comment mentioning why there is no else case here (i.e. that the main
>> > table relation will be vacuumed on the else if branch).
>>
>> This was a hack to avoid another level of indentation for that whole block
>> of code, but based on your comment, it might be better to just surround
>> this entire section with an "if (params->options & VACOPT_PROCESS_MAIN)"
>> check. WDYT?
>
> I think that would be clearer.
Here's a patch. Thanks for reviewing.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.comAttachments:
[text/x-diff] process_main_readability_fix.patch (1.5K, ../20230306211337.GA3076909@nathanxps13/2-process_main_readability_fix.patch)
download | inline diff:diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.cindex 580f966499..fb1ef28fa9 100644--- a/src/backend/commands/vacuum.c+++ b/src/backend/commands/vacuum.c@@ -2060,23 +2060,25 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs)
/*
* Do the actual work --- either FULL or "lazy" vacuum
*/
- if ((params->options & VACOPT_FULL) &&- (params->options & VACOPT_PROCESS_MAIN))+ if (params->options & VACOPT_PROCESS_MAIN)
{
- ClusterParams cluster_params = {0};+ if (params->options & VACOPT_FULL)+ {+ ClusterParams cluster_params = {0};- /* close relation before vacuuming, but hold lock until commit */- relation_close(rel, NoLock);- rel = NULL;+ /* close relation before vacuuming, but hold lock until commit */+ relation_close(rel, NoLock);+ rel = NULL;- if ((params->options & VACOPT_VERBOSE) != 0)- cluster_params.options |= CLUOPT_VERBOSE;+ if ((params->options & VACOPT_VERBOSE) != 0)+ cluster_params.options |= CLUOPT_VERBOSE;- /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */- cluster_rel(relid, InvalidOid, &cluster_params);+ /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */+ cluster_rel(relid, InvalidOid, &cluster_params);+ }+ else+ table_relation_vacuum(rel, params, vac_strategy);
}
- else if (params->options & VACOPT_PROCESS_MAIN)- table_relation_vacuum(rel, params, vac_strategy);
/* Roll back any GUC changes executed by index functions */
AtEOXact_GUC(false, save_nestlevel);
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, melanieplageman@gmail.com, michael@paquier.xyz, vignesh21@gmail.com
Subject: Re: add PROCESS_MAIN to VACUUM
In-Reply-To: <20230306211337.GA3076909@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