doc fixes: vacuum_cleanup_index_scale_factor

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Teodor Sigaev <teodor(at)sigaev(dot)ru>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: doc fixes: vacuum_cleanup_index_scale_factor
Date: 2018-05-02 02:30:25
Message-ID: 20180502023025.GD7631@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Introduced 857f9c36cda520030381bd8c2af20adf0ce0e1d4

The "minimal version" should probably be "minimum version", but I didn't
include it here.

Also, the documentation doesn't indicate the default value of -1 (or its
special meaning).

Also, my understanding of this feature changed when I read this logic:

if (cleanup_scale_factor < 0 ||
metad->btm_last_cleanup_num_heap_tuples < 0 ||
info->num_heap_tuples > (1.0 + cleanup_scale_factor) *
metad->btm_last_cleanup_num_heap_tuples)
result = true;

=> That means that _bt_vacuum_needs_cleanup() returns true unless a nondefault
value is set. That feels wrong, since the doc said:

"When no tuples were deleted from the heap, B-tree indexes
might still be scanned during <command>VACUUM</command>
cleanup stage by two reasons."

Which sounds like "being scanned when no tuples were deleted" is exceptional
rather, than the default.

I changed that paragraph based on that understanding (which is consistent with
the commit message). The language could probably be further improved, but
someone should first verify my tentative understanding of the intent.

Justin

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index eabe2a9..18c0ec0 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1893,14 +1893,15 @@ include_dir 'conf.d'
</term>
<listitem>
<para>
- When no tuples were deleted from the heap, B-tree indexes might still
- be scanned during <command>VACUUM</command> cleanup stage by two
- reasons. The first reason is that B-tree index contains deleted pages
- which can be recycled during cleanup. The second reason is that B-tree
- index statistics is stalled. The criterion of stalled index statistics
- is number of inserted tuples since previous statistics collection
- is greater than <varname>vacuum_cleanup_index_scale_factor</varname>
- fraction of total number of heap tuples.
+ When no tuples were deleted from the heap, B-tree indexes are still
+ scanned during <command>VACUUM</command> cleanup stage unless
+ two conditions are met. First, if a B-tree index contains no deleted pages
+ which can be recycled during cleanup. Second, if B-tree
+ index statistics are not stale. Index statistics are considered stale unless
+ <varname>vacuum_cleanup_index_scale_factor</varname> is non-negative, and the
+ number of inserted tuples since the previous statistics collection is
+ less than that fraction of the total number of heap tuples.
+ The default is -1, meaning index scan during cleanup is not skipped.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 3bcc56e..22b4a75 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -189,7 +189,7 @@ _bt_update_meta_cleanup_info(Relation rel, TransactionId oldestBtpoXact,
if (metad->btm_version < BTREE_VERSION)
_bt_upgrademetapage(metapg);

- /* update cleanup-related infromation */
+ /* update cleanup-related information */
metad->btm_oldest_btpo_xact = oldestBtpoXact;
metad->btm_last_cleanup_num_heap_tuples = numHeapTuples;
MarkBufferDirty(metabuf);
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index e5dce00..4e86280 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -818,10 +818,10 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
float8 cleanup_scale_factor;

/*
- * If table receives large enough amount of insertions and no cleanup
- * was performed, then index might appear to have stalled statistics.
- * In order to evade that, we perform cleanup when table receives
- * vacuum_cleanup_index_scale_factor fractions of insertions.
+ * 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 added tuples
+ * exceeds vacuum_cleanup_index_scale_factor fraction of original tuple count.
*/
relopts = (StdRdOptions *) info->index->rd_options;
cleanup_scale_factor = (relopts &&
@@ -870,8 +870,8 @@ btbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
&oldestBtpoXact);

/*
- * Update cleanup-related information in metapage. These information
- * is used only for cleanup but keeping up them to date can avoid
+ * Update cleanup-related information in metapage. This information
+ * is used only for cleanup but keeping them up to date can avoid
* unnecessary cleanup even after bulkdelete.
*/
_bt_update_meta_cleanup_info(info->index, oldestBtpoXact,
@@ -899,8 +899,8 @@ btvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
* If btbulkdelete was called, we need not do anything, just return the
* stats from the latest btbulkdelete call. If it wasn't called, we might
* still need to do a pass over the index, to recycle any newly-recyclable
- * pages and to obtain index statistics. _bt_vacuum_needs_cleanup checks
- * is there are newly-recyclable or stalled index statistics.
+ * pages or to obtain index statistics. _bt_vacuum_needs_cleanup
+ * determines if 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.

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-05-02 02:55:47 Re: A few warnings on Windows
Previous Message Michael Paquier 2018-05-02 02:05:33 Re: Local partitioned indexes and pageinspect