From a46a7011b27188af526047a111969f257aaf4db8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 6 Jan 2023 14:17:25 -0500 Subject: [PATCH] Add options to control whether VACUUM runs vac_update_datfrozenxid. VACUUM normally ends by running vac_update_datfrozenxid(), which requires a scan of pg_class. Therefore, if one attempts to vacuum a database one table at a time --- as vacuumdb has done since v12 --- we will spend O(N^2) time in vac_update_datfrozenxid(). That causes serious performance problems in databases with tens of thousands of tables, and indeed the effect is measurable with only a few hundred. To add insult to injury, only one process can run vac_update_datfrozenxid at the same time per DB, so this behavior largely defeats vacuumdb's -j option. Hence, invent options SKIP_DATABASE_STATS and ONLY_DATABASE_STATS to allow applications to postpone vac_update_datfrozenxid() until the end of a series of VACUUM requests, and teach vacuumdb to use them. Per bug #17717 from Gunnar L. Sadly, this answer doesn't seem like something we'd consider back-patching, so the performance problem will remain in v12-v15. Tom Lane and Nathan Bossart Discussion: https://postgr.es/m/17717-6c50eb1c7d23a886@postgresql.org --- doc/src/sgml/ref/vacuum.sgml | 37 ++++++++++++++++++++++++ src/backend/commands/vacuum.c | 42 ++++++++++++++++++++++++---- src/backend/postmaster/autovacuum.c | 9 ++++-- src/bin/psql/tab-complete.c | 5 ++-- src/bin/scripts/t/100_vacuumdb.pl | 24 ++++++++-------- src/bin/scripts/vacuumdb.c | 33 ++++++++++++++++++++++ src/fe_utils/parallel_slot.c | 3 ++ src/include/commands/vacuum.h | 2 ++ src/test/regress/expected/vacuum.out | 6 ++++ src/test/regress/sql/vacuum.sql | 7 +++++ 10 files changed, 147 insertions(+), 21 deletions(-) diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index e14ead8826..8fa8421847 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -36,6 +36,8 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ boolean ] TRUNCATE [ boolean ] PARALLEL integer + SKIP_DATABASE_STATS [ boolean ] + ONLY_DATABASE_STATS [ boolean ] and table_and_columns is: @@ -295,6 +297,41 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ table_and_columns + list must be empty, and no other option may be enabled + except VERBOSE. + + + + boolean diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 158b1b497b..c4ed7efce3 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -114,6 +114,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) bool full = false; bool disable_page_skipping = false; bool process_toast = true; + bool skip_database_stats = false; + bool only_database_stats = false; ListCell *lc; /* index_cleanup and truncate values unspecified for now */ @@ -200,6 +202,10 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) params.nworkers = nworkers; } } + else if (strcmp(opt->defname, "skip_database_stats") == 0) + skip_database_stats = defGetBoolean(opt); + else if (strcmp(opt->defname, "only_database_stats") == 0) + only_database_stats = defGetBoolean(opt); else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -216,7 +222,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) (freeze ? VACOPT_FREEZE : 0) | (full ? VACOPT_FULL : 0) | (disable_page_skipping ? VACOPT_DISABLE_PAGE_SKIPPING : 0) | - (process_toast ? VACOPT_PROCESS_TOAST : 0); + (process_toast ? VACOPT_PROCESS_TOAST : 0) | + (skip_database_stats ? VACOPT_SKIP_DATABASE_STATS : 0) | + (only_database_stats ? VACOPT_ONLY_DATABASE_STATS : 0); /* sanity checks on options */ Assert(params.options & (VACOPT_VACUUM | VACOPT_ANALYZE)); @@ -349,6 +357,24 @@ vacuum(List *relations, VacuumParams *params, (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 explicitly */ + if (params->options & ~(VACOPT_VACUUM | + VACOPT_VERBOSE | + 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. * @@ -376,7 +402,12 @@ vacuum(List *relations, VacuumParams *params, * Build list of relation(s) to process, putting any new data in * vac_context for safekeeping. */ - if (relations != NIL) + if (params->options & VACOPT_ONLY_DATABASE_STATS) + { + /* We don't process any tables in this case */ + Assert(relations == NIL); + } + else if (relations != NIL) { List *newrels = NIL; ListCell *lc; @@ -528,11 +559,11 @@ vacuum(List *relations, VacuumParams *params, StartTransactionCommand(); } - if ((params->options & VACOPT_VACUUM) && !IsAutoVacuumWorkerProcess()) + if ((params->options & VACOPT_VACUUM) && + !(params->options & VACOPT_SKIP_DATABASE_STATS)) { /* * Update pg_database.datfrozenxid, and truncate pg_xact if possible. - * (autovacuum.c does this for itself.) */ vac_update_datfrozenxid(); } @@ -560,13 +591,14 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple, Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0); - /* + /*---------- * A role has privileges to vacuum or analyze the relation if any of the * following are true: * - the role is a superuser * - the role owns the relation * - the role owns the current database and the relation is not shared * - the role has been granted the MAINTAIN privilege on the relation + *---------- */ if (object_ownercheck(RelationRelationId, relid, GetUserId()) || (object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) && !reltuple->relisshared) || diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index e40bd39b3f..f5ea381c53 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2854,8 +2854,13 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab->at_relid = relid; tab->at_sharedrel = classForm->relisshared; - /* Note that this skips toast relations */ - tab->at_params.options = (dovacuum ? VACOPT_VACUUM : 0) | + /* + * Select VACUUM options. Note we don't say VACOPT_PROCESS_TOAST, so + * that vacuum() skips toast relations. Also note we tell vacuum() to + * skip vac_update_datfrozenxid(); we'll do that separately. + */ + tab->at_params.options = + (dovacuum ? (VACOPT_VACUUM | VACOPT_SKIP_DATABASE_STATS) : 0) | (doanalyze ? VACOPT_ANALYZE : 0) | (!wraparound ? VACOPT_SKIP_LOCKED : 0); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 3942bea72d..23750ea5fb 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -4598,8 +4598,9 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("FULL", "FREEZE", "ANALYZE", "VERBOSE", "DISABLE_PAGE_SKIPPING", "SKIP_LOCKED", "INDEX_CLEANUP", "PROCESS_TOAST", - "TRUNCATE", "PARALLEL"); - else if (TailMatches("FULL|FREEZE|ANALYZE|VERBOSE|DISABLE_PAGE_SKIPPING|SKIP_LOCKED|PROCESS_TOAST|TRUNCATE")) + "TRUNCATE", "PARALLEL", "SKIP_DATABASE_STATS", + "ONLY_DATABASE_STATS"); + else if (TailMatches("FULL|FREEZE|ANALYZE|VERBOSE|DISABLE_PAGE_SKIPPING|SKIP_LOCKED|PROCESS_TOAST|TRUNCATE|SKIP_DATABASE_STATS|ONLY_DATABASE_STATS")) COMPLETE_WITH("ON", "OFF"); else if (TailMatches("INDEX_CLEANUP")) COMPLETE_WITH("AUTO", "ON", "OFF"); diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl index cd356b11c5..3cfbaaec0d 100644 --- a/src/bin/scripts/t/100_vacuumdb.pl +++ b/src/bin/scripts/t/100_vacuumdb.pl @@ -22,15 +22,15 @@ $node->issues_sql_like( 'SQL VACUUM run'); $node->issues_sql_like( [ 'vacuumdb', '-f', 'postgres' ], - qr/statement: VACUUM \(FULL\).*;/, + qr/statement: VACUUM \(SKIP_DATABASE_STATS, FULL\).*;/, 'vacuumdb -f'); $node->issues_sql_like( [ 'vacuumdb', '-F', 'postgres' ], - qr/statement: VACUUM \(FREEZE\).*;/, + qr/statement: VACUUM \(SKIP_DATABASE_STATS, FREEZE\).*;/, 'vacuumdb -F'); $node->issues_sql_like( [ 'vacuumdb', '-zj2', 'postgres' ], - qr/statement: VACUUM \(ANALYZE\).*;/, + qr/statement: VACUUM \(SKIP_DATABASE_STATS, ANALYZE\).*;/, 'vacuumdb -zj2'); $node->issues_sql_like( [ 'vacuumdb', '-Z', 'postgres' ], @@ -38,11 +38,11 @@ $node->issues_sql_like( 'vacuumdb -Z'); $node->issues_sql_like( [ 'vacuumdb', '--disable-page-skipping', 'postgres' ], - qr/statement: VACUUM \(DISABLE_PAGE_SKIPPING\).*;/, + qr/statement: VACUUM \(DISABLE_PAGE_SKIPPING, SKIP_DATABASE_STATS\).*;/, 'vacuumdb --disable-page-skipping'); $node->issues_sql_like( [ 'vacuumdb', '--skip-locked', 'postgres' ], - qr/statement: VACUUM \(SKIP_LOCKED\).*;/, + qr/statement: VACUUM \(SKIP_DATABASE_STATS, SKIP_LOCKED\).*;/, 'vacuumdb --skip-locked'); $node->issues_sql_like( [ 'vacuumdb', '--skip-locked', '--analyze-only', 'postgres' ], @@ -53,32 +53,32 @@ $node->command_fails( '--analyze-only and --disable-page-skipping specified together'); $node->issues_sql_like( [ 'vacuumdb', '--no-index-cleanup', 'postgres' ], - qr/statement: VACUUM \(INDEX_CLEANUP FALSE\).*;/, + qr/statement: VACUUM \(INDEX_CLEANUP FALSE, SKIP_DATABASE_STATS\).*;/, 'vacuumdb --no-index-cleanup'); $node->command_fails( [ 'vacuumdb', '--analyze-only', '--no-index-cleanup', 'postgres' ], '--analyze-only and --no-index-cleanup specified together'); $node->issues_sql_like( [ 'vacuumdb', '--no-truncate', 'postgres' ], - qr/statement: VACUUM \(TRUNCATE FALSE\).*;/, + qr/statement: VACUUM \(TRUNCATE FALSE, SKIP_DATABASE_STATS\).*;/, 'vacuumdb --no-truncate'); $node->command_fails( [ 'vacuumdb', '--analyze-only', '--no-truncate', 'postgres' ], '--analyze-only and --no-truncate specified together'); $node->issues_sql_like( [ 'vacuumdb', '--no-process-toast', 'postgres' ], - qr/statement: VACUUM \(PROCESS_TOAST FALSE\).*;/, + qr/statement: VACUUM \(PROCESS_TOAST FALSE, SKIP_DATABASE_STATS\).*;/, 'vacuumdb --no-process-toast'); $node->command_fails( [ 'vacuumdb', '--analyze-only', '--no-process-toast', 'postgres' ], '--analyze-only and --no-process-toast specified together'); $node->issues_sql_like( [ 'vacuumdb', '-P', 2, 'postgres' ], - qr/statement: VACUUM \(PARALLEL 2\).*;/, + qr/statement: VACUUM \(SKIP_DATABASE_STATS, PARALLEL 2\).*;/, 'vacuumdb -P 2'); $node->issues_sql_like( [ 'vacuumdb', '-P', 0, 'postgres' ], - qr/statement: VACUUM \(PARALLEL 0\).*;/, + qr/statement: VACUUM \(SKIP_DATABASE_STATS, PARALLEL 0\).*;/, 'vacuumdb -P 0'); $node->command_ok([qw(vacuumdb -Z --table=pg_am dbname=template1)], 'vacuumdb with connection string'); @@ -119,7 +119,7 @@ $node->command_fails([ 'vacuumdb', '-P', -1, 'postgres' ], 'negative parallel degree'); $node->issues_sql_like( [ 'vacuumdb', '--analyze', '--table', 'vactable(a, b)', 'postgres' ], - qr/statement: VACUUM \(ANALYZE\) public.vactable\(a, b\);/, + qr/statement: VACUUM \(SKIP_DATABASE_STATS, ANALYZE\) public.vactable\(a, b\);/, 'vacuumdb --analyze with complete column list'); $node->issues_sql_like( [ 'vacuumdb', '--analyze-only', '--table', 'vactable(b)', 'postgres' ], @@ -150,7 +150,7 @@ $node->issues_sql_like( 'vacuumdb --table --min-xid-age'); $node->issues_sql_like( [ 'vacuumdb', '--schema', '"Foo"', 'postgres' ], - qr/VACUUM "Foo".bar/, + qr/VACUUM \(SKIP_DATABASE_STATS\) "Foo".bar/, 'vacuumdb --schema'); $node->issues_sql_like( [ 'vacuumdb', '--exclude-schema', '"Foo"', 'postgres' ], diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c index c66386d6d3..58b894216b 100644 --- a/src/bin/scripts/vacuumdb.c +++ b/src/bin/scripts/vacuumdb.c @@ -44,6 +44,7 @@ typedef struct vacuumingOptions bool force_index_cleanup; bool do_truncate; bool process_toast; + bool skip_database_stats; } vacuumingOptions; /* object filter options */ @@ -533,6 +534,9 @@ vacuum_one_database(ConnParams *cparams, pg_fatal("cannot use the \"%s\" option on server versions older than PostgreSQL %s", "--parallel", "13"); + /* skip_database_stats is used automatically if server supports it */ + vacopts->skip_database_stats = (PQserverVersion(conn) >= 160000); + if (!quiet) { if (stage != ANALYZE_NO_STAGE) @@ -790,7 +794,29 @@ vacuum_one_database(ConnParams *cparams, } while (cell != NULL); if (!ParallelSlotsWaitCompletion(sa)) + { failed = true; + goto finish; + } + + /* If we used SKIP_DATABASE_STATS, mop up with ONLY_DATABASE_STATS */ + if (vacopts->skip_database_stats && stage == ANALYZE_NO_STAGE) + { + const char *cmd = "VACUUM (ONLY_DATABASE_STATS);"; + ParallelSlot *free_slot = ParallelSlotsGetIdle(sa, NULL); + + if (!free_slot) + { + failed = true; + goto finish; + } + + ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); + run_vacuum_command(free_slot->connection, cmd, echo, NULL); + + if (!ParallelSlotsWaitCompletion(sa)) + failed = true; + } finish: ParallelSlotsTerminate(sa); @@ -957,6 +983,13 @@ prepare_vacuum_command(PQExpBuffer sql, int serverVersion, appendPQExpBuffer(sql, "%sPROCESS_TOAST FALSE", sep); sep = comma; } + if (vacopts->skip_database_stats) + { + /* SKIP_DATABASE_STATS is supported since v16 */ + Assert(serverVersion >= 160000); + appendPQExpBuffer(sql, "%sSKIP_DATABASE_STATS", sep); + sep = comma; + } if (vacopts->skip_locked) { /* SKIP_LOCKED is supported since v12 */ diff --git a/src/fe_utils/parallel_slot.c b/src/fe_utils/parallel_slot.c index 02368551f4..aca238bce9 100644 --- a/src/fe_utils/parallel_slot.c +++ b/src/fe_utils/parallel_slot.c @@ -475,6 +475,9 @@ ParallelSlotsWaitCompletion(ParallelSlotArray *sa) continue; if (!consumeQueryResult(&sa->slots[i])) return false; + /* Mark connection as idle */ + sa->slots[i].inUse = false; + ParallelSlotClearHandler(&sa->slots[i]); } return true; diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 5efb942368..689dbb7702 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -188,6 +188,8 @@ typedef struct VacAttrStats #define VACOPT_SKIP_LOCKED 0x20 /* skip if cannot get lock */ #define VACOPT_PROCESS_TOAST 0x40 /* process the TOAST table, if any */ #define VACOPT_DISABLE_PAGE_SKIPPING 0x80 /* don't skip any pages */ +#define VACOPT_SKIP_DATABASE_STATS 0x100 /* skip vac_update_datfrozenxid() */ +#define VACOPT_ONLY_DATABASE_STATS 0x200 /* only vac_update_datfrozenxid() */ /* * Values used by index_cleanup and truncate params. diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index 0035d158b7..d860be0e20 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -282,6 +282,12 @@ ALTER TABLE vactst ALTER COLUMN t SET STORAGE EXTERNAL; VACUUM (PROCESS_TOAST FALSE) vactst; VACUUM (PROCESS_TOAST FALSE, FULL) vactst; ERROR: PROCESS_TOAST required with VACUUM FULL +-- SKIP_DATABASE_STATS option +VACUUM (SKIP_DATABASE_STATS) vactst; +-- ONLY_DATABASE_STATS option +VACUUM (ONLY_DATABASE_STATS); +VACUUM (ONLY_DATABASE_STATS) vactst; -- error +ERROR: ONLY_DATABASE_STATS cannot be specified with a list of tables DROP TABLE vaccluster; DROP TABLE vactst; DROP TABLE vacparted; diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql index 9faa8a34a6..9da8f3e830 100644 --- a/src/test/regress/sql/vacuum.sql +++ b/src/test/regress/sql/vacuum.sql @@ -237,6 +237,13 @@ ALTER TABLE vactst ALTER COLUMN t SET STORAGE EXTERNAL; VACUUM (PROCESS_TOAST FALSE) vactst; VACUUM (PROCESS_TOAST FALSE, FULL) vactst; +-- SKIP_DATABASE_STATS option +VACUUM (SKIP_DATABASE_STATS) vactst; + +-- ONLY_DATABASE_STATS option +VACUUM (ONLY_DATABASE_STATS); +VACUUM (ONLY_DATABASE_STATS) vactst; -- error + DROP TABLE vaccluster; DROP TABLE vactst; DROP TABLE vacparted; -- 2.39.5