Removing vacuum_cleanup_index_scale_factor

Started by Masahiko Sawadaalmost 5 years ago9 messages

sawada.mshk@gmail.com

almost 5 years ago

Hi all,

I've started this new thread separated from the thread[1]/messages/by-id/CAH2-WznUWHOL+nYYT2PLsn+3OWcq8OBfA1sB3FX885rE=ZQVvA@mail.gmail.com to discuss
removing vacuum_cleanup_index_scale_factor GUC parameter proposed by
Peter Geoghegan.

btvacuumcleanup() has been playing two roles: recycling deleted pages
and collecting index statistics. This discussion focuses on the
latter. Since PG11, btvacuumcleanup() uses
vacuum_cleanup_index_scale_factor as a threshold to do an index scan
to update index statistics (pg_class.reltuples and pg_class.relpages).
Therefore, even if there is no update on the btree index at all (e.g.,
btbulkdelete() was not called earlier), btvacuumcleanup() scans the
whole index to collect the index statistics if the number of newly
inserted tuples exceeds the vacuum_cleanup_index_scale_factor fraction
of the total number of heap tuples detected by the previous statistics
collection. On the other hand, those index statistics are updated also
by ANALYZE and autoanalyze. pg_class.reltuples calculated by ANALYZE
is an estimation whereas the value returned by btvacuumcleanup() is an
accurate value. But perhaps we can rely on ANALYZE and autoanalyze to
update those index statistics. The points of this discussion are what
we really need to do in btvacuumcleanup() and whether
btvacuumcleanup() really needs to do an index scan for the purpose of
index statistics update.

The original design that made VACUUM set
pg_class.reltuples/pg_class.relpages in indexes (from 15+ years ago)
assumed that it was cheap to handle statistics in passing. Even if we
have btvacuumcleanup() not do an index scan at all, this is 100%
allowed by the amvacuumcleanup contract described in the
documentation:

"It is OK to return NULL if the index was not changed at all during
the VACUUM operation, but otherwise correct stats should be returned."

The above description was added by commit e57345975cf in 2006 and
hasn't changed for now.

For instance, looking at hash indexes, it hasn't really changed since
2006 in terms of amvacuumcleanup(). hashvacuumcleanup() simply sets
stats->num_pages and stats->num_index_tuples without an index scan.
I'd like to quote the in-depth analysis by Peter Geoghegan:

-----
/*
* Post-VACUUM cleanup.
*
* Result: a palloc'd struct containing statistical info for VACUUM displays.
*/
IndexBulkDeleteResult *
hashvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
{
Relation rel = info->index;
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;

/* update statistics */
num_pages = RelationGetNumberOfBlocks(rel);
stats->num_pages = num_pages;

return stats;
}

Clearly hashvacuumcleanup() was considered by Tom when he revised the
documentation in 2006. Here are some observations about
hashvacuumcleanup() that seem relevant now:

* There is no "analyze_only" handling, just like nbtree.

"analyze_only" is only used by GIN, even now, 15+ years after it was
added. GIN uses it to make autovacuum workers (never VACUUM outside of
an AV worker) do pending list insertions for ANALYZE -- just to make
it happen more often. This is a niche thing -- clearly we don't have
to care about it in nbtree, even if we make btvacuumcleanup() (almost)
always return NULL when there was no btbulkdelete() call.

* num_pages (which will become pg_class.relpages for the index) is not
set when we return NULL -- hashvacuumcleanup() assumes that ANALYZE
will get to it eventually in the case where VACUUM does no real work
(when it just returns NULL).

* We also use RelationGetNumberOfBlocks() to set pg_class.relpages for
index relations during ANALYZE -- it's called when we call
vac_update_relstats() (I quoted this do_analyze_rel() code to you
directly in a recent email).

* In general, pg_class.relpages isn't an estimate (because we use
RelationGetNumberOfBlocks(), both in the VACUUM-updates case and the
ANALYZE-updates case) -- only pg_class.reltuples is truly an estimate
during ANALYZE, and so getting a "true count" seems to have only
limited practical importance.

I think that this sets a precedent in support of my view that we can
simply get rid of vacuum_cleanup_index_scale_factor without any
special effort to maintain pg_class.reltuples. As I said before, we
can safely make btvacuumcleanup() just like hashvacuumcleanup(),
except when there are known deleted-but-not-recycled pages, where a
full index scan really is necessary for reasons that are not related
to statistics at all (of course we still need the *logic* that was
added to nbtree by the vacuum_cleanup_index_scale_factor commit --
that is clearly necessary). My guess is that Tom would have made
btvacuumcleanup() look identical to hashvacuumcleanup() in 2006 if
nbtree didn't have page deletion to consider -- but that had to be
considered.
-----

The above discussions make sense to me as a support for the "removing
vacuum_cleanup_index_scale_factor GUC" proposal. The difference
between the index statistics taken by ANALYZE and btvacuumcleanup() is
that the former statistics is always an estimation. That’s calculated
by compute_index_stats() whereas the latter uses the result of an
index scan. If btvacuumcleanup() doesn’t scan the index and always
returns NULL, it would become hard to collect accurate index
statistics, for example in a static table case. But if collecting an
accurate pg_class.reltuples is not important in practice, I agree that
we don't need btvacuumcleanup() to do an index scan for collecting
statistics purposes.

What do you think about removing vacuum_cleanup_index_scale_factor
parameter? and we'd like to ask the design principles of
amvacuumcleanup() considered in 2006 by various hackers (mostly Tom).
What do you think, Tom?

Regards,

[1]: /messages/by-id/CAH2-WznUWHOL+nYYT2PLsn+3OWcq8OBfA1sB3FX885rE=ZQVvA@mail.gmail.com

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

Peter Geoghegan

pg@bowt.ie

almost 5 years ago

