From f46fa37268b2873b4f98118ce8561fd87ef7297a Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 5 Apr 2023 17:20:41 -0400 Subject: [PATCH v1 1/2] wip vacuum refactor --- src/backend/commands/vacuum.c | 243 ++++++++++++++-------------- src/backend/postmaster/autovacuum.c | 17 +- src/include/commands/vacuum.h | 3 +- 3 files changed, 139 insertions(+), 124 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index da85330ef4..c6eb1f2fad 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,13 @@ 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, + use_own_xacts; + /* index_cleanup and truncate values unspecified for now */ params.index_cleanup = VACOPTVALUE_UNSPECIFIED; params.truncate = VACOPTVALUE_UNSPECIFIED; @@ -254,6 +260,61 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) } } + + 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); + + /* + * 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 +340,67 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) /* user-invoked vacuum uses VACOPT_VERBOSE instead of log_min_duration */ params.log_min_duration = -1; + /* + * 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 a Buffer Access Strategy for VACUUM (FULL) + * (unless ANALYZE is also specified) or VACUUM (ONLY_DATABASE_STATS) as + * they'll not make use of it. + */ + if ((params.options & VACOPT_ONLY_DATABASE_STATS) == 0 || + (params.options & VACOPT_FULL && (params.options & VACOPT_ANALYZE) == 0)) + { + MemoryContext old_context = MemoryContextSwitchTo(vac_context); + + bstrategy = GetAccessStrategy(BAS_VACUUM); + MemoryContextSwitchTo(old_context); + } + + /* + * 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 + { + Assert(params.options & VACOPT_ANALYZE); + if (in_outer_xact) + use_own_xacts = false; + else if (list_length(vacstmt->rels) > 1) + use_own_xacts = true; + else + use_own_xacts = false; + } + /* Now go through the common routine */ - vacuum(vacstmt->rels, ¶ms, NULL, isTopLevel); + vacuum(vacstmt->rels, ¶ms, bstrategy, vac_context, isTopLevel, + use_own_xacts, 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 +424,16 @@ 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 use_own_xacts, 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; 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 +445,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. - */ - if (bstrategy == NULL && - (params->options & (VACOPT_ONLY_DATABASE_STATS | - VACOPT_FULL)) == 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. @@ -435,35 +475,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 - { - Assert(params->options & VACOPT_ANALYZE); - if (IsAutoVacuumWorkerProcess()) - use_own_xacts = true; - else if (in_outer_xact) - use_own_xacts = false; - else if (list_length(relations) > 1) - use_own_xacts = true; - else - use_own_xacts = false; - } - /* * vacuum_rel expects to be entered with no transaction active; it will * start and commit its own transaction. But we are called by an SQL @@ -576,12 +587,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..8016a2dd47 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, true, false); + + MemoryContextDelete(vac_context); } /* diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index bdfd96cfec..a4c928a877 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 use_own_xacts, 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