From 304839183156a11dbb33812ef040e0317f9d614b Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Mon, 1 Mar 2021 14:40:57 -0800 Subject: [PATCH v8 2/3] VACUUM ANALYZE: Distrust cleanup-only stats. Distrust the stats from VACUUM within VACUUM ANALYZE when we know that index AMs must only have had amvacuumcleanup() calls, without any calls to ambulkdelete(). This establishes the convention that amvacuumcleanup() usually only gives an estimate for num_index_tuples. --- src/include/commands/vacuum.h | 6 +++++ src/backend/access/heap/vacuumlazy.c | 15 +++++++++++- src/backend/commands/analyze.c | 34 ++++++++++++++++++++++++---- src/backend/commands/vacuum.c | 1 + src/backend/postmaster/autovacuum.c | 1 + 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index d029da5ac0..efacaf758a 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -221,6 +221,12 @@ typedef struct VacuumParams VacOptTernaryValue truncate; /* Truncate empty pages at the end, * default value depends on reloptions */ + /* XXX: output param approach is grotty, breaks backbranch ABI */ + + bool indexvacuuming; /* Output param: VACUUM took place and + * performed ambulkdelete calls for + * indexes? */ + /* * The number of parallel vacuum workers. 0 by default which means choose * based on the number of indexes. -1 indicates parallel vacuum is diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index d8f847b0e6..8716d305d0 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1054,6 +1054,12 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, lazy_vacuum_all_indexes(onerel, Irel, indstats, vacrelstats, lps, nindexes); + /* + * Remember index VACUUMing (not just cleanup) having taken + * place + */ + params->indexvacuuming = true; + /* Remove tuples from heap */ lazy_vacuum_heap(onerel, vacrelstats); @@ -1711,6 +1717,12 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, lazy_vacuum_all_indexes(onerel, Irel, indstats, vacrelstats, lps, nindexes); + /* + * Remember index VACUUMing (not just cleanup) having taken + * place + */ + params->indexvacuuming = true; + /* Remove tuples from heap */ lazy_vacuum_heap(onerel, vacrelstats); } @@ -1737,7 +1749,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, end_parallel_vacuum(indstats, lps, nindexes); /* Update index statistics */ - update_index_statistics(Irel, indstats, nindexes); + if (vacrelstats->useindex) + update_index_statistics(Irel, indstats, nindexes); /* If no indexes, make log report that lazy_vacuum_heap would've made */ if (vacuumed_pages) diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 7295cf0215..d28febdd4b 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -620,11 +620,21 @@ do_analyze_rel(Relation onerel, VacuumParams *params, } /* - * Same for indexes. Vacuum always scans all indexes, so if we're part of - * VACUUM ANALYZE, don't overwrite the accurate count already inserted by - * VACUUM. + * Same for indexes, at least in most cases. + * + * VACUUM usually scans all indexes. When we're part of VACUUM ANALYZE, + * and when VACUUM is known to have actually deleted index tuples, index + * AMs will generally give accurate reltuples -- so don't overwrite the + * accurate count already inserted by VACUUM. + * + * Most individual index AMs only give an estimate in the event of a + * cleanup-only VACUUM, though -- update stats in these cases, since our + * estimate will be at least as good anyway. (It's possible that + * individual index AMs will have accurate num_index_tuples statistics + * even for a cleanup-only VACUUM. We don't bother recognizing that; it's + * pretty rare.) */ - if (!inh && !(params->options & VACOPT_VACUUM)) + if (!inh && !params->indexvacuuming) { for (ind = 0; ind < nindexes; ind++) { @@ -654,9 +664,23 @@ do_analyze_rel(Relation onerel, VacuumParams *params, pgstat_report_analyze(onerel, totalrows, totaldeadrows, (va_cols == NIL)); - /* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */ + /* + * If this isn't part of VACUUM ANALYZE, let index AMs do cleanup. + * + * Note that most index AMs perform a no-op as a matter of policy for + * amvacuumcleanup() when called in ANALYZE-only mode, so in practice this + * usually does no work (GIN indexes rely on ANALYZE cleanup calls). + * + * Do not confuse this no-op case with the !indexvacuuming VACUUM ANALYZE + * case, which is the case where ambulkdelete() wasn't called for any + * indexes during a VACUUM or a VACUUM ANALYZE. There probably _were_ + * amvacuumcleanup() calls for VACUUM ANALYZE -- they probably did very + * little work, but they're not no-ops to the index AM generally. + */ if (!(params->options & VACOPT_VACUUM)) { + Assert(!params->indexvacuuming); + for (ind = 0; ind < nindexes; ind++) { IndexBulkDeleteResult *stats; diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index c064352e23..8e98ae98cd 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -110,6 +110,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) /* Set default value */ params.index_cleanup = VACOPT_TERNARY_DEFAULT; params.truncate = VACOPT_TERNARY_DEFAULT; + params.indexvacuuming = false; /* For now */ /* By default parallel vacuum is enabled */ params.nworkers = 0; diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 23ef23c13e..27d87bac34 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2925,6 +2925,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, (!wraparound ? VACOPT_SKIP_LOCKED : 0); tab->at_params.index_cleanup = VACOPT_TERNARY_DEFAULT; tab->at_params.truncate = VACOPT_TERNARY_DEFAULT; + tab->at_params.indexvacuuming = false; /* for now */ /* As of now, we don't support parallel vacuum for autovacuum */ tab->at_params.nworkers = -1; tab->at_params.freeze_min_age = freeze_min_age; -- 2.27.0