From cc8ed3c40ef36dc19fb30f38d445983baac40d1e Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 14 Dec 2020 21:18:36 -0600 Subject: [PATCH 3/4] Move booleans directly into VacuumParams --- src/backend/access/heap/vacuumlazy.c | 8 +-- src/backend/commands/analyze.c | 12 ++-- src/backend/commands/vacuum.c | 90 ++++++++++++++-------------- src/backend/postmaster/autovacuum.c | 14 ++--- src/include/commands/vacuum.h | 26 ++++---- 5 files changed, 73 insertions(+), 77 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 6bfed2b2da..f7995b33a8 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -456,7 +456,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, starttime = GetCurrentTimestamp(); } - if (params->options.verbose) + if (params->verbose) elevel = INFO; else elevel = DEBUG2; @@ -484,7 +484,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, xidFullScanLimit); aggressive |= MultiXactIdPrecedesOrEquals(onerel->rd_rel->relminmxid, mxactFullScanLimit); - if (params->options.disable_page_skipping) + if (params->disable_page_skipping) aggressive = true; vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats)); @@ -902,7 +902,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, * be replayed on any hot standby, where it can be disruptive. */ next_unskippable_block = 0; - if (!params->options.disable_page_skipping) + if (!params->disable_page_skipping) { while (next_unskippable_block < nblocks) { @@ -960,7 +960,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, { /* Time to advance next_unskippable_block */ next_unskippable_block++; - if (!params->options.disable_page_skipping) + if (!params->disable_page_skipping) { while (next_unskippable_block < nblocks) { diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 14c436b8c0..175309a834 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -126,7 +126,7 @@ analyze_rel(Oid relid, RangeVar *relation, BlockNumber relpages = 0; /* Select logging level */ - if (params->options.verbose) + if (params->verbose) elevel = INFO; else elevel = DEBUG2; @@ -148,10 +148,10 @@ analyze_rel(Oid relid, RangeVar *relation, * * Make sure to generate only logs for ANALYZE in this case. */ - Assert(params->options.analyze); + Assert(params->do_analyze); onerel = vacuum_open_relation(relid, relation, false, true, params->log_min_duration >= 0, - params->options.skip_locked, + params->skip_locked, ShareUpdateExclusiveLock); /* leave if relation could not be opened or locked */ @@ -240,7 +240,7 @@ analyze_rel(Oid relid, RangeVar *relation, else { /* No need for a WARNING if we already complained during VACUUM */ - if (!params->options.vacuum) + if (!params->do_vacuum) ereport(WARNING, (errmsg("skipping \"%s\" --- cannot analyze non-tables or special system tables", RelationGetRelationName(onerel)))); @@ -626,7 +626,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params, * VACUUM ANALYZE, don't overwrite the accurate count already inserted by * VACUUM. */ - if (!inh && !params->options.vacuum) + if (!inh && !params->do_vacuum) { for (ind = 0; ind < nindexes; ind++) { @@ -657,7 +657,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params, (va_cols == NIL)); /* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */ - if (!params->options.vacuum) + if (!params->do_vacuum) { for (ind = 0; ind < nindexes; ind++) { diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 2652880474..5fa88de650 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -78,8 +78,8 @@ pg_atomic_uint32 *VacuumActiveNWorkers = NULL; int VacuumCostBalanceLocal = 0; /* non-export function prototypes */ -static List *expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options); -static List *get_all_vacuum_rels(VacuumOptions *options); +static List *expand_vacuum_rel(VacuumRelation *vrel, VacuumParams *params); +static List *get_all_vacuum_rels(VacuumParams *params); static void vac_truncate_clog(TransactionId frozenXID, MultiXactId minMulti, TransactionId lastSaneFrozenXid, @@ -97,7 +97,7 @@ static VacOptTernaryValue get_vacopt_ternary_value(DefElem *def); void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) { - VacuumParams params = {.options={false} }; + VacuumParams params = {false}; ListCell *lc; /* Set default value */ @@ -114,9 +114,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) /* Parse common options for VACUUM and ANALYZE */ if (strcmp(opt->defname, "verbose") == 0) - params.options.verbose = defGetBoolean(opt); + params.verbose = defGetBoolean(opt); else if (strcmp(opt->defname, "skip_locked") == 0) - params.options.skip_locked = defGetBoolean(opt); + params.skip_locked = defGetBoolean(opt); else if (!vacstmt->is_vacuumcmd) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -125,13 +125,13 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) /* Parse options available on VACUUM */ else if (strcmp(opt->defname, "analyze") == 0) - params.options.analyze = defGetBoolean(opt); + params.do_analyze = defGetBoolean(opt); else if (strcmp(opt->defname, "freeze") == 0) - params.options.freeze = defGetBoolean(opt); + params.freeze = defGetBoolean(opt); else if (strcmp(opt->defname, "full") == 0) - params.options.full = defGetBoolean(opt); + params.full = defGetBoolean(opt); else if (strcmp(opt->defname, "disable_page_skipping") == 0) - params.options.disable_page_skipping = defGetBoolean(opt); + params.disable_page_skipping = defGetBoolean(opt); else if (strcmp(opt->defname, "index_cleanup") == 0) params.index_cleanup = get_vacopt_ternary_value(opt); else if (strcmp(opt->defname, "truncate") == 0) @@ -176,17 +176,17 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) } if (vacstmt->is_vacuumcmd) - params.options.vacuum = true; + params.do_vacuum = true; else - params.options.analyze = true; + params.do_analyze = true; /* sanity checks on options */ - Assert(params.options.vacuum || params.options.analyze); - Assert(params.options.vacuum || - !(params.options.full || params.options.freeze)); - Assert(!params.options.skiptoast); + Assert(params.do_vacuum || params.do_analyze); + Assert(params.do_vacuum || + !(params.full || params.freeze)); + Assert(!params.skiptoast); - if (params.options.full && params.nworkers > 0) + if (params.full && params.nworkers > 0) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("VACUUM FULL cannot be performed in parallel"))); @@ -194,7 +194,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) /* * Make sure analyze is specified if any column lists are present. */ - if (!params.options.analyze) + if (!params.do_analyze) { ListCell *lc; @@ -213,7 +213,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) * All freeze ages are zero if the FREEZE option is given; otherwise pass * them as -1 which means to use the default values. */ - if (params.options.freeze) + if (params.freeze) { params.freeze_min_age = 0; params.freeze_table_age = 0; @@ -269,7 +269,7 @@ vacuum(List *relations, VacuumParams *params, Assert(params != NULL); - stmttype = params->options.vacuum ? "VACUUM" : "ANALYZE"; + stmttype = params->do_vacuum ? "VACUUM" : "ANALYZE"; /* * We cannot run VACUUM inside a user transaction block; if we were inside @@ -279,7 +279,7 @@ vacuum(List *relations, VacuumParams *params, * * ANALYZE (without VACUUM) can run either way. */ - if (params->options.vacuum) + if (params->do_vacuum) { PreventInTransactionBlock(isTopLevel, stmttype); in_outer_xact = false; @@ -301,8 +301,8 @@ vacuum(List *relations, VacuumParams *params, /* * Sanity check DISABLE_PAGE_SKIPPING option. */ - if (params->options.full && - params->options.disable_page_skipping) + if (params->full && + params->disable_page_skipping) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("VACUUM option DISABLE_PAGE_SKIPPING cannot be used with FULL"))); @@ -311,7 +311,7 @@ vacuum(List *relations, VacuumParams *params, * Send info about dead objects to the statistics collector, unless we are * in autovacuum --- autovacuum.c does this for itself. */ - if (params->options.vacuum && !IsAutoVacuumWorkerProcess()) + if (params->do_vacuum && !IsAutoVacuumWorkerProcess()) pgstat_vacuum_stat(); /* @@ -352,7 +352,7 @@ vacuum(List *relations, VacuumParams *params, List *sublist; MemoryContext old_context; - sublist = expand_vacuum_rel(vrel, ¶ms->options); + sublist = expand_vacuum_rel(vrel, params); old_context = MemoryContextSwitchTo(vac_context); newrels = list_concat(newrels, sublist); MemoryContextSwitchTo(old_context); @@ -360,7 +360,7 @@ vacuum(List *relations, VacuumParams *params, relations = newrels; } else - relations = get_all_vacuum_rels(¶ms->options); + relations = get_all_vacuum_rels(params); /* * Decide whether we need to start/commit our own transactions. @@ -376,11 +376,11 @@ vacuum(List *relations, VacuumParams *params, * transaction block, and also in an autovacuum worker, use own * transactions so we can release locks sooner. */ - if (params->options.vacuum) + if (params->do_vacuum) use_own_xacts = true; else { - Assert(params->options.analyze); + Assert(params->do_analyze); if (IsAutoVacuumWorkerProcess()) use_own_xacts = true; else if (in_outer_xact) @@ -433,13 +433,13 @@ vacuum(List *relations, VacuumParams *params, { VacuumRelation *vrel = lfirst_node(VacuumRelation, cur); - if (params->options.vacuum) + if (params->do_vacuum) { if (!vacuum_rel(vrel->oid, vrel->relation, params)) continue; } - if (params->options.analyze) + if (params->do_analyze) { /* * If using separate xacts, start one for analyze. Otherwise, @@ -493,7 +493,7 @@ vacuum(List *relations, VacuumParams *params, StartTransactionCommand(); } - if (params->options.vacuum && !IsAutoVacuumWorkerProcess()) + if (params->do_vacuum && !IsAutoVacuumWorkerProcess()) { /* * Update pg_database.datfrozenxid, and truncate pg_xact if possible. @@ -707,7 +707,7 @@ vacuum_open_relation(Oid relid, RangeVar *relation, bool do_vacuum, * are made in vac_context. */ static List * -expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options) +expand_vacuum_rel(VacuumRelation *vrel, VacuumParams *params) { List *vacrels = NIL; MemoryContext oldcontext; @@ -739,7 +739,7 @@ expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options) * below, as well as find_all_inheritors's expectation that the caller * holds some lock on the starting relation. */ - rvr_opts = (options->skip_locked) ? RVR_SKIP_LOCKED : 0; + rvr_opts = (params->skip_locked) ? RVR_SKIP_LOCKED : 0; relid = RangeVarGetRelidExtended(vrel->relation, AccessShareLock, rvr_opts, @@ -751,7 +751,7 @@ expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options) */ if (!OidIsValid(relid)) { - if (options->vacuum) + if (params->do_vacuum) ereport(WARNING, (errcode(ERRCODE_LOCK_NOT_AVAILABLE), errmsg("skipping vacuum of \"%s\" --- lock not available", @@ -777,8 +777,8 @@ expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options) * Make a returnable VacuumRelation for this rel if user is a proper * owner. */ - if (vacuum_is_relation_owner(relid, classForm, options->vacuum, - options->analyze)) + if (vacuum_is_relation_owner(relid, classForm, params->do_vacuum, + params->do_analyze)) { oldcontext = MemoryContextSwitchTo(vac_context); vacrels = lappend(vacrels, makeVacuumRelation(vrel->relation, @@ -847,7 +847,7 @@ expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options) * the current database. The list is built in vac_context. */ static List * -get_all_vacuum_rels(VacuumOptions *options) +get_all_vacuum_rels(VacuumParams *params) { List *vacrels = NIL; Relation pgclass; @@ -865,8 +865,8 @@ get_all_vacuum_rels(VacuumOptions *options) Oid relid = classForm->oid; /* check permissions of relation */ - if (!vacuum_is_relation_owner(relid, classForm, options->vacuum, - options->analyze)) + if (!vacuum_is_relation_owner(relid, classForm, params->do_vacuum, + params->do_analyze)) continue; /* @@ -1721,7 +1721,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) /* Begin a transaction for vacuuming this relation */ StartTransactionCommand(); - if (!params->options.full) + if (!params->full) { /* * In lazy vacuum, we can set the PROC_IN_VACUUM flag, which lets @@ -1771,13 +1771,13 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) * vacuum, but just ShareUpdateExclusiveLock for concurrent vacuum. Either * way, we can be sure that no other backend is vacuuming the same table. */ - lmode = (params->options.full) ? + lmode = (params->full) ? AccessExclusiveLock : ShareUpdateExclusiveLock; /* open the relation and get the appropriate lock on it */ - Assert(params->options.vacuum); + Assert(params->do_vacuum); onerel = vacuum_open_relation(relid, relation, true, false, - params->log_min_duration >= 0, params->options.skip_locked, lmode); + params->log_min_duration >= 0, params->skip_locked, lmode); /* leave if relation could not be opened or locked */ if (!onerel) @@ -1888,7 +1888,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) * us to process it. In VACUUM FULL, though, the toast table is * automatically rebuilt by cluster_rel so we shouldn't recurse to it. */ - if (!params->options.skiptoast && !params->options.full) + if (!params->skiptoast && !params->full) toast_relid = onerel->rd_rel->reltoastrelid; else toast_relid = InvalidOid; @@ -1907,10 +1907,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) /* * Do the actual work --- either FULL or "lazy" vacuum */ - if (params->options.full) + if (params->full) { ClusterOptions cluster_options = { - .verbose = params->options.verbose, + .verbose = params->verbose, /* Other members initialized to false/0/NULL */ }; diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 8b151bd994..1f8d0d1b88 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2504,7 +2504,7 @@ do_autovacuum(void) * next table in our list. */ HOLD_INTERRUPTS(); - if (tab->at_params.options.vacuum) + if (tab->at_params.do_vacuum) errcontext("automatic vacuum of table \"%s.%s.%s\"", tab->at_datname, tab->at_nspname, tab->at_relname); else @@ -2918,10 +2918,10 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab = palloc(sizeof(autovac_table)); tab->at_relid = relid; tab->at_sharedrel = classForm->relisshared; - tab->at_params.options.skiptoast = true; - tab->at_params.options.vacuum = dovacuum; - tab->at_params.options.analyze = doanalyze; - tab->at_params.options.skip_locked = !wraparound; + tab->at_params.skiptoast = true; + tab->at_params.do_vacuum = dovacuum; + tab->at_params.do_analyze = doanalyze; + tab->at_params.skip_locked = !wraparound; tab->at_params.index_cleanup = VACOPT_TERNARY_DEFAULT; tab->at_params.truncate = VACOPT_TERNARY_DEFAULT; /* As of now, we don't support parallel vacuum for autovacuum */ @@ -3249,10 +3249,10 @@ autovac_report_activity(autovac_table *tab) int len; /* Report the command and possible options */ - if (tab->at_params.options.vacuum) + if (tab->at_params.do_vacuum) snprintf(activity, MAX_AUTOVAC_ACTIV_LEN, "autovacuum: VACUUM%s", - tab->at_params.options.analyze ? " ANALYZE" : ""); + tab->at_params.do_analyze ? " ANALYZE" : ""); else snprintf(activity, MAX_AUTOVAC_ACTIV_LEN, "autovacuum: ANALYZE"); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 740fa7aaa1..e51f2adcd5 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -174,18 +174,6 @@ typedef struct VacAttrStats int rowstride; } VacAttrStats; -typedef struct VacuumOptions -{ - bool vacuum; /* do VACUUM */ - bool analyze; /* do ANALYZE */ - bool verbose; /* print progress info */ - bool freeze; /* FREEZE option */ - bool full; /* FULL (non-concurrent) vacuum */ - bool skip_locked; /* skip if cannot get lock */ - bool skiptoast; /* don't process the TOAST table, if any */ - bool disable_page_skipping; /* don't skip any pages */ -} VacuumOptions; - /* * A ternary value used by vacuum parameters. * @@ -202,12 +190,20 @@ typedef enum VacOptTernaryValue /* * Parameters customizing behavior of VACUUM and ANALYZE. * - * Note that at least one of VACUUM and ANALYZE must be set - * in options. + * Note that at least one of VACUUM and ANALYZE must be set. */ typedef struct VacuumParams { - VacuumOptions options; /* struct of boolean options */ + /* boolean options */ + bool do_vacuum; /* do VACUUM */ + bool do_analyze; /* do ANALYZE */ + bool verbose; /* print progress info */ + bool freeze; /* FREEZE option */ + bool full; /* FULL (non-concurrent) vacuum */ + bool skip_locked; /* skip if cannot get lock */ + bool skiptoast; /* don't process the TOAST table, if any */ + bool disable_page_skipping; /* don't skip any pages */ + int freeze_min_age; /* min freeze age, -1 to use default */ int freeze_table_age; /* age at which to scan whole table */ int multixact_freeze_min_age; /* min multixact freeze age, -1 to -- 2.17.0