New vacuum option to do only freezing

On Mon, Oct 1, 2018 at 7:20 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Hi,

Attached patch adds a new option FREEZE_ONLY to VACUUM command. This
option is same as FREEZE option except for it disables reclaiming dead
tuples. That is, with this option vacuum does pruning HOT chain,
freezing live tuples and maintaining both visibility map and freespace
map but does not collect dead tuples and invoke neither heap vacuum
nor index vacuum. This option will be useful if user wants to prevent
XID wraparound a table as quick as possible, especially when table is
quite large and is about to XID wraparound. I think this usecase was
mentioned in some threads but I couldn't find them.

Currently this patch just adds the new option to VACUUM command but it
might be good to make autovacuum use it when emergency vacuum is
required.

This is a performance-test result for FREEZE option and FREEZE_ONLY
option. I've tested them on the table which is about 3.8GB table
without indexes and randomly modified.

* FREEZE
INFO: aggressively vacuuming "public.pgbench_accounts"
INFO: "pgbench_accounts": removed 5 row versions in 8 pages
INFO: "pgbench_accounts": found 5 removable, 30000000 nonremovable
row versions in 491804 out of 491804 pages
DETAIL: 0 dead row versions cannot be removed yet, oldest xmin: 722
There were 0 unused item pointers.
Skipped 0 pages due to buffer pins, 0 frozen pages.
0 pages are entirely empty.
CPU: user: 4.20 s, system: 16.47 s, elapsed: 50.28 s.
VACUUM
Time: 50301.262 ms (00:50.301)

* FREEZE_ONLY
INFO: aggressively vacuuming "public.pgbench_accounts"
INFO: "pgbench_accounts": found 4 removable, 30000000 nonremovable
row versions in 491804 out of 491804 pages
DETAIL: freeze 30000000 rows
There were 0 unused item pointers.
Skipped 0 pages due to buffer pins, 0 frozen pages.
0 pages are entirely empty.
CPU: user: 3.10 s, system: 14.85 s, elapsed: 44.56 s.
VACUUM
Time: 44589.794 ms (00:44.590)

Feedback is very welcome.

Added to the next commit fest.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Thu, Oct 4, 2018 at 6:15 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Mon, Oct 1, 2018 at 7:20 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Hi,

Attached patch adds a new option FREEZE_ONLY to VACUUM command. This
option is same as FREEZE option except for it disables reclaiming dead
tuples. That is, with this option vacuum does pruning HOT chain,
freezing live tuples and maintaining both visibility map and freespace
map but does not collect dead tuples and invoke neither heap vacuum
nor index vacuum. This option will be useful if user wants to prevent
XID wraparound a table as quick as possible, especially when table is
quite large and is about to XID wraparound. I think this usecase was
mentioned in some threads but I couldn't find them.

Currently this patch just adds the new option to VACUUM command but it
might be good to make autovacuum use it when emergency vacuum is
required.

This is a performance-test result for FREEZE option and FREEZE_ONLY
option. I've tested them on the table which is about 3.8GB table
without indexes and randomly modified.

* FREEZE
INFO: aggressively vacuuming "public.pgbench_accounts"
INFO: "pgbench_accounts": removed 5 row versions in 8 pages
INFO: "pgbench_accounts": found 5 removable, 30000000 nonremovable
row versions in 491804 out of 491804 pages
DETAIL: 0 dead row versions cannot be removed yet, oldest xmin: 722
There were 0 unused item pointers.
Skipped 0 pages due to buffer pins, 0 frozen pages.
0 pages are entirely empty.
CPU: user: 4.20 s, system: 16.47 s, elapsed: 50.28 s.
VACUUM
Time: 50301.262 ms (00:50.301)

* FREEZE_ONLY
INFO: aggressively vacuuming "public.pgbench_accounts"
INFO: "pgbench_accounts": found 4 removable, 30000000 nonremovable
row versions in 491804 out of 491804 pages
DETAIL: freeze 30000000 rows
There were 0 unused item pointers.
Skipped 0 pages due to buffer pins, 0 frozen pages.
0 pages are entirely empty.
CPU: user: 3.10 s, system: 14.85 s, elapsed: 44.56 s.
VACUUM
Time: 44589.794 ms (00:44.590)