In reply to: Masahiko Sawada (#1)

Re: Removing vacuum_cleanup_index_scale_factor

On Mon, Mar 1, 2021 at 10:33 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

The original design that made VACUUM set
pg_class.reltuples/pg_class.relpages in indexes (from 15+ years ago)
assumed that it was cheap to handle statistics in passing. Even if we
have btvacuumcleanup() not do an index scan at all, this is 100%
allowed by the amvacuumcleanup contract described in the
documentation:

"It is OK to return NULL if the index was not changed at all during
the VACUUM operation, but otherwise correct stats should be returned."

The above description was added by commit e57345975cf in 2006 and
hasn't changed for now.

The intention here is not to revise the amvacuumcleanup() contract --
the contract already allows us to do what we want inside nbtree. We
want to teach btvacuumcleanup() to not do any real work, at least
outside of rare cases where we have known deleted pages that must
still be placed in the FSM for recycling -- btvacuumcleanup() would
generally just return NULL when there was no btbulkdelete() call
during the same VACUUM operation (the main thing that prevents this
today is vacuum_cleanup_index_scale_factor). More generally, we would
like to change the *general* expectations that we make of index AMs in
places like vacuumlazy.c and analyze.c. But we're worried about
dependencies that aren't formalized anywhere but still matter -- code
may have evolved to assume that index AMs behaved a certain way in
common and important cases (probably also in code like vacuumlazy.c).
That's what we want to avoid breaking.

Masahiko has already given an example of such a problem: currently,
VACUUM ANALYZE simply assumes that its VACUUM will call each indexes'
amvacuumcleanup() routine in all cases, and will have each call set
pg_class.reltuples and pg_class.relpages in respect of each index.
ANALYZE therefore avoids overwriting indexes' pg_class stats inside
do_analyze_rel() (at the point where it calls vac_update_relstats()
for each index). That approach is already wrong with hash indexes, but
under this new directly for btvacuumcleanup(), it would become wrong
in just the same way with nbtree indexes (if left unaddressed).

Clearly "the letter of the law" and "the spirit of the law" must both
be considered. We want to talk about the latter on this thread.
Concretely, here are specific questions (perhaps Tom can weigh in on
these as the principal designer of the relevant interfaces):

1. Any objections to the idea of teaching VACUUM ANALYZE to
distinguish between the cases where VACUUM ran and performed "real
index vacuuming", to make it more intelligent about overwriting
pg_class stats for indexes?

I define "real index vacuuming" as calling any indexes ambulkdelete() routine.

2. Does anybody anticipate any other issues? Possibly an issue that
resembles this existing known VACUUM ANALYZE issue?

Thanks
--
Peter Geoghegan

Peter Geoghegan

pg@bowt.ie

almost 5 years ago

In reply to: Peter Geoghegan (#2)

Re: Removing vacuum_cleanup_index_scale_factor

On Tue, Mar 2, 2021 at 6:01 PM Peter Geoghegan <pg@bowt.ie> wrote:

1. Any objections to the idea of teaching VACUUM ANALYZE to
distinguish between the cases where VACUUM ran and performed "real
index vacuuming", to make it more intelligent about overwriting
pg_class stats for indexes?

I think that a simpler approach would work better: When
ANALYZE/do_analyze_rel() decides whether or not it should call
vac_update_relstats() for each index, it should simply not care
whether or not this is a VACUUM ANALYZE (as opposed to a simple
ANALYZE). This is already what we do for the heap relation itself. Why
shouldn't we do something similar for indexes?

What do you think, Tom? Your bugfix commit b4b6923e03f from 2011
taught do_analyze_rel() to not care about whether VACUUM took place
earlier in the same command -- though only in the case of the heap
relation (not in the case of its indexes). That decision now seems a
bit arbitrary to me.

I should point out that this is the *opposite* of what we did from
2004 - 2011 (following Tom's 2004 commit f0c9397f808). During that
time the policy was to not update pg_class.reltuples inside
do_analyze_rel() when we knew that VACUUM ran. The policy was at least
the same for indexes and the heap/table during this period, so it was
consistent in that sense. However, I don't think that we should
reintroduce that policy now. Doing so would be contrary to the API
contract for index AMs established by Tom's 2006 commit e57345975cf --
that allowed amvacuumcleanup() to be a no-op when there was no
ambulkdelete() call (it also taught hashvacuumcleanup() to do just
that).

To recap, our ultimate goal here is to make btvacuumcleanup() close to
hashvacuumcleanup() -- it should be able to skip all cleanup when
there was no btbulkdelete() call during the same VACUUM (nbtree page
deletion still has cases that force us to do real work in the absence
of a btbulkdelete() call for the VACUUM, but that remaining exception
should be very rare).

--
Peter Geoghegan

Tom Lane

tgl@sss.pgh.pa.us

almost 5 years ago

In reply to: Peter Geoghegan (#3)

Re: Removing vacuum_cleanup_index_scale_factor

Peter Geoghegan <pg@bowt.ie> writes:

I think that a simpler approach would work better: When
ANALYZE/do_analyze_rel() decides whether or not it should call
vac_update_relstats() for each index, it should simply not care
whether or not this is a VACUUM ANALYZE (as opposed to a simple
ANALYZE). This is already what we do for the heap relation itself. Why
shouldn't we do something similar for indexes?

What do you think, Tom? Your bugfix commit b4b6923e03f from 2011
taught do_analyze_rel() to not care about whether VACUUM took place
earlier in the same command -- though only in the case of the heap
relation (not in the case of its indexes). That decision now seems a
bit arbitrary to me.

Well, nobody had complained about the index stats at that point,
so I don't think I was thinking about that aspect of it.

As you say, the history here is a bit convoluted, but it seems like
a good principle to avoid interconnections between VACUUM and ANALYZE
as much as we can. I haven't been paying enough attention to this
thread to have more insight than that.

regards, tom lane

Peter Geoghegan

pg@bowt.ie

almost 5 years ago

In reply to: Tom Lane (#4)

2 attachment(s)

Re: Removing vacuum_cleanup_index_scale_factor

On Mon, Mar 8, 2021 at 1:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

As you say, the history here is a bit convoluted, but it seems like
a good principle to avoid interconnections between VACUUM and ANALYZE
as much as we can. I haven't been paying enough attention to this
thread to have more insight than that.

The attached patch does what I proposed earlier today: it teaches
do_analyze_rel() to always set pg_class.reltuples for indexes when it
would do the same thing for the heap/table relation already. It's now
uniform in that sense.

Also included is a patch that removes the
vacuum_cleanup_index_scale_factor mechanism for triggering an index
scan during VACUUM -- that's what the second patch does (this depends
on the first patch, really).

Do you think that a backpatch to Postgres 13 for both of these patches
would be acceptable? There are two main concerns that I have in mind
here, both of which are only issues in Postgres 13:

1. Arguably the question of skipping scanning the index should have been
considered by the autovacuum_vacuum_insert_scale_factor patch when it
was committed for Postgres 13 -- but it wasn't. There is a regression
that was tied to autovacuum_vacuum_insert_scale_factor in Postgres 13
by Mark Callaghan:

https://smalldatum.blogspot.com/2021/01/insert-benchmark-postgres-is-still.html

The blog post says: "Updates - To understand the small regression
mentioned above for the l.i1 test (more CPU & write IO) I repeated the
test with 100M rows using 2 configurations: one disabled index
deduplication and the other disabled insert-triggered autovacuum.
Disabling index deduplication had no effect and disabling
insert-triggered autovacuum resolves the regression."

I think that this regression is almost entirely explainable by the
need to unnecessarily scan indexes for autovacuum VACUUMs that just
need to set the visibility map. This issue is basically avoidable,
just by removing the vacuum_cleanup_index_scale_factor cleanup-only
VACUUM criteria (per my second patch).

2. I fixed a bug in nbtree deduplication btvacuumcleanup() stats in
commit 48e12913. This fix still left things in kind of a bad state:
there are still cases where the btvacuumcleanup()-only VACUUM case
will set pg_class.reltuples to a value that is significantly below
what it should be (it all depends on how effective deduplication is
with the data). These remaining cases are effectively fixed by the
second patch.

I probably should have made btvacuumcleanup()-only VACUUMs set
"stats->estimate_count = true" when I was working on the fix that
became commit 48e12913. Purely because my approach was inherently
approximate with posting list tuples, and so shouldn't be trusted for
anything important (num_index_tuples is suitable for VACUUM VERBOSE
output only in affected cases). I didn't set "stats->estimate_count =
true" in affected cases because I was worried about unforeseen
consequences. But this seems defensible now, all things considered.

There are other things that are slightly broken but will be fixed by
the first patch. But I'm really just worried about these two cases in
Postgres 13.

Thanks for weighing in
--
Peter Geoghegan

Attachments:

0001-VACUUM-ANALYZE-Always-set-pg_class.reltuples.patchapplication/x-patch; name=0001-VACUUM-ANALYZE-Always-set-pg_class.reltuples.patchDownload

From 29079ec92eed077c8d4d65e4c3db16ce41578657 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Mon, 1 Mar 2021 14:40:57 -0800
Subject: [PATCH 1/2] VACUUM ANALYZE: Always set pg_class.reltuples.

Stop assuming that pg_class must have been updated with accurate
statistics within VACUUM ANALYZE -- update pg_class for indexes at the
same time as the table relation in all cases.

VACUUM sometimes gives less accurate statistics than ANALYZE, or no
statistics at all.  It seems advisable to effectively assume that that
always happens.  The worst that can happen is that we'll inaccurately
set pg_class.reltuples for indexes whose heap relation would already
have had an inaccurate pg_class.reltuples anyway.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WzknxdComjhqo4SUxVFk_Q1171GJO2ZgHZ1Y6pion6u8rA@mail.gmail.com
---
 src/backend/access/heap/vacuumlazy.c |  3 +-
 src/backend/commands/analyze.c       | 49 ++++++++++++++++------------
 2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d8f847b0e6..22ca7f5c54 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1737,7 +1737,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..7ae1cd426c 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -600,8 +600,22 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 								 PROGRESS_ANALYZE_PHASE_FINALIZE_ANALYZE);
 
 	/*
-	 * Update pages/tuples stats in pg_class ... but not if we're doing
-	 * inherited stats.
+	 * Update pages/tuples stats in pg_class, and report ANALYZE to the stats
+	 * collector ... but not if we're doing inherited stats.
+	 *
+	 * We make a uniform assumption that VACUUM hasn't accurately set a
+	 * pg_class.reltuples value, regardless of whether this is a VACUUM
+	 * ANALYZE.  This is possible with a "VACUUM (ANALYZE, INDEX_CLEANUP OFF)"
+	 * command.  It's also possible with index AMs that opt to return NULL
+	 * from their amvacuumcleanup() routine (which is permitted by the index
+	 * AM interface in cases where the index doesn't have any dead tuples).
+	 * Besides, it seems like a good idea to only update pg_class.reltuples in
+	 * the heap relation when we'll do the same for its indexes.
+	 *
+	 * We don't report to the stats collector here because the stats collector
+	 * only tracks per-table stats.  Reset the changes_since_analyze counter
+	 * only if we analyzed all columns; otherwise, there is still work for
+	 * auto-analyze to do.
 	 */
 	if (!inh)
 	{
@@ -609,6 +623,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 
 		visibilitymap_count(onerel, &relallvisible, NULL);
 
+		/* Update pg_class for table relation */
 		vac_update_relstats(onerel,
 							relpages,
 							totalrows,
@@ -617,15 +632,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 							InvalidTransactionId,
 							InvalidMultiXactId,
 							in_outer_xact);
-	}
 
-	/*
-	 * 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.
-	 */
-	if (!inh && !(params->options & VACOPT_VACUUM))
-	{
+		/* Same for indexes */
 		for (ind = 0; ind < nindexes; ind++)
 		{
 			AnlIndexData *thisdata = &indexdata[ind];
@@ -641,20 +649,21 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 								InvalidMultiXactId,
 								in_outer_xact);
 		}
+
+		/* Now report ANALYZE to the stats collector */
+		pgstat_report_analyze(onerel, totalrows, totaldeadrows,
+							  (va_cols == NIL));
 	}
 
 	/*
-	 * Report ANALYZE to the stats collector, too.  However, if doing
-	 * inherited stats we shouldn't report, because the stats collector only
-	 * tracks per-table stats.  Reset the changes_since_analyze counter only
-	 * if we analyzed all columns; otherwise, there is still work for
-	 * auto-analyze to do.
+	 * 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.  In practice only
+	 * ginvacuumcleanup() currently does real work when called through here
+	 * (and even GIN makes it a no-op unless we're running is an autovacuum
+	 * worker).
 	 */
-	if (!inh)
-		pgstat_report_analyze(onerel, totalrows, totaldeadrows,
-							  (va_cols == NIL));
-
-	/* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */
 	if (!(params->options & VACOPT_VACUUM))
 	{
 		for (ind = 0; ind < nindexes; ind++)
-- 
2.27.0

0002-Remove-vacuum_cleanup_index_scale_factor-GUC-param.patchapplication/x-patch; name=0002-Remove-vacuum_cleanup_index_scale_factor-GUC-param.patchDownload

From 0bda6b4c02ce135e208dc0bbb4004d4c8b7de695 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Mon, 1 Mar 2021 15:58:56 -0800
Subject: [PATCH 2/2] Remove vacuum_cleanup_index_scale_factor GUC + param.

Always skip full index scan during a VACUUM for nbtree indexes in the
case where VACUUM never called btbulkdelete(), even when pg_class stats
for the index relation would be considered "stale" by criteria applied
using vacuum_cleanup_index_scale_factor (remove the GUC and storage
param entirely).  It should be fine to rely on ANALYZE to keep
pg_class.reltuples up to date for nbtree indexes, which is the behavior
of hashvacuumcleanup()/hash indexes.

This still means that we can do a cleanup-only scan of the index for the
one remaining case where that makes sense: to recycle pages known to be
deleted but not yet recycled following a previous VACUUM.  However,
cleanup-only nbtree VACUUMS that scan the index will now be very rare.
---
 src/include/access/nbtree.h                   |  5 +-
 src/include/access/nbtxlog.h                  |  1 -
 src/include/miscadmin.h                       |  2 -
 src/backend/access/common/reloptions.c        |  9 ---
 src/backend/access/nbtree/nbtinsert.c         |  3 -
 src/backend/access/nbtree/nbtpage.c           | 40 ++++------
 src/backend/access/nbtree/nbtree.c            | 75 ++++++-------------
 src/backend/access/nbtree/nbtutils.c          |  2 -
 src/backend/access/nbtree/nbtxlog.c           |  2 +-
 src/backend/access/rmgrdesc/nbtdesc.c         |  5 +-
 src/backend/utils/init/globals.c              |  2 -
 src/backend/utils/misc/guc.c                  | 10 ---
 src/backend/utils/misc/postgresql.conf.sample |  3 -
 src/bin/psql/tab-complete.c                   |  4 +-
 doc/src/sgml/config.sgml                      | 40 ----------
 doc/src/sgml/ref/create_index.sgml            | 14 ----
 src/test/regress/expected/btree_index.out     | 29 -------
 src/test/regress/sql/btree_index.sql          | 19 -----
 18 files changed, 43 insertions(+), 222 deletions(-)

diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index b56b7b7868..c9d6000cbe 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -1067,8 +1067,6 @@ typedef struct BTOptions
 {
 	int32		varlena_header_;	/* varlena header (do not touch directly!) */
 	int			fillfactor;		/* page fill factor in percent (0..100) */
-	/* fraction of newly inserted tuples needed to trigger index cleanup */
-	float8		vacuum_cleanup_index_scale_factor;
 	bool		deduplicate_items;	/* Try to deduplicate items? */
 } BTOptions;
 
@@ -1171,8 +1169,7 @@ extern OffsetNumber _bt_findsplitloc(Relation rel, Page origpage,
  */
 extern void _bt_initmetapage(Page page, BlockNumber rootbknum, uint32 level,
 							 bool allequalimage);
-extern void _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
-								 float8 num_heap_tuples);
+extern void _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages);
 extern void _bt_upgrademetapage(Page page);
 extern Buffer _bt_getroot(Relation rel, int access);
 extern Buffer _bt_gettrueroot(Relation rel);
diff --git a/src/include/access/nbtxlog.h b/src/include/access/nbtxlog.h
index 3df34fcda2..0f7731856b 100644
--- a/src/include/access/nbtxlog.h
+++ b/src/include/access/nbtxlog.h
@@ -54,7 +54,6 @@ typedef struct xl_btree_metadata
 	BlockNumber fastroot;
 	uint32		fastlevel;
 	uint32		last_cleanup_num_delpages;
-	float8		last_cleanup_num_heap_tuples;
 	bool		allequalimage;
 } xl_btree_metadata;
 
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 1bdc97e308..54693e047a 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -261,8 +261,6 @@ extern int64 VacuumPageDirty;
 extern int	VacuumCostBalance;
 extern bool VacuumCostActive;
 
-extern double vacuum_cleanup_index_scale_factor;
-
 
 /* in tcop/postgres.c */
 
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index c687d3ee9e..433e236722 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -461,15 +461,6 @@ static relopt_real realRelOpts[] =
 		},
 		0, -1.0, DBL_MAX
 	},
