From 045a6e94dbc1fa6e53cf88ee39b56abbc18a0d0f Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 5 Apr 2023 20:05:25 -0400 Subject: [PATCH v14 1/3] Push vacuum() setup code up into ExecVacuum() Much of the sanity checking and setup done in vacuum() code was specific to explicit VACUUM and ANALYZE. Move that code up into ExecVacuum(). While we are at it, allocate VACUUM/ANALYZE's BufferAccessStrategy in ExecVacuum() and pass it into vacuum() instead of expecting vacuum() to make it. This mimics autovacuum's behavior. To do this, create the relevant memory context in ExecVacuum() and pass it in as a new parameter to vacuum(). --- src/backend/commands/vacuum.c | 186 ++++++++++++++-------------- src/backend/postmaster/autovacuum.c | 17 ++- src/include/commands/vacuum.h | 3 +- 3 files changed, 110 insertions(+), 96 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 2c31745fbc..bb2787411c 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -105,6 +105,7 @@ void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) { VacuumParams params; + BufferAccessStrategy bstrategy = NULL; bool verbose = false; bool skip_locked = false; bool analyze = false; @@ -115,8 +116,12 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) bool process_toast = true; bool skip_database_stats = false; bool only_database_stats = false; + MemoryContext vac_context; ListCell *lc; + const char *stmttype; + volatile bool in_outer_xact; + /* index_cleanup and truncate values unspecified for now */ params.index_cleanup = VACOPTVALUE_UNSPECIFIED; params.truncate = VACOPTVALUE_UNSPECIFIED; @@ -254,6 +259,42 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) } } + + /* + * Sanity check DISABLE_PAGE_SKIPPING option. + */ + if ((params.options & VACOPT_FULL) != 0 && + (params.options & VACOPT_DISABLE_PAGE_SKIPPING) != 0) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("VACUUM option DISABLE_PAGE_SKIPPING cannot be used with FULL"))); + + /* sanity check for PROCESS_TOAST */ + if ((params.options & VACOPT_FULL) != 0 && + (params.options & VACOPT_PROCESS_TOAST) == 0) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("PROCESS_TOAST required with VACUUM FULL"))); + + /* sanity check for ONLY_DATABASE_STATS */ + if (params.options & VACOPT_ONLY_DATABASE_STATS) + { + Assert(params.options & VACOPT_VACUUM); + if (vacstmt->rels != NIL) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("ONLY_DATABASE_STATS cannot be specified with a list of tables"))); + /* don't require people to turn off PROCESS_TOAST/MAIN explicitly */ + if (params.options & ~(VACOPT_VACUUM | + VACOPT_VERBOSE | + VACOPT_PROCESS_MAIN | + VACOPT_PROCESS_TOAST | + VACOPT_ONLY_DATABASE_STATS)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("ONLY_DATABASE_STATS cannot be specified with other VACUUM options"))); + } + /* * All freeze ages are zero if the FREEZE option is given; otherwise pass * them as -1 which means to use the default values. @@ -279,8 +320,57 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) /* user-invoked vacuum uses VACOPT_VERBOSE instead of log_min_duration */ params.log_min_duration = -1; + stmttype = (params.options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE"; + + /* + * We cannot run VACUUM inside a user transaction block; if we were inside + * a transaction, then our commit- and start-transaction-command calls + * would not have the intended effect! There are numerous other subtle + * dependencies on this, too. + * + * ANALYZE (without VACUUM) can run either way. + */ + if (params.options & VACOPT_VACUUM) + { + PreventInTransactionBlock(isTopLevel, stmttype); + in_outer_xact = false; + } + else + in_outer_xact = IsInTransactionBlock(isTopLevel); + + /* + * Create special memory context for cross-transaction storage. + * + * Since it is a child of PortalContext, it will go away eventually even + * if we suffer an error; there's no need for special abort cleanup logic. + */ + vac_context = AllocSetContextCreate(PortalContext, + "Vacuum", + ALLOCSET_DEFAULT_SIZES); + + /* + * Make a buffer strategy object in the cross-transaction memory context. + * We needn't bother making this for VACUUM (FULL) or VACUUM + * (ONLY_DATABASE_STATS) as they'll not make use of it. VACUUM (FULL, + * ANALYZE) is possible, so we'd better ensure that we make a strategy when + * we see ANALYZE. + */ + if ((params.options & (VACOPT_ONLY_DATABASE_STATS | + VACOPT_FULL)) == 0 || + (params.options & VACOPT_ANALYZE) != 0) + { + + MemoryContext old_context = MemoryContextSwitchTo(vac_context); + + bstrategy = GetAccessStrategy(BAS_VACUUM); + MemoryContextSwitchTo(old_context); + } + /* Now go through the common routine */ - vacuum(vacstmt->rels, ¶ms, NULL, isTopLevel); + vacuum(vacstmt->rels, ¶ms, bstrategy, vac_context, isTopLevel, + in_outer_xact); + + MemoryContextDelete(vac_context); } /* @@ -304,35 +394,18 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) */ void vacuum(List *relations, VacuumParams *params, - BufferAccessStrategy bstrategy, bool isTopLevel) + BufferAccessStrategy bstrategy, MemoryContext vac_context, + bool isTopLevel, bool in_outer_xact) { - static bool in_vacuum = false; - MemoryContext vac_context; const char *stmttype; - volatile bool in_outer_xact, - use_own_xacts; + static bool in_vacuum = false; + volatile bool use_own_xacts; Assert(params != NULL); stmttype = (params->options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE"; - /* - * We cannot run VACUUM inside a user transaction block; if we were inside - * a transaction, then our commit- and start-transaction-command calls - * would not have the intended effect! There are numerous other subtle - * dependencies on this, too. - * - * ANALYZE (without VACUUM) can run either way. - */ - if (params->options & VACOPT_VACUUM) - { - PreventInTransactionBlock(isTopLevel, stmttype); - in_outer_xact = false; - } - else - in_outer_xact = IsInTransactionBlock(isTopLevel); - /* * Check for and disallow recursive calls. This could happen when VACUUM * FULL or ANALYZE calls a hostile index expression that itself calls @@ -344,69 +417,6 @@ vacuum(List *relations, VacuumParams *params, errmsg("%s cannot be executed from VACUUM or ANALYZE", stmttype))); - /* - * Sanity check DISABLE_PAGE_SKIPPING option. - */ - if ((params->options & VACOPT_FULL) != 0 && - (params->options & VACOPT_DISABLE_PAGE_SKIPPING) != 0) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("VACUUM option DISABLE_PAGE_SKIPPING cannot be used with FULL"))); - - /* sanity check for PROCESS_TOAST */ - if ((params->options & VACOPT_FULL) != 0 && - (params->options & VACOPT_PROCESS_TOAST) == 0) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("PROCESS_TOAST required with VACUUM FULL"))); - - /* sanity check for ONLY_DATABASE_STATS */ - if (params->options & VACOPT_ONLY_DATABASE_STATS) - { - Assert(params->options & VACOPT_VACUUM); - if (relations != NIL) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("ONLY_DATABASE_STATS cannot be specified with a list of tables"))); - /* don't require people to turn off PROCESS_TOAST/MAIN explicitly */ - if (params->options & ~(VACOPT_VACUUM | - VACOPT_VERBOSE | - VACOPT_PROCESS_MAIN | - VACOPT_PROCESS_TOAST | - VACOPT_ONLY_DATABASE_STATS)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("ONLY_DATABASE_STATS cannot be specified with other VACUUM options"))); - } - - /* - * Create special memory context for cross-transaction storage. - * - * Since it is a child of PortalContext, it will go away eventually even - * if we suffer an error; there's no need for special abort cleanup logic. - */ - vac_context = AllocSetContextCreate(PortalContext, - "Vacuum", - ALLOCSET_DEFAULT_SIZES); - - /* - * If caller didn't give us a buffer strategy object, make one in the - * cross-transaction memory context. We needn't bother making this for - * VACUUM (FULL) or VACUUM (ONLY_DATABASE_STATS) as they'll not make use - * of it. VACUUM (FULL, ANALYZE) is possible, so we'd better ensure that - * we make a strategy when we see ANALYZE. - */ - if (bstrategy == NULL && - ((params->options & (VACOPT_ONLY_DATABASE_STATS | - VACOPT_FULL)) == 0 || - (params->options & VACOPT_ANALYZE) != 0)) - { - MemoryContext old_context = MemoryContextSwitchTo(vac_context); - - bstrategy = GetAccessStrategy(BAS_VACUUM); - MemoryContextSwitchTo(old_context); - } - /* * Build list of relation(s) to process, putting any new data in * vac_context for safekeeping. @@ -578,12 +588,6 @@ vacuum(List *relations, VacuumParams *params, vac_update_datfrozenxid(); } - /* - * Clean up working storage --- note we must do this after - * StartTransactionCommand, else we might be trying to delete the active - * context! - */ - MemoryContextDelete(vac_context); } /* diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index e9ba0dc17c..d2b6f41ead 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -338,7 +338,8 @@ static void relation_needs_vacanalyze(Oid relid, AutoVacOpts *relopts, bool *dovacuum, bool *doanalyze, bool *wraparound); static void autovacuum_do_vac_analyze(autovac_table *tab, - BufferAccessStrategy bstrategy); + BufferAccessStrategy bstrategy, + MemoryContext portal_context); static AutoVacOpts *extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc); static void perform_work_item(AutoVacuumWorkItem *workitem); @@ -2469,7 +2470,7 @@ do_autovacuum(void) MemoryContextSwitchTo(PortalContext); /* have at it */ - autovacuum_do_vac_analyze(tab, bstrategy); + autovacuum_do_vac_analyze(tab, bstrategy, PortalContext); /* * Clear a possible query-cancel signal, to avoid a late reaction @@ -3142,11 +3143,13 @@ relation_needs_vacanalyze(Oid relid, * Vacuum and/or analyze the specified table */ static void -autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy) +autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy, + MemoryContext portal_context) { RangeVar *rangevar; VacuumRelation *rel; List *rel_list; + MemoryContext vac_context; /* Let pgstat know what we're doing */ autovac_report_activity(tab); @@ -3156,7 +3159,13 @@ autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy) rel = makeVacuumRelation(rangevar, tab->at_relid, NIL); rel_list = list_make1(rel); - vacuum(rel_list, &tab->at_params, bstrategy, true); + vac_context = AllocSetContextCreate(portal_context, + "Vacuum", + ALLOCSET_DEFAULT_SIZES); + + vacuum(rel_list, &tab->at_params, bstrategy, vac_context, true, false); + + MemoryContextDelete(vac_context); } /* diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index bdfd96cfec..410eb9dece 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -310,7 +310,8 @@ extern PGDLLIMPORT int VacuumCostBalanceLocal; /* in commands/vacuum.c */ extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel); extern void vacuum(List *relations, VacuumParams *params, - BufferAccessStrategy bstrategy, bool isTopLevel); + BufferAccessStrategy bstrategy, MemoryContext vac_context, + bool isTopLevel, bool in_outer_xact); extern void vac_open_indexes(Relation relation, LOCKMODE lockmode, int *nindexes, Relation **Irel); extern void vac_close_indexes(int nindexes, Relation *Irel, LOCKMODE lockmode); -- 2.37.2