Feedback is very welcome.

Added to the next commit fest.

Rebaed to the current HEAD.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Hi,

On 10/1/18, 5:23 AM, "Masahiko Sawada" <sawada.mshk@gmail.com> wrote:

Attached patch adds a new option FREEZE_ONLY to VACUUM command. This
option is same as FREEZE option except for it disables reclaiming dead
tuples. That is, with this option vacuum does pruning HOT chain,
freezing live tuples and maintaining both visibility map and freespace
map but does not collect dead tuples and invoke neither heap vacuum
nor index vacuum. This option will be useful if user wants to prevent
XID wraparound a table as quick as possible, especially when table is
quite large and is about to XID wraparound. I think this usecase was
mentioned in some threads but I couldn't find them.

I've thought about this a bit myself. One of the reasons VACUUM can
take so long is because of all the index scans needed. If you're in a
potential XID wraparound situation and just need a quick way out, it
would be nice to have a way to do the minimum amount of work necessary
to reclaim transaction IDs. At a high level, I think there are some
improvements to this design we should consider.

1. Create a separate FREEZE command instead of adding a new VACUUM
option

The first line of the VACUUM documentation reads, "VACUUM reclaims
storage occupied by dead tuples," which is something that we would
explicitly not be doing with FREEZE_ONLY. I think it makes sense to
reuse many of the VACUUM code paths to implement this feature, but
from a user perspective, it should be separate.

2. We should reclaim transaction IDs from dead tuples as well

Unless we also have a way to freeze XMAX like we do XMIN, I doubt this
feature will be useful for the imminent-XID-wraparound use-case. In
short, we won't be able to advance relfrozenxid and relminmxid beyond
the oldest XMAX value for the relation. IIUC the idea of freezing
XMAX doesn't really exist yet. Either the XMAX is aborted/invalid and
can be reset to InvalidTransactionId, or it is committed and the tuple
can be removed if it beyond the freezing threshold. So, we probably
also want to look into adding a way to freeze XMAX, either by setting
it to FrozenTransactionId or by setting the hint bits to
(HEAP_XMAX_COMMITTED | HEAP_XMIN_INVALID) as is done for XMIN.

Looking closer, I see that the phrase "freezing XMAX" is currently
used to refer to setting it to InvalidTransactionId if it is aborted
or invalid (e.g. lock-only).

Currently this patch just adds the new option to VACUUM command but it
might be good to make autovacuum use it when emergency vacuum is
required.

This also seems like a valid use-case, but it should definitely be
done as a separate effort after this feature has been committed.

This is a performance-test result for FREEZE option and FREEZE_ONLY
option. I've tested them on the table which is about 3.8GB table
without indexes and randomly modified.

* FREEZE
...
Time: 50301.262 ms (00:50.301)

* FREEZE_ONLY
...
Time: 44589.794 ms (00:44.590)

I'd be curious to see the improvements you get when there are several
indexes on the relation. The ability to skip the index scans is
likely how this feature will really help speed things up.

Nathan

On Mon, Oct 1, 2018 at 6:23 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Attached patch adds a new option FREEZE_ONLY to VACUUM command. This
option is same as FREEZE option except for it disables reclaiming dead
tuples. That is, with this option vacuum does pruning HOT chain,
freezing live tuples and maintaining both visibility map and freespace
map but does not collect dead tuples and invoke neither heap vacuum
nor index vacuum. This option will be useful if user wants to prevent
XID wraparound a table as quick as possible, especially when table is
quite large and is about to XID wraparound. I think this usecase was
mentioned in some threads but I couldn't find them.

The feature seems like a reasonable one, but the name doesn't seem
like a good choice. It doesn't only freeze - e.g. it HOT-prunes, as
it must. Maybe consider something like (without_index_cleanup true)
or (index_cleanup false).

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Nov 2, 2018 at 1:32 AM Bossart, Nathan <bossartn@amazon.com> wrote:

Hi,

On 10/1/18, 5:23 AM, "Masahiko Sawada" <sawada.mshk@gmail.com> wrote:

Attached patch adds a new option FREEZE_ONLY to VACUUM command. This
option is same as FREEZE option except for it disables reclaiming dead
tuples. That is, with this option vacuum does pruning HOT chain,
freezing live tuples and maintaining both visibility map and freespace
map but does not collect dead tuples and invoke neither heap vacuum
nor index vacuum. This option will be useful if user wants to prevent
XID wraparound a table as quick as possible, especially when table is
quite large and is about to XID wraparound. I think this usecase was
mentioned in some threads but I couldn't find them.

Thank you for the comment!

I've thought about this a bit myself. One of the reasons VACUUM can
take so long is because of all the index scans needed. If you're in a
potential XID wraparound situation and just need a quick way out, it
would be nice to have a way to do the minimum amount of work necessary
to reclaim transaction IDs. At a high level, I think there are some
improvements to this design we should consider.

1. Create a separate FREEZE command instead of adding a new VACUUM
option

The first line of the VACUUM documentation reads, "VACUUM reclaims
storage occupied by dead tuples," which is something that we would
explicitly not be doing with FREEZE_ONLY.

No. Actually FREEZE_ONLY option (maybe will be changed its name) could
reclaim dead tuples by HOT-purning. If a page have HOT-updated chains
the FREEZE_ONLY prunes them and reclaim disk space occupied.

I think it makes sense to
reuse many of the VACUUM code paths to implement this feature, but
from a user perspective, it should be separate.

I'm concernced that since the existing users already have recognized
that vacuuming and freezing are closely related they would get
confused more if we have a similar purpose feature with different
name.

2. We should reclaim transaction IDs from dead tuples as well

Unless we also have a way to freeze XMAX like we do XMIN, I doubt this
feature will be useful for the imminent-XID-wraparound use-case. In
short, we won't be able to advance relfrozenxid and relminmxid beyond
the oldest XMAX value for the relation.
IIUC the idea of freezing> XMAX doesn't really exist yet. Either the XMAX is aborted/invalid and
can be reset to InvalidTransactionId, or it is committed and the tuple
can be removed if it beyond the freezing threshold. So, we probably
also want to look into adding a way to freeze XMAX, either by setting
it to FrozenTransactionId or by setting the hint bits to
(HEAP_XMAX_COMMITTED | HEAP_XMIN_INVALID) as is done for XMIN.

That's a good point. If the oldest xmax is close to the old
relfrozenxid we will not be able to advance relfrozenxid enough.
However, since dead tuples are vacuumed by autovacuum periodically I
think that we can advance relfrozenxid enough in common case. There is
possible that we eventually need to do vacuum with removing dead
tuples after done FREEZE_ONLY but it would be a rare case. Thought?

Looking closer, I see that the phrase "freezing XMAX" is currently
used to refer to setting it to InvalidTransactionId if it is aborted
or invalid (e.g. lock-only).

Currently this patch just adds the new option to VACUUM command but it
might be good to make autovacuum use it when emergency vacuum is
required.

This also seems like a valid use-case, but it should definitely be
done as a separate effort after this feature has been committed.

Agreed.

This is a performance-test result for FREEZE option and FREEZE_ONLY
option. I've tested them on the table which is about 3.8GB table
without indexes and randomly modified.

* FREEZE
...
Time: 50301.262 ms (00:50.301)

* FREEZE_ONLY
...
Time: 44589.794 ms (00:44.590)

I'd be curious to see the improvements you get when there are several
indexes on the relation. The ability to skip the index scans is
likely how this feature will really help speed things up.

I've tested performance of FREEZE option and FREEZE_ONLY option using
a 3GB table having 3 indexes. Before do vacuum I modified 1 % of data
on the table.

* FREEZE
Time: 78677.211 ms (01:18.677)
Time: 86958.452 ms (01:26.958)
Time: 78351.190 ms (01:18.351)

* FREEZE_ONLY
Time: 19913.863 ms (00:19.914)
Time: 18917.379 ms (00:18.917)
Time: 20048.541 ms (00:20.049)

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Fri, Nov 2, 2018 at 2:02 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Oct 1, 2018 at 6:23 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Attached patch adds a new option FREEZE_ONLY to VACUUM command. This
option is same as FREEZE option except for it disables reclaiming dead
tuples. That is, with this option vacuum does pruning HOT chain,
freezing live tuples and maintaining both visibility map and freespace
map but does not collect dead tuples and invoke neither heap vacuum
nor index vacuum. This option will be useful if user wants to prevent
XID wraparound a table as quick as possible, especially when table is
quite large and is about to XID wraparound. I think this usecase was
mentioned in some threads but I couldn't find them.

Thank you for the comment!

The feature seems like a reasonable one, but the name doesn't seem
like a good choice. It doesn't only freeze - e.g. it HOT-prunes, as
it must. Maybe consider something like (without_index_cleanup true)
or (index_cleanup false).

Adding a field-and-value style option might be worth. Or maybe we can
add one option for example freeze_without_index_cleanup?

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On 11/5/18, 2:07 AM, "Masahiko Sawada" <sawada.mshk@gmail.com> wrote:

On Fri, Nov 2, 2018 at 1:32 AM Bossart, Nathan <bossartn@amazon.com> wrote:

1. Create a separate FREEZE command instead of adding a new VACUUM
option

The first line of the VACUUM documentation reads, "VACUUM reclaims
storage occupied by dead tuples," which is something that we would
explicitly not be doing with FREEZE_ONLY.

No. Actually FREEZE_ONLY option (maybe will be changed its name) could
reclaim dead tuples by HOT-purning. If a page have HOT-updated chains
the FREEZE_ONLY prunes them and reclaim disk space occupied.

I see.

I think it makes sense to
reuse many of the VACUUM code paths to implement this feature, but
from a user perspective, it should be separate.

I'm concernced that since the existing users already have recognized
that vacuuming and freezing are closely related they would get
confused more if we have a similar purpose feature with different
name.

That seems reasonable to me. Perhaps decoupling this option from
FREEZE would make it clearer to users and easier to name. This would
allow users to do both VACUUM (WITHOUT_INDEX_CLEANUP) and VACUUM
(FREEZE, WITHOUT_INDEX_CLEANUP).

2. We should reclaim transaction IDs from dead tuples as well

Unless we also have a way to freeze XMAX like we do XMIN, I doubt this
feature will be useful for the imminent-XID-wraparound use-case. In
short, we won't be able to advance relfrozenxid and relminmxid beyond
the oldest XMAX value for the relation.
IIUC the idea of freezing> XMAX doesn't really exist yet. Either the XMAX is aborted/invalid and
can be reset to InvalidTransactionId, or it is committed and the tuple
can be removed if it beyond the freezing threshold. So, we probably
also want to look into adding a way to freeze XMAX, either by setting
it to FrozenTransactionId or by setting the hint bits to
(HEAP_XMAX_COMMITTED | HEAP_XMIN_INVALID) as is done for XMIN.

That's a good point. If the oldest xmax is close to the old
relfrozenxid we will not be able to advance relfrozenxid enough.
However, since dead tuples are vacuumed by autovacuum periodically I
think that we can advance relfrozenxid enough in common case. There is
possible that we eventually need to do vacuum with removing dead
tuples after done FREEZE_ONLY but it would be a rare case. Thought?

Given that a stated goal of this patch is to help recover from near
wraparound, I think this is very important optimization. It's true
that you might be able to advance relfrozenxid/relminmxid a bit in
some cases, but there are many others where you won't. For example,
if I create a row, delete it, and insert many more rows, my table's
XID age would be stuck at the row deletion. If I was in a situation
where this table was near wraparound and autovacuum wasn't keeping up,
I would run VACUUM (WITHOUT_INDEX_CLEANUP, FREEZE) with the intent of
reclaiming transaction IDs as fast as possible.

I'd be curious to see the improvements you get when there are several
indexes on the relation. The ability to skip the index scans is
likely how this feature will really help speed things up.

I've tested performance of FREEZE option and FREEZE_ONLY option using
a 3GB table having 3 indexes. Before do vacuum I modified 1 % of data
on the table.

* FREEZE
Time: 78677.211 ms (01:18.677)
Time: 86958.452 ms (01:26.958)
Time: 78351.190 ms (01:18.351)

