pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Bossart, Nathan <bossartn@amazon.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: New vacuum option to do only freezing
Date: Wed, 9 Jan 2019 20:23:44 +0000
Message-ID: <B3FDA4F0-385B-4764-88D2-C07C010328CD@amazon.com> (raw)
In-Reply-To: <CAD21AoD9b-WE1cyFqRC0HefNkOy-qZH3-EaOLy-ue0Y0pgQPqw@mail.gmail.com>
References: <CAD21AoAt5R3DNUZSjOoXDUY=naYPUOuffVsRzuTYMz29yLzQCA@mail.gmail.com>
	<CA+TgmoZQ_sLvsmTG251+i_Y-v3AARJ0FAy2dUH4DgL3p8xsgrw@mail.gmail.com>
	<CAD21AoCn03NJWWZ3hwxkLZk+u8xRrDk3FXHpwOwHFr=pUpipBQ@mail.gmail.com>
	<CA+TgmoZU_qfkL0Wn2aZL9yuewFARXutXwAyCw0D9HxWL5PEsqw@mail.gmail.com>
	<CAD21AoDcuhtbqChEbn1+AXHGJ6xt21zvFxsztVaBKb_ZWfbChA@mail.gmail.com>
	<CA+TgmobZ68Oaf2V7NxFDx8eiPHSW-U_b=zyOQ+kcCs6k91B-qQ@mail.gmail.com>
	<CAD21AoD9b-WE1cyFqRC0HefNkOy-qZH3-EaOLy-ue0Y0pgQPqw@mail.gmail.com>

Hi,

On 1/8/19, 7:03 PM, "Masahiko Sawada" <sawada.mshk@gmail.com> wrote:
> Attached the updated version patch incorporated all comments I got.

Thanks for the new patch!

> * Instead of freezing xmax I've changed the behaviour of the new
> option (DISABLE_INDEX_CLEANUP) so that it sets dead tuples as dead
> instead of as unused and skips both index vacuum and index cleanup.
> That is, we remove the storage of dead tuple but leave dead itemids
> for the index lookups. These are removed by the next vacuum execution
> enabling index cleanup. Currently HOT-pruning doesn't set the root of
> the chain as unused even if the whole chain is dead. Since setting
> tuples as dead removes storage the freezing xmax is no longer
> required.

I tested this option with a variety of cases (HOT, indexes, etc.), and
it seems to work well.  I haven't looked too deeply into the
implications of using LP_DEAD this way, but it seems like a reasonable
approach at first glance.

+    <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> marks tuples as dead (i.e., it doesn't
+      remove tuple storage) and disables removing dead tuples from indexes.
+      This is suitable for avoiding transaction ID wraparound but not
+      sufficient for avoiding index bloat. This option cannot be used in
+      conjunction with <literal>FULL</literal> option.
+     </para>
+    </listitem>
+   </varlistentry>

I think we should clarify the expected behavior when this option is
used on a table with no indexes.  We probably do not want to fail, as
this could disrupt VACUUM commands that touch several tables.  Also,
we don't need to set tuples as dead instead of unused, which appears
to be what this patch is actually doing:

+                       if (nindexes > 0 && disable_index_cleanup)
+                               lazy_set_tuples_dead(onerel, blkno, buf, vacrelstats);
+                       else
+                               lazy_vacuum_page(onerel, blkno, buf, 0, vacrelstats, &vmbuffer);

In this case, perhaps we should generate a log statement that notes
that DISABLE_INDEX_CLEANUP is being ignored and set
disable_index_cleanup to false during processing.

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

Should the first log statement only be generated when there are also
indexes?

+static void
+lazy_set_tuples_dead(Relation onerel, BlockNumber blkno, Buffer buffer,
+                                       LVRelStats *vacrelstats)

This function looks very similar to lazy_vacuum_page().  Perhaps the
two could be combined?

Nathan



view thread (118+ messages)  latest in thread

Message-ID: <B3FDA4F0-385B-4764-88D2-C07C010328CD@amazon.com>
Permalink:  ../B3FDA4F0-385B-4764-88D2-C07C010328CD@amazon.com/
Also on:    postgresql.org/message-id/B3FDA4F0-385B-4764-88D2-C07C010328CD@amazon.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: bossartn@amazon.com, sawada.mshk@gmail.com, robertmhaas@gmail.com
  Subject: Re: New vacuum option to do only freezing
  In-Reply-To: <B3FDA4F0-385B-4764-88D2-C07C010328CD@amazon.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox