From 98aa041bd421fb1a9817bf72480f6e910a128b71 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 5 Apr 2023 20:05:25 -0400 Subject: [PATCH v12 2/4] 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 | 197 +++++++++++++--------------- src/backend/postmaster/autovacuum.c | 17 ++- src/include/commands/vacuum.h | 3 +- 3 files changed, 109 insertions(+), 108 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 438e94410e..9b3f5c03ca 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,41 @@ 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 +319,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); + + /* + * We needn't bother making this for VACUUM (ONLY_DATABASE_STATS) or VACUUM + * (FULL) without the ANALYZE option, as they'll not make use of it. + */ + if ((params.options & VACOPT_ONLY_DATABASE_STATS) == 0 && + ((params.options & VACOPT_FULL) == 0 || (params.options & VACOPT_ANALYZE))) + { + 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); + + /* + * Clean up working storage --- note we must do this after + * StartTransactionCommand, else we might be trying to delete the active + * context! + */ + MemoryContextDelete(vac_context); } /* @@ -304,35 +393,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,67 +416,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 (ONLY_DATABASE_STATS) or VACUUM (FULL) without the ANALYZE - * option, as they'll not make use of it. - */ - if (bstrategy == NULL && - ((params->options & VACOPT_ONLY_DATABASE_STATS) == 0 && - ((params->options & VACOPT_FULL) == 0 || (params->options & VACOPT_ANALYZE)))) - { - - 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 @@ -436,20 +447,6 @@ vacuum(List *relations, VacuumParams *params, else relations = get_all_vacuum_rels(vac_context, params->options); - /* - * Decide whether we need to start/commit our own transactions. - * - * For VACUUM (with or without ANALYZE): always do so, so that we can - * release locks as soon as possible. (We could possibly use the outer - * transaction for a one-table VACUUM, but handling TOAST tables would be - * problematic.) - * - * For ANALYZE (no VACUUM): if inside a transaction block, we cannot - * start/commit our own transactions. Also, there's no need to do so if - * only processing one relation. For multiple relations when not within a - * transaction block, and also in an autovacuum worker, use own - * transactions so we can release locks sooner. - */ if (params->options & VACOPT_VACUUM) use_own_xacts = true; else @@ -577,12 +574,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 585d28148c..76fac1fea7 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); @@ -2470,7 +2471,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 @@ -3143,11 +3144,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); @@ -3157,7 +3160,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