* FREEZE_ONLY
Time: 19913.863 ms (00:19.914)
Time: 18917.379 ms (00:18.917)
Time: 20048.541 ms (00:20.049)

Nice.

Nathan

On Mon, Nov 5, 2018 at 3:12 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Adding a field-and-value style option might be worth. Or maybe we can
add one option for example freeze_without_index_cleanup?

That seems non-orthogonal. We have an existing flag to force freezing
(FREEZE); we don't need a second option that does the same thing.
Skipping the index scans (and thus necessarily the second heap pass)
is a separate behavior which we don't currently have a way to control.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon 5 Nov 2018, 12:49 Robert Haas <robertmhaas@gmail.com wrote:

That seems non-orthogonal. We have an existing flag to force freezing
(FREEZE); we don't need a second option that does the same thing.
Skipping the index scans (and thus necessarily the second heap pass)
is a separate behavior which we don't currently have a way to control.

It sounds like it might be better to name this "VACUUM (FAST)” and document
that it skips some of the normal (and necessary) work that vacuum does and
is only suitable for avoiding wraparounds and not sufficient for avoiding
bloat

On Mon, Nov 5, 2018 at 9:12 PM Greg Stark <stark@mit.edu> wrote:

It sounds like it might be better to name this "VACUUM (FAST)” and document that it skips some of the normal (and necessary) work that vacuum does and is only suitable for avoiding wraparounds and not sufficient for avoiding bloat

We could do that, but I don't see why that's better than VACUUM
(SKIP_INDEX_SCANS) or similar. There are, perhaps, multiple kinds of
shortcuts that could make vacuum run faster, but skipping index scans
is what it is.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Nov 6, 2018 at 2:48 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Nov 5, 2018 at 3:12 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Adding a field-and-value style option might be worth. Or maybe we can
add one option for example freeze_without_index_cleanup?

That seems non-orthogonal. We have an existing flag to force freezing
(FREEZE); we don't need a second option that does the same thing.
Skipping the index scans (and thus necessarily the second heap pass)
is a separate behavior which we don't currently have a way to control.

We already have disable_page_skipping option, not (page_skipping
false). So ISMT disable_index_cleanup would be more natural. Also,
since what to do with this option is not only skipping vacuum indexes
but also skipping removeing dead tuples on heap, I think that the
option should have a more understandable name for users indicating
that both it removes dead tuples less than the normal vacuum and it's
aimed to freeze tuple more faster. Of course we document them, though.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On 2018-Nov-08, Masahiko Sawada wrote:

On Tue, Nov 6, 2018 at 2:48 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Nov 5, 2018 at 3:12 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Adding a field-and-value style option might be worth. Or maybe we can
add one option for example freeze_without_index_cleanup?

That seems non-orthogonal. We have an existing flag to force freezing
(FREEZE); we don't need a second option that does the same thing.
Skipping the index scans (and thus necessarily the second heap pass)
is a separate behavior which we don't currently have a way to control.

We already have disable_page_skipping option, not (page_skipping
false). So ISMT disable_index_cleanup would be more natural. Also,
since what to do with this option is not only skipping vacuum indexes
but also skipping removeing dead tuples on heap, I think that the
option should have a more understandable name for users indicating
that both it removes dead tuples less than the normal vacuum and it's
aimed to freeze tuple more faster. Of course we document them, though.

I would name this based on the fact that it freezes quickly -- something
like FAST_FREEZE perhaps. The other things seem implementation details.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Nov 8, 2018 at 4:36 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

We already have disable_page_skipping option, not (page_skipping
false). So ISMT disable_index_cleanup would be more natural.

Sure.

Also,
since what to do with this option is not only skipping vacuum indexes
but also skipping removeing dead tuples on heap, I think that the
option should have a more understandable name for users indicating
that both it removes dead tuples less than the normal vacuum and it's
aimed to freeze tuple more faster. Of course we document them, though.