-	{
-		{
-			"vacuum_cleanup_index_scale_factor",
-			"Number of tuple inserts prior to index cleanup as a fraction of reltuples.",
-			RELOPT_KIND_BTREE,
-			ShareUpdateExclusiveLock
-		},
-		-1, 0.0, 1e10
-	},
 	/* list terminator */
 	{{NULL}}
 };
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 1edb9f9579..0bc86943eb 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -1332,8 +1332,6 @@ _bt_insertonpg(Relation rel,
 					xlmeta.fastroot = metad->btm_fastroot;
 					xlmeta.fastlevel = metad->btm_fastlevel;
 					xlmeta.last_cleanup_num_delpages = metad->btm_last_cleanup_num_delpages;
-					xlmeta.last_cleanup_num_heap_tuples =
-						metad->btm_last_cleanup_num_heap_tuples;
 					xlmeta.allequalimage = metad->btm_allequalimage;
 
 					XLogRegisterBuffer(2, metabuf,
@@ -2549,7 +2547,6 @@ _bt_newroot(Relation rel, Buffer lbuf, Buffer rbuf)
 		md.fastroot = rootblknum;
 		md.fastlevel = metad->btm_level;
 		md.last_cleanup_num_delpages = metad->btm_last_cleanup_num_delpages;
-		md.last_cleanup_num_heap_tuples = metad->btm_last_cleanup_num_heap_tuples;
 		md.allequalimage = metad->btm_allequalimage;
 
 		XLogRegisterBufData(2, (char *) &md, sizeof(xl_btree_metadata));
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index c09e492a5f..4a0578dff4 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -175,26 +175,15 @@ _bt_getmeta(Relation rel, Buffer metabuf)
  *		_bt_vacuum_needs_cleanup() to decide whether or not a btvacuumscan()
  *		call should go ahead for an entire VACUUM operation.
  *
- *		See btvacuumcleanup() and _bt_vacuum_needs_cleanup() for details of
- *		the two fields that we maintain here.
- *
- *		The information that we maintain for btvacuumcleanup() describes the
- *		state of the index (as well as the table it indexes) just _after_ the
- *		ongoing VACUUM operation.  The next _bt_vacuum_needs_cleanup() call
- *		will consider the information we saved for it during the next VACUUM
- *		operation (assuming that there will be no btbulkdelete() call during
- *		the next VACUUM operation -- if there is then the question of skipping
- *		btvacuumscan() doesn't even arise).
+ *		See btvacuumcleanup() and _bt_vacuum_needs_cleanup() for the
+ *		definition of num_delpages.
  */
 void
-_bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
-					 float8 num_heap_tuples)
+_bt_set_cleanup_info(Relation rel, BlockNumber num_delpages)
 {
 	Buffer		metabuf;
 	Page		metapg;
 	BTMetaPageData *metad;
-	bool		rewrite = false;
-	XLogRecPtr	recptr;
 
 	/*
 	 * On-disk compatibility note: The btm_last_cleanup_num_delpages metapage
@@ -209,21 +198,20 @@ _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
 	 * in reality there are only one or two.  The worst that can happen is
 	 * that there will be a call to btvacuumscan a little earlier, which will
 	 * set btm_last_cleanup_num_delpages to a sane value when we're called.
+	 *
+	 * Note also that the metapage's btm_last_cleanup_num_heap_tuples field is
+	 * no longer used as of PostgreSQL 14.  We set it to -1.0 on rewrite, just
+	 * to be consistent.
 	 */
 	metabuf = _bt_getbuf(rel, BTREE_METAPAGE, BT_READ);
 	metapg = BufferGetPage(metabuf);
 	metad = BTPageGetMeta(metapg);
 
-	/* Always dynamically upgrade index/metapage when BTREE_MIN_VERSION */
-	if (metad->btm_version < BTREE_NOVAC_VERSION)
-		rewrite = true;
-	else if (metad->btm_last_cleanup_num_delpages != num_delpages)
-		rewrite = true;
-	else if (metad->btm_last_cleanup_num_heap_tuples != num_heap_tuples)
-		rewrite = true;
-
-	if (!rewrite)
+	/* Don't miss chance to upgrade index/metapage when BTREE_MIN_VERSION */
+	if (metad->btm_version >= BTREE_NOVAC_VERSION &&
+		metad->btm_last_cleanup_num_delpages == num_delpages)
 	{
+		/* Usually means index continues to have num_delpages of 0 */
 		_bt_relbuf(rel, metabuf);
 		return;
 	}
@@ -240,13 +228,14 @@ _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
 
 	/* update cleanup-related information */
 	metad->btm_last_cleanup_num_delpages = num_delpages;
-	metad->btm_last_cleanup_num_heap_tuples = num_heap_tuples;
+	metad->btm_last_cleanup_num_heap_tuples = -1.0;
 	MarkBufferDirty(metabuf);
 
 	/* write wal record if needed */
 	if (RelationNeedsWAL(rel))
 	{
 		xl_btree_metadata md;
+		XLogRecPtr	recptr;
 
 		XLogBeginInsert();
 		XLogRegisterBuffer(0, metabuf, REGBUF_WILL_INIT | REGBUF_STANDARD);
@@ -258,7 +247,6 @@ _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
 		md.fastroot = metad->btm_fastroot;
 		md.fastlevel = metad->btm_fastlevel;
 		md.last_cleanup_num_delpages = num_delpages;
-		md.last_cleanup_num_heap_tuples = num_heap_tuples;
 		md.allequalimage = metad->btm_allequalimage;
 
 		XLogRegisterBufData(0, (char *) &md, sizeof(xl_btree_metadata));
@@ -443,7 +431,6 @@ _bt_getroot(Relation rel, int access)
 			md.fastroot = rootblkno;
 			md.fastlevel = 0;
 			md.last_cleanup_num_delpages = 0;
-			md.last_cleanup_num_heap_tuples = -1.0;
 			md.allequalimage = metad->btm_allequalimage;
 
 			XLogRegisterBufData(2, (char *) &md, sizeof(xl_btree_metadata));
@@ -2632,7 +2619,6 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno,
 			xlmeta.fastroot = metad->btm_fastroot;
 			xlmeta.fastlevel = metad->btm_fastlevel;
 			xlmeta.last_cleanup_num_delpages = metad->btm_last_cleanup_num_delpages;
-			xlmeta.last_cleanup_num_heap_tuples = metad->btm_last_cleanup_num_heap_tuples;
 			xlmeta.allequalimage = metad->btm_allequalimage;
 
 			XLogRegisterBufData(4, (char *) &xlmeta, sizeof(xl_btree_metadata));
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 504f5bef17..f2818b1ca3 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -789,11 +789,8 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
 	Buffer		metabuf;
 	Page		metapg;
 	BTMetaPageData *metad;
-	BTOptions  *relopts;
-	float8		cleanup_scale_factor;
 	uint32		btm_version;
 	BlockNumber prev_num_delpages;
-	float8		prev_num_heap_tuples;
 
 	/*
 	 * Copy details from metapage to local variables quickly.
@@ -816,32 +813,8 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
 	}
 
 	prev_num_delpages = metad->btm_last_cleanup_num_delpages;
-	prev_num_heap_tuples = metad->btm_last_cleanup_num_heap_tuples;
 	_bt_relbuf(info->index, metabuf);
 
-	/*
-	 * If the underlying table has received a sufficiently high number of
-	 * insertions since the last VACUUM operation that called btvacuumscan(),
-	 * then have the current VACUUM operation call btvacuumscan() now.  This
-	 * happens when the statistics are deemed stale.
-	 *
-	 * XXX: We should have a more principled way of determining what
-	 * "staleness" means. The  vacuum_cleanup_index_scale_factor GUC (and the
-	 * index-level storage param) seem hard to tune in a principled way.
-	 */
-	relopts = (BTOptions *) info->index->rd_options;
-	cleanup_scale_factor = (relopts &&
-							relopts->vacuum_cleanup_index_scale_factor >= 0)
-		? relopts->vacuum_cleanup_index_scale_factor
-		: vacuum_cleanup_index_scale_factor;
-
-	if (cleanup_scale_factor <= 0 ||
-		info->num_heap_tuples < 0 ||
-		prev_num_heap_tuples <= 0 ||
-		(info->num_heap_tuples - prev_num_heap_tuples) /
-		prev_num_heap_tuples >= cleanup_scale_factor)
-		return true;
-
 	/*
 	 * Trigger cleanup in rare cases where prev_num_delpages exceeds 5% of the
 	 * total size of the index.  We can reasonably expect (though are not
@@ -925,48 +898,46 @@ btvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
 
 		/*
 		 * Since we aren't going to actually delete any leaf items, there's no
-		 * need to go through all the vacuum-cycle-ID pushups here
+		 * need to go through all the vacuum-cycle-ID pushups here.
+		 *
+		 * Posting list tuples are a source of inaccuracy for cleanup-only
+		 * scans.  btvacuumscan() will assume that the number of index tuples
+		 * from each page can be used as num_index_tuples, even though
+		 * num_index_tuples is supposed to represent the number of TIDs in the
+		 * index.  This naive approach can underestimate the number of tuples
+		 * in the index significantly.
+		 *
+		 * We handle the problem by making num_index_tuples an estimate in
+		 * cleanup-only case.
 		 */
 		stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
+		stats->estimated_count = true;
 		btvacuumscan(info, stats, NULL, NULL, 0);
 	}
 
 	/*
 	 * By here, we know for sure that this VACUUM operation won't be skipping
-	 * its btvacuumscan() call.  Maintain the count of the current number of
-	 * heap tuples in the metapage.  Also maintain the num_delpages value.
-	 * This information will be used by _bt_vacuum_needs_cleanup() during
-	 * future VACUUM operations that don't need to call btbulkdelete().
+	 * its btvacuumscan() call.  Maintain  the num_delpages value.  This
+	 * information will be used by _bt_vacuum_needs_cleanup() during future
+	 * VACUUM operations that don't need to call btbulkdelete().
 	 *
 	 * num_delpages is the number of deleted pages now in the index that were
 	 * not safe to place in the FSM to be recycled just yet.  We expect that
 	 * it will almost certainly be possible to place all of these pages in the
-	 * FSM during the next VACUUM operation.  That factor alone might cause
-	 * _bt_vacuum_needs_cleanup() to force the next VACUUM to proceed with a
-	 * btvacuumscan() call.
-	 *
-	 * Note: We must delay the _bt_set_cleanup_info() call until this late
-	 * stage of VACUUM (the btvacuumcleanup() phase), to keep num_heap_tuples
-	 * accurate.  The btbulkdelete()-time num_heap_tuples value is generally
-	 * just pg_class.reltuples for the heap relation _before_ VACUUM began.
-	 * In general cleanup info should describe the state of the index/table
-	 * _after_ VACUUM finishes.
+	 * FSM during the next VACUUM operation.  _bt_vacuum_needs_cleanup() will
+	 * force the next VACUUM to consider this before allowing btvacuumscan()
+	 * to be skipped entirely.  This should be rare -- cleanup-only VACUUMs
+	 * almost always manage to skip btvacuumscan() in practice.
 	 */
 	Assert(stats->pages_deleted >= stats->pages_free);
 	num_delpages = stats->pages_deleted - stats->pages_free;
-	_bt_set_cleanup_info(info->index, num_delpages, info->num_heap_tuples);
+	_bt_set_cleanup_info(info->index, num_delpages);
 
 	/*
 	 * It's quite possible for us to be fooled by concurrent page splits into
 	 * double-counting some index tuples, so disbelieve any total that exceeds
 	 * the underlying heap's count ... if we know that accurately.  Otherwise
 	 * this might just make matters worse.
-	 *
-	 * Posting list tuples are another source of inaccuracy.  Cleanup-only
-	 * btvacuumscan calls assume that the number of index tuples can be used
-	 * as num_index_tuples, even though num_index_tuples is supposed to
-	 * represent the number of TIDs in the index.  This naive approach can
-	 * underestimate the number of tuples in the index.
 	 */
 	if (!info->estimated_count)
 	{
@@ -1016,7 +987,6 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 * pages in the index at the end of the VACUUM command.)
 	 */
 	stats->num_pages = 0;
-	stats->estimated_count = false;
 	stats->num_index_tuples = 0;
 	stats->pages_deleted = 0;
 	stats->pages_free = 0;
@@ -1421,7 +1391,10 @@ backtrack:
 		 * We don't count the number of live TIDs during cleanup-only calls to
 		 * btvacuumscan (i.e. when callback is not set).  We count the number
 		 * of index tuples directly instead.  This avoids the expense of
-		 * directly examining all of the tuples on each page.
+		 * directly examining all of the tuples on each page.  VACUUM will
+		 * treat num_index_tuples as an estimate in cleanup-only case, so it
+		 * doesn't matter that this underestimates num_index_tuples
+		 * significantly in some cases.
 		 */
 		if (minoff > maxoff)
 			attempt_pagedel = (blkno == scanblkno);
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index d524310723..fdbe0da472 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2105,8 +2105,6 @@ btoptions(Datum reloptions, bool validate)
 {
 	static const relopt_parse_elt tab[] = {
 		{"fillfactor", RELOPT_TYPE_INT, offsetof(BTOptions, fillfactor)},
-		{"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(BTOptions, vacuum_cleanup_index_scale_factor)},
 		{"deduplicate_items", RELOPT_TYPE_BOOL,
 		offsetof(BTOptions, deduplicate_items)}
 
diff --git a/src/backend/access/nbtree/nbtxlog.c b/src/backend/access/nbtree/nbtxlog.c
index 990f5d0f52..1779b6ba47 100644
--- a/src/backend/access/nbtree/nbtxlog.c
+++ b/src/backend/access/nbtree/nbtxlog.c
@@ -113,7 +113,7 @@ _bt_restore_meta(XLogReaderState *record, uint8 block_id)
 	/* Cannot log BTREE_MIN_VERSION index metapage without upgrade */
 	Assert(md->btm_version >= BTREE_NOVAC_VERSION);
 	md->btm_last_cleanup_num_delpages = xlrec->last_cleanup_num_delpages;
-	md->btm_last_cleanup_num_heap_tuples = xlrec->last_cleanup_num_heap_tuples;
+	md->btm_last_cleanup_num_heap_tuples = -1.0;
 	md->btm_allequalimage = xlrec->allequalimage;
 
 	pageop = (BTPageOpaque) PageGetSpecialPointer(metapg);
diff --git a/src/backend/access/rmgrdesc/nbtdesc.c b/src/backend/access/rmgrdesc/nbtdesc.c
index f7cc4dd3e6..710efbd36a 100644
--- a/src/backend/access/rmgrdesc/nbtdesc.c
+++ b/src/backend/access/rmgrdesc/nbtdesc.c
@@ -113,9 +113,8 @@ btree_desc(StringInfo buf, XLogReaderState *record)
 
 				xlrec = (xl_btree_metadata *) XLogRecGetBlockData(record, 0,
 																  NULL);
-				appendStringInfo(buf, "last_cleanup_num_delpages %u; last_cleanup_num_heap_tuples: %f",
-								 xlrec->last_cleanup_num_delpages,
-								 xlrec->last_cleanup_num_heap_tuples);
+				appendStringInfo(buf, "last_cleanup_num_delpages %u",
+								 xlrec->last_cleanup_num_delpages);
 				break;
 			}
 	}
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index a5976ad5b1..73e0a672ae 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -148,5 +148,3 @@ int64		VacuumPageDirty = 0;
 
 int			VacuumCostBalance = 0;	/* working state for vacuum */
 bool		VacuumCostActive = false;
-
-double		vacuum_cleanup_index_scale_factor;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 3fd1a5fbe2..df199851cc 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3694,16 +3694,6 @@ static struct config_real ConfigureNamesReal[] =
 		NULL, NULL, NULL
 	},
 
-	{
-		{"vacuum_cleanup_index_scale_factor", PGC_USERSET, CLIENT_CONN_STATEMENT,
-			gettext_noop("Number of tuple inserts prior to index cleanup as a fraction of reltuples."),
-			NULL
-		},
-		&vacuum_cleanup_index_scale_factor,
-		0.1, 0.0, 1e10,
-		NULL, NULL, NULL
-	},
-
 	{
 		{"log_statement_sample_rate", PGC_SUSET, LOGGING_WHEN,
 			gettext_noop("Fraction of statements exceeding log_min_duration_sample to be logged."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index ee06528bb0..3736c972a8 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -671,9 +671,6 @@
 #vacuum_freeze_table_age = 150000000
 #vacuum_multixact_freeze_min_age = 5000000
 #vacuum_multixact_freeze_table_age = 150000000
-#vacuum_cleanup_index_scale_factor = 0.1	# fraction of total number of tuples
-						# before index cleanup, 0 always performs
-						# index cleanup
 #bytea_output = 'hex'			# hex, escape
 #xmlbinary = 'base64'
 #xmloption = 'content'
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9f0208ac49..ecdb8d752b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1789,14 +1789,14 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER INDEX <foo> SET|RESET ( */
 	else if (Matches("ALTER", "INDEX", MatchAny, "RESET", "("))
 		COMPLETE_WITH("fillfactor",
-					  "vacuum_cleanup_index_scale_factor", "deduplicate_items", /* BTREE */
+					  "deduplicate_items", /* BTREE */
 					  "fastupdate", "gin_pending_list_limit",	/* GIN */
 					  "buffering",	/* GiST */
 					  "pages_per_range", "autosummarize"	/* BRIN */
 			);
 	else if (Matches("ALTER", "INDEX", MatchAny, "SET", "("))
 		COMPLETE_WITH("fillfactor =",
-					  "vacuum_cleanup_index_scale_factor =", "deduplicate_items =", /* BTREE */
+					  "deduplicate_items =", /* BTREE */
 					  "fastupdate =", "gin_pending_list_limit =",	/* GIN */
 					  "buffering =",	/* GiST */
 					  "pages_per_range =", "autosummarize ="	/* BRIN */
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 967de73596..81e597a80b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8523,46 +8523,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-vacuum-cleanup-index-scale-factor" xreflabel="vacuum_cleanup_index_scale_factor">
-      <term><varname>vacuum_cleanup_index_scale_factor</varname> (<type>floating point</type>)
-      <indexterm>
-       <primary><varname>vacuum_cleanup_index_scale_factor</varname></primary>
-       <secondary>configuration parameter</secondary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Specifies the fraction of the total number of heap tuples counted in
-        the previous statistics collection that can be inserted without
-        incurring an index scan at the <command>VACUUM</command> cleanup stage.
-        This setting currently applies to B-tree indexes only.
-       </para>
-
-       <para>
-        If no tuples were deleted from the heap, B-tree indexes are still
-        scanned at the <command>VACUUM</command> cleanup stage when the
-        index's statistics are stale.  Index statistics are considered
-        stale if the number of newly inserted tuples exceeds the
-        <varname>vacuum_cleanup_index_scale_factor</varname>
-        fraction of the total number of heap tuples detected by the previous
-        statistics collection. The total number of heap tuples is stored in
-        the index meta-page. Note that the meta-page does not include this data
-        until <command>VACUUM</command> finds no dead tuples, so B-tree index
-        scan at the cleanup stage can only be skipped if the second and
-        subsequent <command>VACUUM</command> cycles detect no dead tuples.
-       </para>
-
-       <para>
-        The value can range from <literal>0</literal> to
-        <literal>10000000000</literal>.
-        When <varname>vacuum_cleanup_index_scale_factor</varname> is set to
-        <literal>0</literal>, index scans are never skipped during
-        <command>VACUUM</command> cleanup. The default value is <literal>0.1</literal>.
-       </para>
-
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-bytea-output" xreflabel="bytea_output">
       <term><varname>bytea_output</varname> (<type>enum</type>)
       <indexterm>
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 51b4d57939..bc57adf7d5 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -456,20 +456,6 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
     </note>
     </listitem>
    </varlistentry>
-
-   <varlistentry id="index-reloption-vacuum-cleanup-index-scale-factor" xreflabel="vacuum_cleanup_index_scale_factor">
-    <term><literal>vacuum_cleanup_index_scale_factor</literal> (<type>floating point</type>)
-     <indexterm>
-      <primary><varname>vacuum_cleanup_index_scale_factor</varname></primary>
-      <secondary>storage parameter</secondary>
-     </indexterm>
-    </term>
-    <listitem>
-    <para>
-      Per-index value for <xref linkend="guc-vacuum-cleanup-index-scale-factor"/>.
-    </para>
-    </listitem>
-   </varlistentry>
    </variablelist>
 
    <para>
diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out
index cfd4338e36..bc113a70b4 100644
--- a/src/test/regress/expected/btree_index.out
+++ b/src/test/regress/expected/btree_index.out
@@ -308,35 +308,6 @@ alter table btree_tall_tbl alter COLUMN t set storage plain;
 create index btree_tall_idx on btree_tall_tbl (t, id) with (fillfactor = 10);
 insert into btree_tall_tbl select g, repeat('x', 250)
 from generate_series(1, 130) g;
---
--- Test vacuum_cleanup_index_scale_factor
---
--- Simple create
-create table btree_test(a int);
-create index btree_idx1 on btree_test(a) with (vacuum_cleanup_index_scale_factor = 40.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-                reloptions                
-------------------------------------------
- {vacuum_cleanup_index_scale_factor=40.0}
-(1 row)
-
--- Fail while setting improper values
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = -10.0);
-ERROR:  value -10.0 out of bounds for option "vacuum_cleanup_index_scale_factor"
-DETAIL:  Valid values are between "0.000000" and "10000000000.000000".
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 100.0);
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 'string');
-ERROR:  invalid value for floating point option "vacuum_cleanup_index_scale_factor": string
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = true);
-ERROR:  invalid value for floating point option "vacuum_cleanup_index_scale_factor": true
--- Simple ALTER INDEX
-alter index btree_idx1 set (vacuum_cleanup_index_scale_factor = 70.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-                reloptions                
-------------------------------------------
- {vacuum_cleanup_index_scale_factor=70.0}
-(1 row)
-
 --
 -- Test for multilevel page deletion
 --
diff --git a/src/test/regress/sql/btree_index.sql b/src/test/regress/sql/btree_index.sql
index 96f53818ff..c60312db2d 100644
--- a/src/test/regress/sql/btree_index.sql
+++ b/src/test/regress/sql/btree_index.sql
@@ -150,25 +150,6 @@ create index btree_tall_idx on btree_tall_tbl (t, id) with (fillfactor = 10);
 insert into btree_tall_tbl select g, repeat('x', 250)
 from generate_series(1, 130) g;
 
---
--- Test vacuum_cleanup_index_scale_factor
---
-
--- Simple create
-create table btree_test(a int);
-create index btree_idx1 on btree_test(a) with (vacuum_cleanup_index_scale_factor = 40.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-
--- Fail while setting improper values
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = -10.0);
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 100.0);
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 'string');
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = true);
-
--- Simple ALTER INDEX
-alter index btree_idx1 set (vacuum_cleanup_index_scale_factor = 70.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-
 --
 -- Test for multilevel page deletion
 --
-- 
2.27.0

Masahiko Sawada

sawada.mshk@gmail.com

almost 5 years ago

In reply to: Peter Geoghegan (#5)

Re: Removing vacuum_cleanup_index_scale_factor

On Tue, Mar 9, 2021 at 7:35 AM Peter Geoghegan <pg@bowt.ie> wrote:

On Mon, Mar 8, 2021 at 1:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

As you say, the history here is a bit convoluted, but it seems like
a good principle to avoid interconnections between VACUUM and ANALYZE
as much as we can. I haven't been paying enough attention to this
thread to have more insight than that.

The attached patch does what I proposed earlier today: it teaches
do_analyze_rel() to always set pg_class.reltuples for indexes when it
would do the same thing for the heap/table relation already. It's now
uniform in that sense.

Thank you for the patches. I looked at 0001 patch and have a comment:

+    * We don't report to the stats collector here because the stats collector
+    * only tracks per-table stats.  Reset the changes_since_analyze counter
+    * only if we analyzed all columns; otherwise, there is still work for
+    * auto-analyze to do.

I think the comment becomes clearer if we add "if doing inherited
stats" at top of the above paragraph since we actually report to the
stats collector in !inh case.

Also included is a patch that removes the
vacuum_cleanup_index_scale_factor mechanism for triggering an index
scan during VACUUM -- that's what the second patch does (this depends
on the first patch, really).

0002 patch looks good to me.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

Peter Geoghegan

pg@bowt.ie

almost 5 years ago

In reply to: Masahiko Sawada (#6)

4 attachment(s)

Re: Removing vacuum_cleanup_index_scale_factor

On Mon, Mar 8, 2021 at 10:21 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Thank you for the patches. I looked at 0001 patch and have a comment:
+    * We don't report to the stats collector here because the stats collector
+    * only tracks per-table stats.  Reset the changes_since_analyze counter
+    * only if we analyzed all columns; otherwise, there is still work for
+    * auto-analyze to do.
I think the comment becomes clearer if we add "if doing inherited
stats" at top of the above paragraph since we actually report to the
stats collector in !inh case.

I messed the comment up. Oops. Fixed now.

Also included is a patch that removes the
vacuum_cleanup_index_scale_factor mechanism for triggering an index
scan during VACUUM -- that's what the second patch does (this depends
on the first patch, really).

0002 patch looks good to me.

Great.

Attached revision has a bit more polish. It includes new commit
messages which explains what we're really trying to fix here. I also
included backpatchable versions for Postgres 13 -- that's the other
significant change compared to the last version.

My current plan is to commit everything within the next day or two.
This includes backpatching to Postgres 13 only. I am now leaning
against doing anything in Postgres 11 and 12, for the closely related
btm_last_cleanup_num_heap_tuples VACUUM accuracy issue. There have
been no complaints from users using Postgres 11 or 12, so I'll leave
them alone. (Sorry for changing my mind again and again.)

To be clear: I plan on disabling (though not removing) the
vacuum_cleanup_index_scale_factor GUC and storage parameter on
Postgres 13, even though that is a stable release. This approach is
unorthodox, but it has a kind of a precedent -- the recheck_on_update
storage param was disabled on the Postgres 11 branch by commit
5d28c9bd. More importantly, it just happens to make sense, given the
specifics here.

--
Peter Geoghegan

Attachments:

v2-0002-VACUUM-ANALYZE-Always-update-pg_class.reltuples.patchapplication/octet-stream; name=v2-0002-VACUUM-ANALYZE-Always-update-pg_class.reltuples.patchDownload

From 0603ab5d37a02e05d7a30fbd806eadd17a6232e4 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Tue, 9 Mar 2021 12:58:39 -0800
Subject: [PATCH v2 2/2] VACUUM ANALYZE: Always update pg_class.reltuples.

Stop assuming that pg_class must have been updated with accurate
statistics within VACUUM ANALYZE -- update pg_class for indexes at the
same time as the table relation in all cases.

VACUUM sometimes gives less accurate statistics than ANALYZE by design,
per the contract for amvacuumcleanup() routines established by commit
e57345975cf back in 2006.  It seems advisable to effectively assume that
that always happens.

The worst possible consequence of this approach is that it might
inaccurately set pg_class.reltuples for indexes whose heap relation ends
up with the same inaccurate value anyway, which doesn't seem too bad.
We already consistently called vac_update_relstats() (to update
pg_class) for the heap/table relation twice during any VACUUM ANALYZE --
once in vacuumlazy.c, and once in analyze.c.  We now make sure that we
call vac_update_relstats() at least once (though often twice) for each
index.

This is follow up work to the recent commit that dealt with issues with
btvacuumcleanup().  Though this is technically a long-standing issue
(e.g., hashvacuumcleanup() is just as affected as btvacuumcleanup()), it
seems like a good idea to fix both issues together now.  Having VACUUM
ANALYZE understand that pg_class.reltuples might not have been set in
vacuumlazy.c by the time do_analyze_rel() is reached probably matters
much more in light of the btvacuumcleanup() changes.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAH2-WzknxdComjhqo4SUxVFk_Q1171GJO2ZgHZ1Y6pion6u8rA@mail.gmail.com
Backpatch: 13-, just like the last commit.
---
 src/backend/access/heap/vacuumlazy.c |  3 +-
 src/backend/commands/analyze.c       | 52 +++++++++++++++++-----------
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d8f847b0e6..22ca7f5c54 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1737,7 +1737,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..eb32edd201 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -600,8 +600,16 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 								 PROGRESS_ANALYZE_PHASE_FINALIZE_ANALYZE);
 
 	/*
-	 * Update pages/tuples stats in pg_class ... but not if we're doing
-	 * inherited stats.
+	 * Update pages/tuples stats in pg_class, and report ANALYZE to the stats
+	 * collector ... but not if we're doing inherited stats.
+	 *
+	 * We assume that VACUUM hasn't set pg_class.reltuples already, even
+	 * during a VACUUM ANALYZE.  Although VACUUM often updates pg_class,
+	 * exceptions exists.  A "VACUUM (ANALYZE, INDEX_CLEANUP OFF)" command
+	 * will never update pg_class entries for index relations.  It's also
+	 * possible that an individual index's pg_class entry won't be updated
+	 * during VACUUM if the index AM returns NULL from its amvacuumcleanup()
+	 * routine.
 	 */
 	if (!inh)
 	{
@@ -609,6 +617,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 
 		visibilitymap_count(onerel, &relallvisible, NULL);
 
+		/* Update pg_class for table relation */
 		vac_update_relstats(onerel,
 							relpages,
 							totalrows,
@@ -617,15 +626,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 							InvalidTransactionId,
 							InvalidMultiXactId,
 							in_outer_xact);
-	}
 
-	/*
-	 * 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.
-	 */
-	if (!inh && !(params->options & VACOPT_VACUUM))
-	{
+		/* Same for indexes */
 		for (ind = 0; ind < nindexes; ind++)
 		{
 			AnlIndexData *thisdata = &indexdata[ind];
@@ -641,20 +643,30 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 								InvalidMultiXactId,
 								in_outer_xact);
 		}
+
+		/*
+		 * Now report ANALYZE to the stats collector.
+		 *
+		 * We deliberately don't report to the stats collector when doing
+		 * inherited stats. because the stats collector only tracks per-table
+		 * stats.
+		 *
+		 * Reset the changes_since_analyze counter only if we analyzed all
+		 * columns; otherwise, there is still work for auto-analyze to do.
+		 */
+		pgstat_report_analyze(onerel, totalrows, totaldeadrows,
+							  (va_cols == NIL));
 	}
 
 	/*
-	 * Report ANALYZE to the stats collector, too.  However, if doing
-	 * inherited stats we shouldn't report, because the stats collector only
-	 * tracks per-table stats.  Reset the changes_since_analyze counter only
-	 * if we analyzed all columns; otherwise, there is still work for
-	 * auto-analyze to do.
+	 * 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.  In practice only
+	 * ginvacuumcleanup() currently does real work when called through here
+	 * (and even GIN makes it a no-op unless we're running is an autovacuum
+	 * worker).
 	 */
-	if (!inh)
-		pgstat_report_analyze(onerel, totalrows, totaldeadrows,
-							  (va_cols == NIL));
-
-	/* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */
 	if (!(params->options & VACOPT_VACUUM))
 	{
 		for (ind = 0; ind < nindexes; ind++)
-- 
2.27.0

REL_13_STABLE-v2-0002-VACUUM-ANALYZE-Always-update-pg_class.reltuples.patchapplication/octet-stream; name=REL_13_STABLE-v2-0002-VACUUM-ANALYZE-Always-update-pg_class.reltuples.patchDownload

From 8297a4fd546aa24da68e0a94677b8862feec1634 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Tue, 9 Mar 2021 19:20:51 -0800
Subject: [PATCH v2 2/2] VACUUM ANALYZE: Always update pg_class.reltuples.

Stop assuming that pg_class must have been updated with accurate
statistics within VACUUM ANALYZE -- update pg_class for indexes at the
same time as the table relation in all cases.

VACUUM sometimes gives less accurate statistics than ANALYZE by design,
per the contract for amvacuumcleanup() routines established by commit
e57345975cf back in 2006.  It seems advisable to effectively assume that
that always happens.

The worst possible consequence of this approach is that it might
inaccurately set pg_class.reltuples for indexes whose heap relation ends
up with the same inaccurate value anyway, which doesn't seem too bad.
We already consistently called vac_update_relstats() (to update
pg_class) for the heap/table relation twice during any VACUUM ANALYZE --
once in vacuumlazy.c, and once in analyze.c.  We now make sure that we
call vac_update_relstats() at least once (though often twice) for each
index.

This is follow up work to the recent commit that dealt with issues with
btvacuumcleanup().  Though this is technically a long-standing issue
(e.g., hashvacuumcleanup() is just as affected as btvacuumcleanup()), it
seems like a good idea to fix both issues together now.  Having VACUUM
ANALYZE understand that pg_class.reltuples might not have been set in
vacuumlazy.c by the time do_analyze_rel() is reached probably matters
much more in light of the btvacuumcleanup() changes.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAH2-WzknxdComjhqo4SUxVFk_Q1171GJO2ZgHZ1Y6pion6u8rA@mail.gmail.com
Backpatch: 13-, just like the last commit.
---
 src/backend/access/heap/vacuumlazy.c |  3 +-
 src/backend/commands/analyze.c       | 52 +++++++++++++++++-----------
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 1dfc396c4f..d6a8cf390c 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1717,7 +1717,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 924ef37c81..eef15a9551 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -611,8 +611,16 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 								 PROGRESS_ANALYZE_PHASE_FINALIZE_ANALYZE);
 
 	/*
-	 * Update pages/tuples stats in pg_class ... but not if we're doing
-	 * inherited stats.
+	 * Update pages/tuples stats in pg_class, and report ANALYZE to the stats
+	 * collector ... but not if we're doing inherited stats.
+	 *
+	 * We assume that VACUUM hasn't set pg_class.reltuples already, even
+	 * during a VACUUM ANALYZE.  Although VACUUM often updates pg_class,
+	 * exceptions exists.  A "VACUUM (ANALYZE, INDEX_CLEANUP OFF)" command
+	 * will never update pg_class entries for index relations.  It's also
+	 * possible that an individual index's pg_class entry won't be updated
+	 * during VACUUM if the index AM returns NULL from its amvacuumcleanup()
+	 * routine.
 	 */
 	if (!inh)
 	{
@@ -620,6 +628,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 
 		visibilitymap_count(onerel, &relallvisible, NULL);
 
+		/* Update pg_class for table relation */
 		vac_update_relstats(onerel,
 							relpages,
 							totalrows,
@@ -628,15 +637,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 							InvalidTransactionId,
 							InvalidMultiXactId,
 							in_outer_xact);
-	}
 
-	/*
-	 * 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.
-	 */
-	if (!inh && !(params->options & VACOPT_VACUUM))
-	{
+		/* Same for indexes */
 		for (ind = 0; ind < nindexes; ind++)
 		{
 			AnlIndexData *thisdata = &indexdata[ind];
@@ -652,20 +654,30 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 								InvalidMultiXactId,
 								in_outer_xact);
 		}
+
+		/*
+		 * Now report ANALYZE to the stats collector.
+		 *
+		 * We deliberately don't report to the stats collector when doing
+		 * inherited stats. because the stats collector only tracks per-table
+		 * stats.
+		 *
+		 * Reset the changes_since_analyze counter only if we analyzed all
+		 * columns; otherwise, there is still work for auto-analyze to do.
+		 */
+		pgstat_report_analyze(onerel, totalrows, totaldeadrows,
+							  (va_cols == NIL));
 	}
 
 	/*
-	 * Report ANALYZE to the stats collector, too.  However, if doing
-	 * inherited stats we shouldn't report, because the stats collector only
-	 * tracks per-table stats.  Reset the changes_since_analyze counter only
-	 * if we analyzed all columns; otherwise, there is still work for
-	 * auto-analyze to do.
+	 * 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.  In practice only
+	 * ginvacuumcleanup() currently does real work when called through here
+	 * (and even GIN makes it a no-op unless we're running is an autovacuum
+	 * worker).
 	 */
-	if (!inh)
-		pgstat_report_analyze(onerel, totalrows, totaldeadrows,
-							  (va_cols == NIL));
-
-	/* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */
 	if (!(params->options & VACOPT_VACUUM))
 	{
 		for (ind = 0; ind < nindexes; ind++)
-- 
2.27.0

v2-0001-Don-t-consider-newly-inserted-tuples-in-nbtree-VA.patchapplication/octet-stream; name=v2-0001-Don-t-consider-newly-inserted-tuples-in-nbtree-VA.patchDownload

From cca81d53f95706ed4af3c05795b8cf15ce3492f4 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Tue, 9 Mar 2021 12:58:39 -0800
Subject: [PATCH v2 1/2] Don't consider newly inserted tuples in nbtree VACUUM.

Skip full index scan during a VACUUM for nbtree indexes in the case
where VACUUM never called btbulkdelete(), even when pg_class stats for
the index relation would be considered "stale" by criteria applied using
vacuum_cleanup_index_scale_factor.  Rely on ANALYZE instead -- it can be
relied on to keep pg_class.reltuples up to date, per the amvacuumcleanup
contract.  Also remove the vacuum_cleanup_index_scale_factor GUC/param
in passing (though just disable it on the Postgres 13 branch).

VACUUM will still do scans of the index despite never reaching
btbulkdelete() in one remaining case:  the case where a previous VACUUM
operation is known to have performed index page deletion of pages that
have yet be placed in the free space map for recycling.

Backpatch to Postgres 13 due to an unanticipated interaction with the
autovacuum_vacuum_insert_threshold feature added by commit b07642db and
the "skip full scan" feature added by commit 857f9c36.  This interaction
has been tied to a regression with an append-only insert benchmark [1].
It is reasonable to expect a certain amount of overhead from vacuuming
that just sets visibility map bits, but it does not seem reasonable to
perform a full index scans in btvacuumcleanup() purely to set the
pg_class.reltuples stats in affected indexes.

There is another reason to backpatch: a bugfix commit tied to the nbtree
deduplication feature (bugfix commit 48e12913) taught nbtree VACUUM to
track IndexBulkDeleteResult.num_index_tuples using an inherently
approximate approach.  This made sense -- getting an accurate count in
the presence of posting list tuples just isn't worth the cycles for a
btvacuumcleanup()-only VACUUM.  But btvacuumcleanup() should still
indicate that its final num_index_tuples value is just an estimate when
its approximate approach to tracking live tuples gets used.

Have btvacuumcleanup() acknowledge that its approach to counting is
approximate by setting IndexBulkDeleteResult.estimated_count to 'true'.
This prevents vacuumlazy.c from setting the index's pg_class.reltuples
to a value that significantly underestimates the number of live tuples
(at least in certain scenarios with large posting list tuples).  This is
the same approach that hashvacuumcleanup() has always taken.  Index AMs
have had the option of giving only approximate num_index_tuples
statistics since commit e57345975cf, which updated the relevant index AM
API.

[1] https://smalldatum.blogspot.com/2021/01/insert-benchmark-postgres-is-still.html

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoA4WHthN5uU6+WScZ7+J_RcEjmcuH94qcoUPuB42ShXzg@mail.gmail.com
Backpatch: 13-, where autovacuum_vacuum_insert_threshold was added.
---
 src/include/access/nbtree.h                   |  5 +-
 src/include/access/nbtxlog.h                  |  1 -
 src/include/access/xlog_internal.h            |  2 +-
 src/include/miscadmin.h                       |  2 -
 src/backend/access/common/reloptions.c        |  9 ---
 src/backend/access/nbtree/nbtinsert.c         |  3 -
 src/backend/access/nbtree/nbtpage.c           | 40 ++++------
 src/backend/access/nbtree/nbtree.c            | 75 ++++++-------------
 src/backend/access/nbtree/nbtutils.c          |  2 -
 src/backend/access/nbtree/nbtxlog.c           |  2 +-
 src/backend/access/rmgrdesc/nbtdesc.c         |  5 +-
 src/backend/utils/init/globals.c              |  2 -
 src/backend/utils/misc/guc.c                  | 10 ---
 src/backend/utils/misc/postgresql.conf.sample |  3 -
 src/bin/psql/tab-complete.c                   |  4 +-
 doc/src/sgml/config.sgml                      | 40 ----------
 doc/src/sgml/ref/create_index.sgml            | 14 ----
 src/test/regress/expected/btree_index.out     | 29 -------
 src/test/regress/sql/btree_index.sql          | 19 -----
 19 files changed, 44 insertions(+), 223 deletions(-)

diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index b56b7b7868..c9d6000cbe 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -1067,8 +1067,6 @@ typedef struct BTOptions
 {
 	int32		varlena_header_;	/* varlena header (do not touch directly!) */
 	int			fillfactor;		/* page fill factor in percent (0..100) */
-	/* fraction of newly inserted tuples needed to trigger index cleanup */
-	float8		vacuum_cleanup_index_scale_factor;
 	bool		deduplicate_items;	/* Try to deduplicate items? */
 } BTOptions;
 
@@ -1171,8 +1169,7 @@ extern OffsetNumber _bt_findsplitloc(Relation rel, Page origpage,
  */
 extern void _bt_initmetapage(Page page, BlockNumber rootbknum, uint32 level,
 							 bool allequalimage);
-extern void _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
-								 float8 num_heap_tuples);
+extern void _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages);
 extern void _bt_upgrademetapage(Page page);
 extern Buffer _bt_getroot(Relation rel, int access);
 extern Buffer _bt_gettrueroot(Relation rel);
diff --git a/src/include/access/nbtxlog.h b/src/include/access/nbtxlog.h
index 3df34fcda2..0f7731856b 100644
--- a/src/include/access/nbtxlog.h
+++ b/src/include/access/nbtxlog.h
@@ -54,7 +54,6 @@ typedef struct xl_btree_metadata
 	BlockNumber fastroot;
 	uint32		fastlevel;
 	uint32		last_cleanup_num_delpages;
-	float8		last_cleanup_num_heap_tuples;
 	bool		allequalimage;
 } xl_btree_metadata;
 
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index 8d09eaec93..b23e286406 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -31,7 +31,7 @@
 /*
  * Each page of XLOG file has a header like this:
  */
