Date: Wed, 24 Feb 2021 22:13:52 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Heikki Linnakangas <hlinnaka@iki.fi>
Subject: Re: 64-bit XIDs in deleted nbtree pages
Message-ID: <20210225041352.GK20769@telsasoft.com>
References: 
 <CAD21AoBWhbKJjV4tZ0EretU0GJn+OuTJwD-3X_1kaQ3rL_GnAA@mail.gmail.com>
 <CAH2-WznpLLws36u8jXOY+xe6z4aZ3YAjJb03_BfBVgckNkT7_g@mail.gmail.com>
 <CAH2-Wzk76_P=67iUscb1UN44-gyZL-KgpsXbSxq_bdcMa7Q+wQ@mail.gmail.com>
 <CAD21AoAaHg86bGm=k8cBtK9HeO46QGRMX4pxNt5gt_11ispFGA@mail.gmail.com>
 <CAH2-WzkP=9CUYmbCi7Yzopsx380kaPHi686Sy22Skhwv4nCfOg@mail.gmail.com>
 <CAD21AoA9g=+oE9xgzJVzS62oi8Rx8_zqyDQ3BkaTLt+Qd+_M1Q@mail.gmail.com>
 <CAH2-WzmUcsJd6a9xUdUxEmuCe++jJWJDaoJBzZ=Ku58yyyUv0Q@mail.gmail.com>
 <CAD21AoB8T=idEbdFJoHBDgjsO49YggCERJKUhXQ_M-egbjTP-Q@mail.gmail.com>
 <CAD21AoAJur1ZbZzTo7qier0mrOjSMgNf=fVyPE+H4gk1CxsjOw@mail.gmail.com>
 <CAH2-WznF+abo5B0BfCLC-SUC0F6tbro6Eksj1k=04uKg_THuYA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="IS0zKkzwUGydFO0o"
Content-Disposition: inline
In-Reply-To: 
 <CAH2-WznF+abo5B0BfCLC-SUC0F6tbro6Eksj1k=04uKg_THuYA@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--IS0zKkzwUGydFO0o
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Feb 22, 2021 at 02:54:54PM -0800, Peter Geoghegan wrote:
> On Mon, Feb 22, 2021 at 4:21 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > The 0001 patch looks good to me. In the documentation, I think we need
> > to update the following paragraph in the description of
> > vacuum_cleanup_index_scale_factor:
> 
> Good point. I think that the structure should make the page deletion
> triggering condition have only secondary importance -- it is only
> described at all to be complete and exhaustive. The
> vacuum_cleanup_index_scale_factor-related threshold is all that users
> will really care about in this area.
> 
> The reasons for this are: it's pretty rare to have many page
> deletions, but never again delete/non-hot update even one single
> tuple. But when that happens, it's *much* rarer still to *also* have
> inserts, that might actually benefit from recycling the deleted page.
> So it's very narrow.
> 
> I think that I'll add a "Note" box that talks about the page deletion
> stuff, right at the end. It's actually kind of an awkward thing to
> describe, and yet I think we still need to describe it.
> 
> I also think that the existing documentation should clearly point out
> that the vacuum_cleanup_index_scale_factor only gets considered when
> there are no updates or deletes since the last VACUUM -- that seems
> like an existing problem worth fixing now. It's way too unclear that
> this setting only really concerns append-only tables.

e5d8a999030418a1b9e53d5f15ccaca7ed674877
|    I (pgeoghegan) have chosen to remove any mention of deleted pages in the
|    documentation of the vacuum_cleanup_index_scale_factor GUC/param, since
|    the presence of deleted (though unrecycled) pages is no longer of much
|    concern to users.  The vacuum_cleanup_index_scale_factor description in
|    the docs now seems rather unclear in any case, and it should probably be
|    rewritten in the near future.  Perhaps some passing mention of page
|    deletion will be added back at the same time.

I think 8e12f4a25 wasn't quite aggressive enough in its changes, and I had
another patch laying around.  I rebased and came up with this.

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 9851ca68b4..5da2e705b9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8522,24 +8522,26 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
      </term>
      <listitem>
       <para>
        Specifies [-the fraction-]{+a multiplier+} of the total number of heap tuples[-counted in-]
[-        the previous statistics collection-] that can be
        inserted [-without-]{+before+} incurring an index scan at the <command>VACUUM</command>
        cleanup stage.
        This setting currently applies to B-tree indexes only.
       </para>

       <para>
        [-If-]{+During <command>VACUUM</command>, if there are+} no {+dead+} tuples [-were deleted from-]{+found while+}
{+        scanning+} the heap, [-B-tree-]{+then the index vacuum phase is skipped.+}
{+        However,+} indexes [-are-]{+might+} still {+be+} scanned [-at-]{+during+} the[-<command>VACUUM</command>-] cleanup [-stage when-]{+phase.  Setting this+}
{+        parameter enables+} the [-index's-]{+possibility to skip scanning indexes during cleanup.+}
{+        Indexes will always be scanned when their+} statistics are stale.
        Index statistics are considered {+to be+} stale if the number of newly
        inserted tuples exceeds the <varname>vacuum_cleanup_index_scale_factor</varname>
        [-fraction-]{+multiplier+} of the total number of heap tuples [-detected by-]{+at the time of+} the previous
        [-statistics collection.-]{+vacuum cleanup.+} 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-]{+scans+} at the cleanup stage [-can only-]{+cannot+} be skipped [-if the second and-]
[-        subsequent <command>VACUUM</command> cycles detect-]{+until after a vacuum cycle+}
{+        which detects+} no dead tuples.
       </para>

       <para>

--IS0zKkzwUGydFO0o
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-fix-vacuum_cleanup_index_scale_factor.patch"

From 44cf90c5b06fb21c2c4d379568b9fe86a54f1530 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 15 Mar 2020 13:06:02 -0500
Subject: [PATCH] fix: vacuum_cleanup_index_scale_factor

---
 doc/src/sgml/config.sgml | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 9851ca68b4..5da2e705b9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8522,24 +8522,26 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </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.
+        Specifies a multiplier of the total number of heap tuples that can be
+        inserted before 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
+        During <command>VACUUM</command>, if there are no dead tuples found while
+        scanning the heap, then the index vacuum phase is skipped.
+        However, indexes might still be scanned during the cleanup phase.  Setting this
+        parameter enables the possibility to skip scanning indexes during cleanup.
+        Indexes will always be scanned when their statistics are stale.
+        Index statistics are considered to be stale if the number of newly
+        inserted tuples exceeds the <varname>vacuum_cleanup_index_scale_factor</varname>
+        multiplier of the total number of heap tuples at the time of the previous
+        vacuum cleanup. 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.
+        scans at the cleanup stage cannot be skipped until after a vacuum cycle
+        which detects no dead tuples.
        </para>
 
        <para>
-- 
2.17.0


--IS0zKkzwUGydFO0o--