Well, I actually don't think that you should control two behaviors
with the same option. If you want to vacuum and skip index cleanup,
you should say VACUUM (disable_index_cleanup). If you want to vacuum,
disable index cleanup, and skip aggressively, you should say VACUUM
(freeze, disable_index_cleanup). Both behaviors seem useful.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Nov 9, 2018 at 2:56 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Nov 8, 2018 at 4:36 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

We already have disable_page_skipping option, not (page_skipping
false). So ISMT disable_index_cleanup would be more natural.

Sure.

Also,
since what to do with this option is not only skipping vacuum indexes
but also skipping removeing dead tuples on heap, I think that the
option should have a more understandable name for users indicating
that both it removes dead tuples less than the normal vacuum and it's
aimed to freeze tuple more faster. Of course we document them, though.

Well, I actually don't think that you should control two behaviors
with the same option. If you want to vacuum and skip index cleanup,
you should say VACUUM (disable_index_cleanup). If you want to vacuum,
disable index cleanup, and skip aggressively, you should say VACUUM
(freeze, disable_index_cleanup). Both behaviors seem useful.

Attached the updated version patch incorporated all comments I got.
The patch includes the following changes.

* Changed the option name to DISABLE_INDEX_CLENAUP. I agreed with
Robert and Nathan, having an option separated from FREEZE.

* 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.

The second change might conflict with the retail index deletion
patch[1]/messages/by-id/425db134-8bba-005c-b59d-56e50de3b41e@postgrespro.ru, which makes HOT-pruning *mark* tuples as dead (i.g., using
ItemIdMarkDead() instead). I think that this patch would not block
that patch but I've not considered deeply yet the combination with
these two pathes.

[1]: /messages/by-id/425db134-8bba-005c-b59d-56e50de3b41e@postgrespro.ru

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

On Thu, Jan 10, 2019 at 5:23 AM Bossart, Nathan <bossartn@amazon.com> wrote:

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.

Thank you for reviewing 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> 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.

Agreed. How about output a NOTICE message before calling
lazy_scan_heap() in lazy_vacuum_rel()?

+               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?

You're right. Will fix.

+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?

Yes, I intentionally separed them as I was concerned the these
functions have different assumptions and usage. But the combining them
also could work. Will change it.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Thu, Jan 10, 2019 at 2:45 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Thu, Jan 10, 2019 at 5:23 AM Bossart, Nathan <bossartn@amazon.com> wrote:

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.

Thank you for reviewing 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> 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.

Agreed. How about output a NOTICE message before calling
lazy_scan_heap() in lazy_vacuum_rel()?

+               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?

You're right. Will fix.

+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?

Yes, I intentionally separed them as I was concerned the these
functions have different assumptions and usage. But the combining them
also could work. Will change it.

Attached the updated patch. Please review it.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Fri, Jan 11, 2019 at 1:14 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Attached the updated patch. Please review it.

I'm quite confused by this patch. It seems to me that the easiest way
to implement this patch would be to (1) make lazy_space_alloc take the
maxtuples = MaxHeapTuplesPerPage branch when the new option is
specified, and then (2) forget about them after each page i.e.

if (nindexes == 0 &&
vacrelstats->num_dead_tuples > 0)
{
...
}
else if (skipping index cleanup)
vacrelstats->num_dead_tuples = 0;

I don't see why it should touch the logic inside lazy_vacuum_page() or
the decision about whether to truncate.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Sat, Jan 12, 2019 at 3:27 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jan 11, 2019 at 1:14 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Attached the updated patch. Please review it.

I'm quite confused by this patch. It seems to me that the easiest way
to implement this patch would be to (1) make lazy_space_alloc take the
maxtuples = MaxHeapTuplesPerPage branch when the new option is
specified, and then (2) forget about them after each page i.e.

if (nindexes == 0 &&
vacrelstats->num_dead_tuples > 0)
{
...
}
else if (skipping index cleanup)
vacrelstats->num_dead_tuples = 0;

I don't see why it should touch the logic inside lazy_vacuum_page() or
the decision about whether to truncate.

I think that because the tuples that got dead after heap_page_prune()
looked are recorded but not removed without lazy_vacuum_page() we need
to process them in lazy_vacuum_page(). For decision about whether to
truncate we should not change it, so I will fix it. It should be an
another option to control whether to truncate if we want.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center