From 54447b4d869818cbb906b3de22fe2505f6e0d267 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Wed, 13 Oct 2021 15:01:44 -0700 Subject: [PATCH v1] Desupport analyze_only calls to amvacuumcleanup(). This can cause big problems with large GIN indexes that are run during autoanalyze. Other backends that run VACUUM have their OldestXmin cutoff held back senselessly. The behavior in question has been around since commit ff301d6e69, which introduced GIN's fastupdate mechanism. The idea at the time was to make sure that autovacuum workers that just ran ANALYZE would bulk insert pending entries, despite never performing a conventional VACUUM. This has been totally unnecessary since commit b07642db, which introduced autovacuum_vacuum_insert_threshold-driven autovacuums. Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-WzkjrK556enVtFLmyXEdw91xGuwiyZVep2kp5yQT_-3JDg@mail.gmail.com --- src/include/access/genam.h | 1 - src/backend/access/brin/brin.c | 4 ---- src/backend/access/gin/ginfast.c | 4 ++-- src/backend/access/gin/ginvacuum.c | 14 ----------- src/backend/access/gist/gistvacuum.c | 4 ---- src/backend/access/hash/hash.c | 1 - src/backend/access/heap/vacuumlazy.c | 2 -- src/backend/access/nbtree/nbtree.c | 4 ---- src/backend/access/spgist/spgvacuum.c | 4 ---- src/backend/catalog/index.c | 1 - src/backend/commands/analyze.c | 34 +++------------------------ contrib/bloom/blvacuum.c | 3 --- doc/src/sgml/gin.sgml | 2 +- doc/src/sgml/indexam.sgml | 9 ------- 14 files changed, 6 insertions(+), 81 deletions(-) diff --git a/src/include/access/genam.h b/src/include/access/genam.h index 480a4762f..90da48f91 100644 --- a/src/include/access/genam.h +++ b/src/include/access/genam.h @@ -44,7 +44,6 @@ typedef struct IndexBuildResult typedef struct IndexVacuumInfo { Relation index; /* the index being vacuumed */ - bool analyze_only; /* ANALYZE (without any actual vacuum) */ bool report_progress; /* emit progress.h status reports */ bool estimated_count; /* num_heap_tuples is an estimate */ int message_level; /* ereport level for progress messages */ diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index ccc9fa095..2d336355a 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -940,10 +940,6 @@ brinvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats) { Relation heapRel; - /* No-op in ANALYZE ONLY mode */ - if (info->analyze_only) - return stats; - if (!stats) stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult)); stats->num_pages = RelationGetNumberOfBlocks(info->index); diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c index e0d994094..a1a045cdd 100644 --- a/src/backend/access/gin/ginfast.c +++ b/src/backend/access/gin/ginfast.c @@ -801,8 +801,8 @@ ginInsertCleanup(GinState *ginstate, bool full_clean, if (forceCleanup) { /* - * We are called from [auto]vacuum/analyze or gin_clean_pending_list() - * and we would like to wait concurrent cleanup to finish. + * We are called from [auto]vacuum or gin_clean_pending_list() and we + * would like to wait for concurrent cleanup to finish */ LockPage(index, GIN_METAPAGE_BLKNO, ExclusiveLock); workMemory = diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c index a276eb020..f92e7cd5d 100644 --- a/src/backend/access/gin/ginvacuum.c +++ b/src/backend/access/gin/ginvacuum.c @@ -695,20 +695,6 @@ ginvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats) GinState ginstate; GinStatsData idxStat; - /* - * In an autovacuum analyze, we want to clean up pending insertions. - * Otherwise, an ANALYZE-only call is a no-op. - */ - if (info->analyze_only) - { - if (IsAutoVacuumWorkerProcess()) - { - initGinState(&ginstate, index); - ginInsertCleanup(&ginstate, false, true, true, stats); - } - return stats; - } - /* * Set up all-zero stats and cleanup pending inserts if ginbulkdelete * wasn't called diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c index 066319353..96f5ed8ee 100644 --- a/src/backend/access/gist/gistvacuum.c +++ b/src/backend/access/gist/gistvacuum.c @@ -74,10 +74,6 @@ gistbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, IndexBulkDeleteResult * gistvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats) { - /* No-op in ANALYZE ONLY mode */ - if (info->analyze_only) - return stats; - /* * If gistbulkdelete was called, we need not do anything, just return the * stats from the latest gistbulkdelete call. If it wasn't called, we diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c index eb3810494..c296355f0 100644 --- a/src/backend/access/hash/hash.c +++ b/src/backend/access/hash/hash.c @@ -647,7 +647,6 @@ hashvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats) BlockNumber num_pages; /* If hashbulkdelete wasn't called, return NULL signifying no change */ - /* Note: this covers the analyze_only case too */ if (stats == NULL) return NULL; diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 05221cc1d..8d98dd686 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -3023,7 +3023,6 @@ lazy_vacuum_one_index(Relation indrel, IndexBulkDeleteResult *istat, pg_rusage_init(&ru0); ivinfo.index = indrel; - ivinfo.analyze_only = false; ivinfo.report_progress = false; ivinfo.estimated_count = true; ivinfo.message_level = elevel; @@ -3079,7 +3078,6 @@ lazy_cleanup_one_index(Relation indrel, IndexBulkDeleteResult *istat, pg_rusage_init(&ru0); ivinfo.index = indrel; - ivinfo.analyze_only = false; ivinfo.report_progress = false; ivinfo.estimated_count = estimated_count; ivinfo.message_level = elevel; diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 40ad0956e..3adf54b4a 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -815,10 +815,6 @@ btvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats) { BlockNumber num_delpages; - /* No-op in ANALYZE ONLY mode */ - if (info->analyze_only) - return stats; - /* * If btbulkdelete was called, we need not do anything (we just maintain * the information used within _bt_vacuum_needs_cleanup() by calling diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c index 76fb0374c..5f4c41956 100644 --- a/src/backend/access/spgist/spgvacuum.c +++ b/src/backend/access/spgist/spgvacuum.c @@ -938,10 +938,6 @@ spgvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats) { spgBulkDeleteState bds; - /* No-op in ANALYZE ONLY mode */ - if (info->analyze_only) - return stats; - /* * We don't need to scan the index if there was a preceding bulkdelete * pass. Otherwise, make a pass that won't delete any live tuples, but diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 26bfa74ce..864683768 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3299,7 +3299,6 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot) * Scan the index and gather up all the TIDs into a tuplesort object. */ ivinfo.index = indexRelation; - ivinfo.analyze_only = false; ivinfo.report_progress = true; ivinfo.estimated_count = true; ivinfo.message_level = DEBUG2; diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 8bfb2ad95..5d621ff09 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -681,6 +681,9 @@ do_analyze_rel(Relation onerel, VacuumParams *params, in_outer_xact); } + /* Done with indexes */ + vac_close_indexes(nindexes, Irel, NoLock); + /* * Now report ANALYZE to the stats collector. For regular tables, we do * it only if not doing inherited stats. For partitioned tables, we only @@ -696,37 +699,6 @@ do_analyze_rel(Relation onerel, VacuumParams *params, else if (onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) pgstat_report_analyze(onerel, 0, 0, (va_cols == NIL)); - /* - * 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. The only exception - * among core index AMs is GIN/ginvacuumcleanup(). - */ - if (!(params->options & VACOPT_VACUUM)) - { - for (ind = 0; ind < nindexes; ind++) - { - IndexBulkDeleteResult *stats; - IndexVacuumInfo ivinfo; - - ivinfo.index = Irel[ind]; - ivinfo.analyze_only = true; - ivinfo.estimated_count = true; - ivinfo.message_level = elevel; - ivinfo.num_heap_tuples = onerel->rd_rel->reltuples; - ivinfo.strategy = vac_strategy; - - stats = index_vacuum_cleanup(&ivinfo, NULL); - - if (stats) - pfree(stats); - } - } - - /* Done with indexes */ - vac_close_indexes(nindexes, Irel, NoLock); - /* Log the action if appropriate */ if (IsAutoVacuumWorkerProcess() && params->log_min_duration >= 0) { diff --git a/contrib/bloom/blvacuum.c b/contrib/bloom/blvacuum.c index 88b0a6d29..97b4a1b79 100644 --- a/contrib/bloom/blvacuum.c +++ b/contrib/bloom/blvacuum.c @@ -172,9 +172,6 @@ blvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats) BlockNumber npages, blkno; - if (info->analyze_only) - return stats; - if (stats == NULL) stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult)); diff --git a/doc/src/sgml/gin.sgml b/doc/src/sgml/gin.sgml index d68d12d51..09cf95220 100644 --- a/doc/src/sgml/gin.sgml +++ b/doc/src/sgml/gin.sgml @@ -510,7 +510,7 @@ from the indexed item). GIN is capable of postponing much of this work by inserting new tuples into a temporary, unsorted list of pending entries. - When the table is vacuumed or autoanalyzed, or when + When the table is vacuumed, or when the gin_clean_pending_list function is called, or if the pending list becomes larger than , the entries are moved to the diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml index cf359fa9f..0c86dcc27 100644 --- a/doc/src/sgml/indexam.sgml +++ b/doc/src/sgml/indexam.sgml @@ -395,15 +395,6 @@ amvacuumcleanup (IndexVacuumInfo *info, be returned. - - amvacuumcleanup will also be called at completion of an - ANALYZE operation. In this case stats is always - NULL and any return value will be ignored. This case can be distinguished - by checking info->analyze_only. It is recommended - that the access method do nothing except post-insert cleanup in such a - call, and that only in an autovacuum worker process. - - bool -- 2.30.2