Re: New vacuum option to do only freezing

On 1/21/19, 2:23 AM, "Masahiko Sawada" <sawada.mshk@gmail.com> wrote:
> Understood and Agreed. I've attached the new version patch
> incorporated the review comments.

I took a look at the latest version of the patch.

+    <varlistentry>
+    <term><literal>DISABLE_INDEX_CLEANUP</literal></term>
+    <listitem>
+     <para>
+      <command>VACUUM</command> removes dead tuples and prunes HOT-updated
+      tuples chain for live tuples on table. If the table has any dead tuple
+      it removes them from both table and indexes for re-use. With this
+      option <command>VACUUM</command> doesn't completely remove dead tuples
+      and disables removing dead tuples from indexes.  This is suitable for
+      avoiding transaction ID wraparound but not sufficient for avoiding
+      index bloat. This option is ignored if the table doesn't have index.
+      Also, this cannot be used in conjunction with <literal>FULL</literal>
+      option.
+     </para>
+    </listitem>
+   </varlistentry>

IMHO we could document this feature at a slightly higher level without
leaving out any really important user-facing behavior.  Here's a quick
attempt to show what I am thinking:

        With this option, VACUUM skips all index cleanup behavior and
        only marks tuples as "dead" without reclaiming the storage.
        While this can help reclaim transaction IDs faster to avoid
        transaction ID wraparound (see Section 24.1.5), it will not
        reduce bloat.  Note that this option is ignored for tables
        that have no indexes.  Also, this option cannot be used in
        conjunction with the FULL option, since FULL requires
        rewriting the table.

+    /* Notify user that DISABLE_INDEX_CLEANUP option is ignored */
+    if (!vacrelstats->hasindex && (options & VACOPT_DISABLE_INDEX_CLEANUP))
+        ereport(NOTICE,
+                (errmsg("DISABLE_INDEX_CLEANUP is ignored because table \"%s\" does not have index",
+                        RelationGetRelationName(onerel))));

It might make more sense to emit this in lazy_scan_heap() where we
determine the value of skip_index_vacuum.

+        if (skip_index_vacuum)
+            ereport(elevel,
+                    (errmsg("\"%s\": marked %.0f row versions as dead in %u pages",
+                            RelationGetRelationName(onerel),
+                            tups_vacuumed, vacuumed_pages)));

IIUC tups_vacuumed will include tuples removed during HOT pruning, so
it could be inaccurate to say all of these tuples have only been
marked "dead."  Perhaps we could keep a separate count of tuples
removed via HOT pruning in case we are using DISABLE_INDEX_CLEANUP.
There might be similar problems with the values stored in vacrelstats
that are used at the end of heap_vacuum_rel() (e.g. tuples_deleted).

I would suggest adding this option to vacuumdb in this patch set as
well.

Nathan