-#define XLOG_PAGE_MAGIC 0xD10A	/* can be used as WAL version indicator */
+#define XLOG_PAGE_MAGIC 0xD10B	/* can be used as WAL version indicator */
 
 typedef struct XLogPageHeaderData
 {
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 1bdc97e308..54693e047a 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -261,8 +261,6 @@ extern int64 VacuumPageDirty;
 extern int	VacuumCostBalance;
 extern bool VacuumCostActive;
 
-extern double vacuum_cleanup_index_scale_factor;
-
 
 /* in tcop/postgres.c */
 
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index c687d3ee9e..433e236722 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -461,15 +461,6 @@ static relopt_real realRelOpts[] =
 		},
 		0, -1.0, DBL_MAX
 	},
-	{
-		{
-			"vacuum_cleanup_index_scale_factor",
-			"Number of tuple inserts prior to index cleanup as a fraction of reltuples.",
-			RELOPT_KIND_BTREE,
-			ShareUpdateExclusiveLock
-		},
-		-1, 0.0, 1e10
-	},
 	/* list terminator */
 	{{NULL}}
 };
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 1edb9f9579..0bc86943eb 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -1332,8 +1332,6 @@ _bt_insertonpg(Relation rel,
 					xlmeta.fastroot = metad->btm_fastroot;
 					xlmeta.fastlevel = metad->btm_fastlevel;
 					xlmeta.last_cleanup_num_delpages = metad->btm_last_cleanup_num_delpages;
-					xlmeta.last_cleanup_num_heap_tuples =
-						metad->btm_last_cleanup_num_heap_tuples;
 					xlmeta.allequalimage = metad->btm_allequalimage;
 
 					XLogRegisterBuffer(2, metabuf,
@@ -2549,7 +2547,6 @@ _bt_newroot(Relation rel, Buffer lbuf, Buffer rbuf)
 		md.fastroot = rootblknum;
 		md.fastlevel = metad->btm_level;
 		md.last_cleanup_num_delpages = metad->btm_last_cleanup_num_delpages;
-		md.last_cleanup_num_heap_tuples = metad->btm_last_cleanup_num_heap_tuples;
 		md.allequalimage = metad->btm_allequalimage;
 
 		XLogRegisterBufData(2, (char *) &md, sizeof(xl_btree_metadata));
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index c09e492a5f..4a0578dff4 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -175,26 +175,15 @@ _bt_getmeta(Relation rel, Buffer metabuf)
  *		_bt_vacuum_needs_cleanup() to decide whether or not a btvacuumscan()
  *		call should go ahead for an entire VACUUM operation.
  *
- *		See btvacuumcleanup() and _bt_vacuum_needs_cleanup() for details of
- *		the two fields that we maintain here.
- *
- *		The information that we maintain for btvacuumcleanup() describes the
- *		state of the index (as well as the table it indexes) just _after_ the
- *		ongoing VACUUM operation.  The next _bt_vacuum_needs_cleanup() call
- *		will consider the information we saved for it during the next VACUUM
- *		operation (assuming that there will be no btbulkdelete() call during
- *		the next VACUUM operation -- if there is then the question of skipping
- *		btvacuumscan() doesn't even arise).
+ *		See btvacuumcleanup() and _bt_vacuum_needs_cleanup() for the
+ *		definition of num_delpages.
  */
 void
-_bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
-					 float8 num_heap_tuples)
+_bt_set_cleanup_info(Relation rel, BlockNumber num_delpages)
 {
 	Buffer		metabuf;
 	Page		metapg;
 	BTMetaPageData *metad;
-	bool		rewrite = false;
-	XLogRecPtr	recptr;
 
 	/*
 	 * On-disk compatibility note: The btm_last_cleanup_num_delpages metapage
@@ -209,21 +198,20 @@ _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
 	 * in reality there are only one or two.  The worst that can happen is
 	 * that there will be a call to btvacuumscan a little earlier, which will
 	 * set btm_last_cleanup_num_delpages to a sane value when we're called.
+	 *
+	 * Note also that the metapage's btm_last_cleanup_num_heap_tuples field is
+	 * no longer used as of PostgreSQL 14.  We set it to -1.0 on rewrite, just
+	 * to be consistent.
 	 */
 	metabuf = _bt_getbuf(rel, BTREE_METAPAGE, BT_READ);
 	metapg = BufferGetPage(metabuf);
 	metad = BTPageGetMeta(metapg);
 
-	/* Always dynamically upgrade index/metapage when BTREE_MIN_VERSION */
-	if (metad->btm_version < BTREE_NOVAC_VERSION)
-		rewrite = true;
-	else if (metad->btm_last_cleanup_num_delpages != num_delpages)
-		rewrite = true;
-	else if (metad->btm_last_cleanup_num_heap_tuples != num_heap_tuples)
-		rewrite = true;
-
-	if (!rewrite)
+	/* Don't miss chance to upgrade index/metapage when BTREE_MIN_VERSION */
+	if (metad->btm_version >= BTREE_NOVAC_VERSION &&
+		metad->btm_last_cleanup_num_delpages == num_delpages)
 	{
+		/* Usually means index continues to have num_delpages of 0 */
 		_bt_relbuf(rel, metabuf);
 		return;
 	}
@@ -240,13 +228,14 @@ _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
 
 	/* update cleanup-related information */
 	metad->btm_last_cleanup_num_delpages = num_delpages;
-	metad->btm_last_cleanup_num_heap_tuples = num_heap_tuples;
+	metad->btm_last_cleanup_num_heap_tuples = -1.0;
 	MarkBufferDirty(metabuf);
 
 	/* write wal record if needed */
 	if (RelationNeedsWAL(rel))
 	{
 		xl_btree_metadata md;
+		XLogRecPtr	recptr;
 
 		XLogBeginInsert();
 		XLogRegisterBuffer(0, metabuf, REGBUF_WILL_INIT | REGBUF_STANDARD);
@@ -258,7 +247,6 @@ _bt_set_cleanup_info(Relation rel, BlockNumber num_delpages,
 		md.fastroot = metad->btm_fastroot;
 		md.fastlevel = metad->btm_fastlevel;
 		md.last_cleanup_num_delpages = num_delpages;
-		md.last_cleanup_num_heap_tuples = num_heap_tuples;
 		md.allequalimage = metad->btm_allequalimage;
 
 		XLogRegisterBufData(0, (char *) &md, sizeof(xl_btree_metadata));
@@ -443,7 +431,6 @@ _bt_getroot(Relation rel, int access)
 			md.fastroot = rootblkno;
 			md.fastlevel = 0;
 			md.last_cleanup_num_delpages = 0;
-			md.last_cleanup_num_heap_tuples = -1.0;
 			md.allequalimage = metad->btm_allequalimage;
 
 			XLogRegisterBufData(2, (char *) &md, sizeof(xl_btree_metadata));
@@ -2632,7 +2619,6 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno,
 			xlmeta.fastroot = metad->btm_fastroot;
 			xlmeta.fastlevel = metad->btm_fastlevel;
 			xlmeta.last_cleanup_num_delpages = metad->btm_last_cleanup_num_delpages;
-			xlmeta.last_cleanup_num_heap_tuples = metad->btm_last_cleanup_num_heap_tuples;
 			xlmeta.allequalimage = metad->btm_allequalimage;
 
 			XLogRegisterBufData(4, (char *) &xlmeta, sizeof(xl_btree_metadata));
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 504f5bef17..fefdf3f955 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -789,11 +789,8 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
 	Buffer		metabuf;
 	Page		metapg;
 	BTMetaPageData *metad;
-	BTOptions  *relopts;
-	float8		cleanup_scale_factor;
 	uint32		btm_version;
 	BlockNumber prev_num_delpages;
-	float8		prev_num_heap_tuples;
 
 	/*
 	 * Copy details from metapage to local variables quickly.
@@ -816,32 +813,8 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
 	}
 
 	prev_num_delpages = metad->btm_last_cleanup_num_delpages;
-	prev_num_heap_tuples = metad->btm_last_cleanup_num_heap_tuples;
 	_bt_relbuf(info->index, metabuf);
 
-	/*
-	 * If the underlying table has received a sufficiently high number of
-	 * insertions since the last VACUUM operation that called btvacuumscan(),
-	 * then have the current VACUUM operation call btvacuumscan() now.  This
-	 * happens when the statistics are deemed stale.
-	 *
-	 * XXX: We should have a more principled way of determining what
-	 * "staleness" means. The  vacuum_cleanup_index_scale_factor GUC (and the
-	 * index-level storage param) seem hard to tune in a principled way.
-	 */
-	relopts = (BTOptions *) info->index->rd_options;
-	cleanup_scale_factor = (relopts &&
-							relopts->vacuum_cleanup_index_scale_factor >= 0)
-		? relopts->vacuum_cleanup_index_scale_factor
-		: vacuum_cleanup_index_scale_factor;
-
-	if (cleanup_scale_factor <= 0 ||
-		info->num_heap_tuples < 0 ||
-		prev_num_heap_tuples <= 0 ||
-		(info->num_heap_tuples - prev_num_heap_tuples) /
-		prev_num_heap_tuples >= cleanup_scale_factor)
-		return true;
-
 	/*
 	 * Trigger cleanup in rare cases where prev_num_delpages exceeds 5% of the
 	 * total size of the index.  We can reasonably expect (though are not
@@ -925,48 +898,46 @@ btvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
 
 		/*
 		 * Since we aren't going to actually delete any leaf items, there's no
-		 * need to go through all the vacuum-cycle-ID pushups here
+		 * need to go through all the vacuum-cycle-ID pushups here.
+		 *
+		 * Posting list tuples are a source of inaccuracy for cleanup-only
+		 * scans.  btvacuumscan() will assume that the number of index tuples
+		 * from each page can be used as num_index_tuples, even though
+		 * num_index_tuples is supposed to represent the number of TIDs in the
+		 * index.  This naive approach can underestimate the number of tuples
+		 * in the index significantly.
+		 *
+		 * We handle the problem by making num_index_tuples an estimate in
+		 * cleanup-only case.
 		 */
 		stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
 		btvacuumscan(info, stats, NULL, NULL, 0);
+		stats->estimated_count = true;
 	}
 
 	/*
 	 * By here, we know for sure that this VACUUM operation won't be skipping
-	 * its btvacuumscan() call.  Maintain the count of the current number of
-	 * heap tuples in the metapage.  Also maintain the num_delpages value.
-	 * This information will be used by _bt_vacuum_needs_cleanup() during
-	 * future VACUUM operations that don't need to call btbulkdelete().
+	 * its btvacuumscan() call.  Maintain  the num_delpages value.  This
+	 * information will be used by _bt_vacuum_needs_cleanup() during future
+	 * VACUUM operations that don't need to call btbulkdelete().
 	 *
 	 * num_delpages is the number of deleted pages now in the index that were
 	 * not safe to place in the FSM to be recycled just yet.  We expect that
 	 * it will almost certainly be possible to place all of these pages in the
-	 * FSM during the next VACUUM operation.  That factor alone might cause
-	 * _bt_vacuum_needs_cleanup() to force the next VACUUM to proceed with a
-	 * btvacuumscan() call.
-	 *
-	 * Note: We must delay the _bt_set_cleanup_info() call until this late
-	 * stage of VACUUM (the btvacuumcleanup() phase), to keep num_heap_tuples
-	 * accurate.  The btbulkdelete()-time num_heap_tuples value is generally
-	 * just pg_class.reltuples for the heap relation _before_ VACUUM began.
-	 * In general cleanup info should describe the state of the index/table
-	 * _after_ VACUUM finishes.
+	 * FSM during the next VACUUM operation.  _bt_vacuum_needs_cleanup() will
+	 * force the next VACUUM to consider this before allowing btvacuumscan()
+	 * to be skipped entirely.  This should be rare -- cleanup-only VACUUMs
+	 * almost always manage to skip btvacuumscan() in practice.
 	 */
 	Assert(stats->pages_deleted >= stats->pages_free);
 	num_delpages = stats->pages_deleted - stats->pages_free;
-	_bt_set_cleanup_info(info->index, num_delpages, info->num_heap_tuples);
+	_bt_set_cleanup_info(info->index, num_delpages);
 
 	/*
 	 * It's quite possible for us to be fooled by concurrent page splits into
 	 * double-counting some index tuples, so disbelieve any total that exceeds
 	 * the underlying heap's count ... if we know that accurately.  Otherwise
 	 * this might just make matters worse.
-	 *
-	 * Posting list tuples are another source of inaccuracy.  Cleanup-only
-	 * btvacuumscan calls assume that the number of index tuples can be used
-	 * as num_index_tuples, even though num_index_tuples is supposed to
-	 * represent the number of TIDs in the index.  This naive approach can
-	 * underestimate the number of tuples in the index.
 	 */
 	if (!info->estimated_count)
 	{
@@ -1016,7 +987,6 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 * pages in the index at the end of the VACUUM command.)
 	 */
 	stats->num_pages = 0;
-	stats->estimated_count = false;
 	stats->num_index_tuples = 0;
 	stats->pages_deleted = 0;
 	stats->pages_free = 0;
@@ -1421,7 +1391,10 @@ backtrack:
 		 * We don't count the number of live TIDs during cleanup-only calls to
 		 * btvacuumscan (i.e. when callback is not set).  We count the number
 		 * of index tuples directly instead.  This avoids the expense of
-		 * directly examining all of the tuples on each page.
+		 * directly examining all of the tuples on each page.  VACUUM will
+		 * treat num_index_tuples as an estimate in cleanup-only case, so it
+		 * doesn't matter that this underestimates num_index_tuples
+		 * significantly in some cases.
 		 */
 		if (minoff > maxoff)
 			attempt_pagedel = (blkno == scanblkno);
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index d524310723..fdbe0da472 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2105,8 +2105,6 @@ btoptions(Datum reloptions, bool validate)
 {
 	static const relopt_parse_elt tab[] = {
 		{"fillfactor", RELOPT_TYPE_INT, offsetof(BTOptions, fillfactor)},
-		{"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(BTOptions, vacuum_cleanup_index_scale_factor)},
 		{"deduplicate_items", RELOPT_TYPE_BOOL,
 		offsetof(BTOptions, deduplicate_items)}
 
diff --git a/src/backend/access/nbtree/nbtxlog.c b/src/backend/access/nbtree/nbtxlog.c
index 990f5d0f52..1779b6ba47 100644
--- a/src/backend/access/nbtree/nbtxlog.c
+++ b/src/backend/access/nbtree/nbtxlog.c
@@ -113,7 +113,7 @@ _bt_restore_meta(XLogReaderState *record, uint8 block_id)
 	/* Cannot log BTREE_MIN_VERSION index metapage without upgrade */
 	Assert(md->btm_version >= BTREE_NOVAC_VERSION);
 	md->btm_last_cleanup_num_delpages = xlrec->last_cleanup_num_delpages;
-	md->btm_last_cleanup_num_heap_tuples = xlrec->last_cleanup_num_heap_tuples;
+	md->btm_last_cleanup_num_heap_tuples = -1.0;
 	md->btm_allequalimage = xlrec->allequalimage;
 
 	pageop = (BTPageOpaque) PageGetSpecialPointer(metapg);
diff --git a/src/backend/access/rmgrdesc/nbtdesc.c b/src/backend/access/rmgrdesc/nbtdesc.c
index f7cc4dd3e6..710efbd36a 100644
--- a/src/backend/access/rmgrdesc/nbtdesc.c
+++ b/src/backend/access/rmgrdesc/nbtdesc.c
@@ -113,9 +113,8 @@ btree_desc(StringInfo buf, XLogReaderState *record)
 
 				xlrec = (xl_btree_metadata *) XLogRecGetBlockData(record, 0,
 																  NULL);
-				appendStringInfo(buf, "last_cleanup_num_delpages %u; last_cleanup_num_heap_tuples: %f",
-								 xlrec->last_cleanup_num_delpages,
-								 xlrec->last_cleanup_num_heap_tuples);
+				appendStringInfo(buf, "last_cleanup_num_delpages %u",
+								 xlrec->last_cleanup_num_delpages);
 				break;
 			}
 	}
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index a5976ad5b1..73e0a672ae 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -148,5 +148,3 @@ int64		VacuumPageDirty = 0;
 
 int			VacuumCostBalance = 0;	/* working state for vacuum */
 bool		VacuumCostActive = false;
-
-double		vacuum_cleanup_index_scale_factor;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 3fd1a5fbe2..df199851cc 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3694,16 +3694,6 @@ static struct config_real ConfigureNamesReal[] =
 		NULL, NULL, NULL
 	},
 
-	{
-		{"vacuum_cleanup_index_scale_factor", PGC_USERSET, CLIENT_CONN_STATEMENT,
-			gettext_noop("Number of tuple inserts prior to index cleanup as a fraction of reltuples."),
-			NULL
-		},
-		&vacuum_cleanup_index_scale_factor,
-		0.1, 0.0, 1e10,
-		NULL, NULL, NULL
-	},
-
 	{
 		{"log_statement_sample_rate", PGC_SUSET, LOGGING_WHEN,
 			gettext_noop("Fraction of statements exceeding log_min_duration_sample to be logged."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index ee06528bb0..3736c972a8 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -671,9 +671,6 @@
 #vacuum_freeze_table_age = 150000000
 #vacuum_multixact_freeze_min_age = 5000000
 #vacuum_multixact_freeze_table_age = 150000000
-#vacuum_cleanup_index_scale_factor = 0.1	# fraction of total number of tuples
-						# before index cleanup, 0 always performs
-						# index cleanup
 #bytea_output = 'hex'			# hex, escape
 #xmlbinary = 'base64'
 #xmloption = 'content'
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9f0208ac49..ecdb8d752b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1789,14 +1789,14 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER INDEX <foo> SET|RESET ( */
 	else if (Matches("ALTER", "INDEX", MatchAny, "RESET", "("))
 		COMPLETE_WITH("fillfactor",
-					  "vacuum_cleanup_index_scale_factor", "deduplicate_items", /* BTREE */
+					  "deduplicate_items", /* BTREE */
 					  "fastupdate", "gin_pending_list_limit",	/* GIN */
 					  "buffering",	/* GiST */
 					  "pages_per_range", "autosummarize"	/* BRIN */
 			);
 	else if (Matches("ALTER", "INDEX", MatchAny, "SET", "("))
 		COMPLETE_WITH("fillfactor =",
-					  "vacuum_cleanup_index_scale_factor =", "deduplicate_items =", /* BTREE */
+					  "deduplicate_items =", /* BTREE */
 					  "fastupdate =", "gin_pending_list_limit =",	/* GIN */
 					  "buffering =",	/* GiST */
 					  "pages_per_range =", "autosummarize ="	/* BRIN */
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 967de73596..81e597a80b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8523,46 +8523,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-vacuum-cleanup-index-scale-factor" xreflabel="vacuum_cleanup_index_scale_factor">
-      <term><varname>vacuum_cleanup_index_scale_factor</varname> (<type>floating point</type>)
-      <indexterm>
-       <primary><varname>vacuum_cleanup_index_scale_factor</varname></primary>
-       <secondary>configuration parameter</secondary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Specifies the fraction of the total number of heap tuples counted in
-        the previous statistics collection that can be inserted without
-        incurring an index scan at the <command>VACUUM</command> cleanup stage.
-        This setting currently applies to B-tree indexes only.
-       </para>
-
-       <para>
-        If no tuples were deleted from the heap, B-tree indexes are still
-        scanned at the <command>VACUUM</command> cleanup stage when the
-        index's statistics are stale.  Index statistics are considered
-        stale if the number of newly inserted tuples exceeds the
-        <varname>vacuum_cleanup_index_scale_factor</varname>
-        fraction of the total number of heap tuples detected by the previous
-        statistics collection. The total number of heap tuples is stored in
-        the index meta-page. Note that the meta-page does not include this data
-        until <command>VACUUM</command> finds no dead tuples, so B-tree index
-        scan at the cleanup stage can only be skipped if the second and
-        subsequent <command>VACUUM</command> cycles detect no dead tuples.
-       </para>
-
-       <para>
-        The value can range from <literal>0</literal> to
-        <literal>10000000000</literal>.
-        When <varname>vacuum_cleanup_index_scale_factor</varname> is set to
-        <literal>0</literal>, index scans are never skipped during
-        <command>VACUUM</command> cleanup. The default value is <literal>0.1</literal>.
-       </para>
-
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-bytea-output" xreflabel="bytea_output">
       <term><varname>bytea_output</varname> (<type>enum</type>)
       <indexterm>
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 51b4d57939..bc57adf7d5 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -456,20 +456,6 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
     </note>
     </listitem>
    </varlistentry>
-
-   <varlistentry id="index-reloption-vacuum-cleanup-index-scale-factor" xreflabel="vacuum_cleanup_index_scale_factor">
-    <term><literal>vacuum_cleanup_index_scale_factor</literal> (<type>floating point</type>)
-     <indexterm>
-      <primary><varname>vacuum_cleanup_index_scale_factor</varname></primary>
-      <secondary>storage parameter</secondary>
-     </indexterm>
-    </term>
-    <listitem>
-    <para>
-      Per-index value for <xref linkend="guc-vacuum-cleanup-index-scale-factor"/>.
-    </para>
-    </listitem>
-   </varlistentry>
    </variablelist>
 
    <para>
diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out
index cfd4338e36..bc113a70b4 100644
--- a/src/test/regress/expected/btree_index.out
+++ b/src/test/regress/expected/btree_index.out
@@ -308,35 +308,6 @@ alter table btree_tall_tbl alter COLUMN t set storage plain;
 create index btree_tall_idx on btree_tall_tbl (t, id) with (fillfactor = 10);
 insert into btree_tall_tbl select g, repeat('x', 250)
 from generate_series(1, 130) g;
---
--- Test vacuum_cleanup_index_scale_factor
---
--- Simple create
-create table btree_test(a int);
-create index btree_idx1 on btree_test(a) with (vacuum_cleanup_index_scale_factor = 40.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-                reloptions                
-------------------------------------------
- {vacuum_cleanup_index_scale_factor=40.0}
-(1 row)
-
--- Fail while setting improper values
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = -10.0);
-ERROR:  value -10.0 out of bounds for option "vacuum_cleanup_index_scale_factor"
-DETAIL:  Valid values are between "0.000000" and "10000000000.000000".
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 100.0);
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 'string');
-ERROR:  invalid value for floating point option "vacuum_cleanup_index_scale_factor": string
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = true);
-ERROR:  invalid value for floating point option "vacuum_cleanup_index_scale_factor": true
--- Simple ALTER INDEX
-alter index btree_idx1 set (vacuum_cleanup_index_scale_factor = 70.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-                reloptions                
-------------------------------------------
- {vacuum_cleanup_index_scale_factor=70.0}
-(1 row)
-
 --
 -- Test for multilevel page deletion
 --
diff --git a/src/test/regress/sql/btree_index.sql b/src/test/regress/sql/btree_index.sql
index 96f53818ff..c60312db2d 100644
--- a/src/test/regress/sql/btree_index.sql
+++ b/src/test/regress/sql/btree_index.sql
@@ -150,25 +150,6 @@ create index btree_tall_idx on btree_tall_tbl (t, id) with (fillfactor = 10);
 insert into btree_tall_tbl select g, repeat('x', 250)
 from generate_series(1, 130) g;
 
---
--- Test vacuum_cleanup_index_scale_factor
---
-
--- Simple create
-create table btree_test(a int);
-create index btree_idx1 on btree_test(a) with (vacuum_cleanup_index_scale_factor = 40.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-
--- Fail while setting improper values
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = -10.0);
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 100.0);
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 'string');
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = true);
-
--- Simple ALTER INDEX
-alter index btree_idx1 set (vacuum_cleanup_index_scale_factor = 70.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-
 --
 -- Test for multilevel page deletion
 --

base-commit: 8a812e5106c5db50039336288d376a188844e2cc
-- 
2.27.0

REL_13_STABLE-v2-0001-Don-t-consider-newly-inserted-tuples-in-nbtree-VA.patchapplication/octet-stream; name=REL_13_STABLE-v2-0001-Don-t-consider-newly-inserted-tuples-in-nbtree-VA.patchDownload

From 62c0fd0583d7c163c36855eae1c8b6817a6f5e69 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Tue, 9 Mar 2021 19:21:40 -0800
Subject: [PATCH v2 1/2] Don't consider newly inserted tuples in nbtree VACUUM.

Skip full index scan during a VACUUM for nbtree indexes in the case
where VACUUM never called btbulkdelete(), even when pg_class stats for
the index relation would be considered "stale" by criteria applied using
vacuum_cleanup_index_scale_factor.  Rely on ANALYZE instead -- it can be
relied on to keep pg_class.reltuples up to date, per the amvacuumcleanup
contract.  Also remove the vacuum_cleanup_index_scale_factor GUC/param
in passing (though just disable it on the Postgres 13 branch).

VACUUM will still do scans of the index despite never reaching
btbulkdelete() in one remaining case:  the case where a previous VACUUM
operation is known to have performed index page deletion of pages that
have yet be placed in the free space map for recycling.

Backpatch to Postgres 13 due to an unanticipated interaction with the
autovacuum_vacuum_insert_threshold feature added by commit b07642db and
the "skip full scan" feature added by commit 857f9c36.  This interaction
has been tied to a regression with an append-only insert benchmark [1].
It is reasonable to expect a certain amount of overhead from vacuuming
that just sets visibility map bits, but it does not seem reasonable to
perform a full index scans in btvacuumcleanup() purely to set the
pg_class.reltuples stats in affected indexes.

There is another reason to backpatch: a bugfix commit tied to the nbtree
deduplication feature (bugfix commit 48e12913) taught nbtree VACUUM to
track IndexBulkDeleteResult.num_index_tuples using an inherently
approximate approach.  This made sense -- getting an accurate count in
the presence of posting list tuples just isn't worth the cycles for a
btvacuumcleanup()-only VACUUM.  But btvacuumcleanup() should still
indicate that its final num_index_tuples value is just an estimate when
its approximate approach to tracking live tuples gets used.

Have btvacuumcleanup() acknowledge that its approach to counting is
approximate by setting IndexBulkDeleteResult.estimated_count to 'true'.
This prevents vacuumlazy.c from setting the index's pg_class.reltuples
to a value that significantly underestimates the number of live tuples
(at least in certain scenarios with large posting list tuples).  This is
the same approach that hashvacuumcleanup() has always taken.  Index AMs
have had the option of giving only approximate num_index_tuples
statistics since commit e57345975cf, which updated the relevant index AM
API.

[1] https://smalldatum.blogspot.com/2021/01/insert-benchmark-postgres-is-still.html

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoA4WHthN5uU6+WScZ7+J_RcEjmcuH94qcoUPuB42ShXzg@mail.gmail.com
Backpatch: 13-, where autovacuum_vacuum_insert_threshold was added.
---
 src/backend/access/nbtree/nbtpage.c       |  22 +++--
 src/backend/access/nbtree/nbtree.c        | 102 +++++++++++-----------
 doc/src/sgml/config.sgml                  |  41 ---------
 doc/src/sgml/ref/create_index.sgml        |  14 ---
 src/test/regress/expected/btree_index.out |  29 ------
 src/test/regress/sql/btree_index.sql      |  19 ----
 6 files changed, 60 insertions(+), 167 deletions(-)

diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 75628e0eb9..ac6c38b155 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -168,6 +168,9 @@ _bt_getmeta(Relation rel, Buffer metabuf)
  *
  *		This routine checks if provided cleanup-related information is matching
  *		to those written in the metapage.  On mismatch, metapage is overwritten.
+ *
+ *		Postgres 13 ignores btm_last_cleanup_num_heap_tuples value here
+ *		following backbranch disabling of vacuum_cleanup_index_scale_factor.
  */
 void
 _bt_update_meta_cleanup_info(Relation rel, TransactionId oldestBtpoXact,
@@ -176,22 +179,15 @@ _bt_update_meta_cleanup_info(Relation rel, TransactionId oldestBtpoXact,
 	Buffer		metabuf;
 	Page		metapg;
 	BTMetaPageData *metad;
-	bool		needsRewrite = false;
-	XLogRecPtr	recptr;
 
 	/* read the metapage and check if it needs rewrite */
 	metabuf = _bt_getbuf(rel, BTREE_METAPAGE, BT_READ);
 	metapg = BufferGetPage(metabuf);
 	metad = BTPageGetMeta(metapg);
 
-	/* outdated version of metapage always needs rewrite */
-	if (metad->btm_version < BTREE_NOVAC_VERSION)
-		needsRewrite = true;
-	else if (metad->btm_oldest_btpo_xact != oldestBtpoXact ||
-			 metad->btm_last_cleanup_num_heap_tuples != numHeapTuples)
-		needsRewrite = true;
-
-	if (!needsRewrite)
+	/* Don't miss chance to upgrade index/metapage when BTREE_MIN_VERSION */
+	if (metad->btm_version >= BTREE_NOVAC_VERSION &&
+		metad->btm_oldest_btpo_xact == oldestBtpoXact)
 	{
 		_bt_relbuf(rel, metabuf);
 		return;
@@ -209,13 +205,14 @@ _bt_update_meta_cleanup_info(Relation rel, TransactionId oldestBtpoXact,
 
 	/* update cleanup-related information */
 	metad->btm_oldest_btpo_xact = oldestBtpoXact;
-	metad->btm_last_cleanup_num_heap_tuples = numHeapTuples;
+	metad->btm_last_cleanup_num_heap_tuples = -1;
 	MarkBufferDirty(metabuf);
 
 	/* write wal record if needed */
 	if (RelationNeedsWAL(rel))
 	{
 		xl_btree_metadata md;
+		XLogRecPtr	recptr;
 
 		XLogBeginInsert();
 		XLogRegisterBuffer(0, metabuf, REGBUF_WILL_INIT | REGBUF_STANDARD);
@@ -227,7 +224,7 @@ _bt_update_meta_cleanup_info(Relation rel, TransactionId oldestBtpoXact,
 		md.fastroot = metad->btm_fastroot;
 		md.fastlevel = metad->btm_fastlevel;
 		md.oldest_btpo_xact = oldestBtpoXact;
-		md.last_cleanup_num_heap_tuples = numHeapTuples;
+		md.last_cleanup_num_heap_tuples = -1;	/* Disabled */
 		md.allequalimage = metad->btm_allequalimage;
 
 		XLogRegisterBufData(0, (char *) &md, sizeof(xl_btree_metadata));
@@ -238,6 +235,7 @@ _bt_update_meta_cleanup_info(Relation rel, TransactionId oldestBtpoXact,
 	}
 
 	END_CRIT_SECTION();
+
 	_bt_relbuf(rel, metabuf);
 }
 
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index d857afee1c..c37bbe53f9 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -794,6 +794,9 @@ _bt_parallel_advance_array_keys(IndexScanDesc scan)
  * When we return false, VACUUM can even skip the cleanup-only call to
  * btvacuumscan (i.e. there will be no btvacuumscan call for this index at
  * all).  Otherwise, a cleanup-only btvacuumscan call is required.
+ *
+ * Postgres 13 ignores btm_last_cleanup_num_heap_tuples value here following
+ * backbranch disabling of vacuum_cleanup_index_scale_factor.
  */
 static bool
 _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
@@ -801,60 +804,44 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
 	Buffer		metabuf;
 	Page		metapg;
 	BTMetaPageData *metad;
-	bool		result = false;
+	uint32		btm_version;
+	TransactionId prev_btm_oldest_btpo_xact;
 
+	/*
+	 * Copy details from metapage to local variables quickly.
+	 *
+	 * Note that we deliberately avoid using cached version of metapage here.
+	 */
 	metabuf = _bt_getbuf(info->index, BTREE_METAPAGE, BT_READ);
 	metapg = BufferGetPage(metabuf);
 	metad = BTPageGetMeta(metapg);
+	btm_version = metad->btm_version;
 
-	if (metad->btm_version < BTREE_NOVAC_VERSION)
+	if (btm_version < BTREE_NOVAC_VERSION)
 	{
 		/*
-		 * Do cleanup if metapage needs upgrade, because we don't have
-		 * cleanup-related meta-information yet.
+		 * Metapage needs to be dynamically upgraded to store fields that are
+		 * only present when btm_version >= BTREE_NOVAC_VERSION
 		 */
-		result = true;
+		_bt_relbuf(info->index, metabuf);
+		return true;
 	}
-	else if (TransactionIdIsValid(metad->btm_oldest_btpo_xact) &&
-			 TransactionIdPrecedes(metad->btm_oldest_btpo_xact,
-								   RecentGlobalXmin))
+
+	prev_btm_oldest_btpo_xact = metad->btm_oldest_btpo_xact;
+	_bt_relbuf(info->index, metabuf);
+
+	if (TransactionIdIsValid(prev_btm_oldest_btpo_xact) &&
+		TransactionIdPrecedes(prev_btm_oldest_btpo_xact, RecentGlobalXmin))
 	{
 		/*
 		 * If any oldest btpo.xact from a previously deleted page in the index
 		 * is older than RecentGlobalXmin, then at least one deleted page can
 		 * be recycled -- don't skip cleanup.
 		 */
-		result = true;
-	}
-	else
-	{
-		BTOptions  *relopts;
-		float8		cleanup_scale_factor;
-		float8		prev_num_heap_tuples;
-
-		/*
-		 * If table receives enough insertions and no cleanup was performed,
-		 * then index would appear have stale statistics.  If scale factor is
-		 * set, we avoid that by performing cleanup if the number of inserted
-		 * tuples exceeds vacuum_cleanup_index_scale_factor fraction of
-		 * original tuples count.
-		 */
-		relopts = (BTOptions *) info->index->rd_options;
-		cleanup_scale_factor = (relopts &&
-								relopts->vacuum_cleanup_index_scale_factor >= 0)
-			? relopts->vacuum_cleanup_index_scale_factor
-			: vacuum_cleanup_index_scale_factor;
-		prev_num_heap_tuples = metad->btm_last_cleanup_num_heap_tuples;
-
-		if (cleanup_scale_factor <= 0 ||
-			prev_num_heap_tuples <= 0 ||
-			(info->num_heap_tuples - prev_num_heap_tuples) /
-			prev_num_heap_tuples >= cleanup_scale_factor)
-			result = true;
+		return true;
 	}
 
-	_bt_relbuf(info->index, metabuf);
-	return result;
+	return false;
 }
 
 /*
@@ -907,9 +894,6 @@ btvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
 	 * still need to do a pass over the index, to recycle any newly-recyclable
 	 * pages or to obtain index statistics.  _bt_vacuum_needs_cleanup
 	 * determines if either are needed.
-	 *
-	 * Since we aren't going to actually delete any leaf items, there's no
-	 * need to go through all the vacuum-cycle-ID pushups.
 	 */
 	if (stats == NULL)
 	{
@@ -917,8 +901,23 @@ btvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
 		if (!_bt_vacuum_needs_cleanup(info))
 			return NULL;
 
+		/*
+		 * Since we aren't going to actually delete any leaf items, there's no
+		 * need to go through all the vacuum-cycle-ID pushups here.
+		 *
+		 * Posting list tuples are a source of inaccuracy for cleanup-only
+		 * scans.  btvacuumscan() will assume that the number of index tuples
+		 * from each page can be used as num_index_tuples, even though
+		 * num_index_tuples is supposed to represent the number of TIDs in the
+		 * index.  This naive approach can underestimate the number of tuples
+		 * in the index significantly.
+		 *
+		 * We handle the problem by making num_index_tuples an estimate in
+		 * cleanup-only case.
+		 */
 		stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
 		btvacuumscan(info, stats, NULL, NULL, 0);
+		stats->estimated_count = true;
 	}
 
 	/*
@@ -926,12 +925,6 @@ btvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
 	 * double-counting some index tuples, so disbelieve any total that exceeds
 	 * the underlying heap's count ... if we know that accurately.  Otherwise
 	 * this might just make matters worse.
-	 *
-	 * Posting list tuples are another source of inaccuracy.  Cleanup-only
-	 * btvacuumscan calls assume that the number of index tuples can be used
-	 * as num_index_tuples, even though num_index_tuples is supposed to
-	 * represent the number of TIDs in the index.  This naive approach can
-	 * underestimate the number of tuples in the index.
 	 */
 	if (!info->estimated_count)
 	{
@@ -971,7 +964,6 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 * Reset counts that will be incremented during the scan; needed in case
 	 * of multiple scans during a single VACUUM command
 	 */
-	stats->estimated_count = false;
 	stats->num_index_tuples = 0;
 	stats->pages_deleted = 0;
 
@@ -1059,8 +1051,12 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		IndexFreeSpaceMapVacuum(rel);
 
 	/*
-	 * Maintain the oldest btpo.xact and a count of the current number of heap
-	 * tuples in the metapage (for the benefit of _bt_vacuum_needs_cleanup).
+	 * Maintain the oldest btpo.xact using _bt_update_meta_cleanup_info, for
+	 * the benefit of _bt_vacuum_needs_cleanup.
+	 *
+	 * Note: We deliberately don't store the count of heap tuples here
+	 * anymore.  The numHeapTuples argument to _bt_update_meta_cleanup_info()
+	 * is left in place on Postgres 13.
 	 *
 	 * The page with the oldest btpo.xact is typically a page deleted by this
 	 * VACUUM operation, since pages deleted by a previous VACUUM operation
@@ -1070,8 +1066,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 * statistics, despite not counting as deleted pages for the purposes of
 	 * determining the oldest btpo.xact.)
 	 */
-	_bt_update_meta_cleanup_info(rel, vstate.oldestBtpoXact,
-								 info->num_heap_tuples);
+	_bt_update_meta_cleanup_info(rel, vstate.oldestBtpoXact, -1);
 
 	/* update statistics */
 	stats->num_pages = num_pages;
@@ -1399,7 +1394,10 @@ backtrack:
 		 * We don't count the number of live TIDs during cleanup-only calls to
 		 * btvacuumscan (i.e. when callback is not set).  We count the number
 		 * of index tuples directly instead.  This avoids the expense of
-		 * directly examining all of the tuples on each page.
+		 * directly examining all of the tuples on each page.  VACUUM will
+		 * treat num_index_tuples as an estimate in cleanup-only case, so it
+		 * doesn't matter that this underestimates num_index_tuples
+		 * significantly in some cases.
 		 */
 		if (minoff > maxoff)
 			attempt_pagedel = (blkno == scanblkno);
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index ca0d9bd917..dd2778611f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8349,47 +8349,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-vacuum-cleanup-index-scale-factor" xreflabel="vacuum_cleanup_index_scale_factor">
-      <term><varname>vacuum_cleanup_index_scale_factor</varname> (<type>floating point</type>)
-      <indexterm>
-       <primary><varname>vacuum_cleanup_index_scale_factor</varname></primary>
-       <secondary>configuration parameter</secondary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Specifies the fraction of the total number of heap tuples counted in
-        the previous statistics collection that can be inserted without
-        incurring an index scan at the <command>VACUUM</command> cleanup stage.
-        This setting currently applies to B-tree indexes only.
-       </para>
-
-       <para>
-        If no tuples were deleted from the heap, B-tree indexes are still
-        scanned at the <command>VACUUM</command> cleanup stage when at least one
-        of the following conditions is met: the index statistics are stale, or
-        the index contains deleted pages that can be recycled during cleanup.
-        Index statistics are considered to be stale if the number of newly
-        inserted tuples exceeds the <varname>vacuum_cleanup_index_scale_factor</varname>
-        fraction of the total number of heap tuples detected by the previous
-        statistics collection. The total number of heap tuples is stored in
-        the index meta-page. Note that the meta-page does not include this data
-        until <command>VACUUM</command> finds no dead tuples, so B-tree index
-        scan at the cleanup stage can only be skipped if the second and
-        subsequent <command>VACUUM</command> cycles detect no dead tuples.
-       </para>
-
-       <para>
-        The value can range from <literal>0</literal> to
-        <literal>10000000000</literal>.
-        When <varname>vacuum_cleanup_index_scale_factor</varname> is set to
-        <literal>0</literal>, index scans are never skipped during
-        <command>VACUUM</command> cleanup. The default value is <literal>0.1</literal>.
-       </para>
-
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-bytea-output" xreflabel="bytea_output">
       <term><varname>bytea_output</varname> (<type>enum</type>)
       <indexterm>
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 3537dfaade..5a1fd71478 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -434,20 +434,6 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
     </note>
     </listitem>
    </varlistentry>
-
-   <varlistentry id="index-reloption-vacuum-cleanup-index-scale-factor" xreflabel="vacuum_cleanup_index_scale_factor">
-    <term><literal>vacuum_cleanup_index_scale_factor</literal> (<type>floating point</type>)
-     <indexterm>
-      <primary><varname>vacuum_cleanup_index_scale_factor</varname></primary>
-      <secondary>storage parameter</secondary>
-     </indexterm>
-    </term>
-    <listitem>
-    <para>
-      Per-index value for <xref linkend="guc-vacuum-cleanup-index-scale-factor"/>.
-    </para>
-    </listitem>
-   </varlistentry>
    </variablelist>
 
    <para>
diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out
index cfd4338e36..bc113a70b4 100644
--- a/src/test/regress/expected/btree_index.out
+++ b/src/test/regress/expected/btree_index.out
@@ -308,35 +308,6 @@ alter table btree_tall_tbl alter COLUMN t set storage plain;
 create index btree_tall_idx on btree_tall_tbl (t, id) with (fillfactor = 10);
 insert into btree_tall_tbl select g, repeat('x', 250)
 from generate_series(1, 130) g;
---
--- Test vacuum_cleanup_index_scale_factor
---
--- Simple create
-create table btree_test(a int);
-create index btree_idx1 on btree_test(a) with (vacuum_cleanup_index_scale_factor = 40.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-                reloptions                
-------------------------------------------
- {vacuum_cleanup_index_scale_factor=40.0}
-(1 row)
-
--- Fail while setting improper values
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = -10.0);
-ERROR:  value -10.0 out of bounds for option "vacuum_cleanup_index_scale_factor"
-DETAIL:  Valid values are between "0.000000" and "10000000000.000000".
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 100.0);
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 'string');
-ERROR:  invalid value for floating point option "vacuum_cleanup_index_scale_factor": string
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = true);
-ERROR:  invalid value for floating point option "vacuum_cleanup_index_scale_factor": true
--- Simple ALTER INDEX
-alter index btree_idx1 set (vacuum_cleanup_index_scale_factor = 70.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-                reloptions                
-------------------------------------------
- {vacuum_cleanup_index_scale_factor=70.0}
-(1 row)
-
 --
 -- Test for multilevel page deletion
 --
diff --git a/src/test/regress/sql/btree_index.sql b/src/test/regress/sql/btree_index.sql
index 96f53818ff..c60312db2d 100644
--- a/src/test/regress/sql/btree_index.sql
+++ b/src/test/regress/sql/btree_index.sql
@@ -150,25 +150,6 @@ create index btree_tall_idx on btree_tall_tbl (t, id) with (fillfactor = 10);
 insert into btree_tall_tbl select g, repeat('x', 250)
 from generate_series(1, 130) g;
 
---
--- Test vacuum_cleanup_index_scale_factor
---
-
--- Simple create
-create table btree_test(a int);
-create index btree_idx1 on btree_test(a) with (vacuum_cleanup_index_scale_factor = 40.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-
--- Fail while setting improper values
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = -10.0);
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 100.0);
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = 'string');
-create index btree_idx_err on btree_test(a) with (vacuum_cleanup_index_scale_factor = true);
-
--- Simple ALTER INDEX
-alter index btree_idx1 set (vacuum_cleanup_index_scale_factor = 70.0);
-select reloptions from pg_class WHERE oid = 'btree_idx1'::regclass;
-
 --
 -- Test for multilevel page deletion
 --

base-commit: 21d5a065fd5f0ed71e0f6726a869c64d13716ceb
-- 
2.27.0

Peter Geoghegan

pg@bowt.ie

almost 5 years ago

In reply to: Peter Geoghegan (#7)

Re: Removing vacuum_cleanup_index_scale_factor

On Tue, Mar 9, 2021 at 7:42 PM Peter Geoghegan <pg@bowt.ie> wrote:

My current plan is to commit everything within the next day or two.
This includes backpatching to Postgres 13 only.

Pushed, thanks.

--
Peter Geoghegan

Masahiko Sawada

sawada.mshk@gmail.com

almost 5 years ago

In reply to: Peter Geoghegan (#8)

Re: Removing vacuum_cleanup_index_scale_factor

On Thu, Mar 11, 2021 at 10:12 AM Peter Geoghegan <pg@bowt.ie> wrote:

On Tue, Mar 9, 2021 at 7:42 PM Peter Geoghegan <pg@bowt.ie> wrote:

My current plan is to commit everything within the next day or two.
This includes backpatching to Postgres 13 only.

Pushed, thanks.

Great! Thank you!

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/