Re: Should we remove vacuum_defer_cleanup_age?

On 2023-Apr-11, Andres Freund wrote:

> Updated patch attached. I think we should either apply something like that
> patch, or at least add a <warning/> to the docs.

I gave this patch a look. The only code change is that
ComputeXidHorizons() and GetSnapshotData() no longer handle the case
where vacuum_defer_cleanup_age is different from zero. It looks good.
The TransactionIdRetreatSafely() code being removed is pretty weird (I
spent a good dozen minutes writing a complaint that your rewrite was
faulty, but it turns out I had misunderstood the function), so I'm glad
it's being retired.

> <para>
> - Similarly, <xref linkend="guc-hot-standby-feedback"/>
> - and <xref linkend="guc-vacuum-defer-cleanup-age"/> provide protection against
> - relevant rows being removed by vacuum, but the former provides no
> - protection during any time period when the standby is not connected,
> - and the latter often needs to be set to a high value to provide adequate
> - protection. Replication slots overcome these disadvantages.
> + Similarly, <xref linkend="guc-hot-standby-feedback"/> on its own, without
> + also using a replication slot, provides protection against relevant rows
> + being removed by vacuum, but provides no protection during any time period
> + when the standby is not connected. Replication slots overcome these
> + disadvantages.

I think it made sense to have this paragraph be separate from the
previous one when it was talking about two separate variables, but now
that it's just one, it looks a bit isolated. I would merge it with the
one above, which is talking about pretty much the same thing, and
reorder the whole thing approximately like this

<para>
In lieu of using replication slots, it is possible to prevent the removal
of old WAL segments using <xref linkend="guc-wal-keep-size"/>, or by
storing the segments in an archive using
<xref linkend="guc-archive-command"/> or <xref linkend="guc-archive-library"/>.
However, these methods often result in retaining more WAL segments than
required.
Similarly, <xref linkend="guc-hot-standby-feedback"/> without
a replication slot provides protection against relevant rows
being removed by vacuum, but provides no protection during any time period
when the standby is not connected.
</para>
<para>
Replication slots overcome these disadvantages by retaining only the number
of segments known to be needed.
On the other hand, replication slots can retain so
many WAL segments that they fill up the space allocated
for <literal>pg_wal</literal>;
<xref linkend="guc-max-slot-wal-keep-size"/> limits the size of WAL files
retained by replication slots.
</para>

Though the "However," looks a poor fit; I would do this:

<para>
In lieu of using replication slots, it is possible to prevent the removal
of old WAL segments using <xref linkend="guc-wal-keep-size"/>, or by
storing the segments in an archive using
<xref linkend="guc-archive-command"/> or <xref linkend="guc-archive-library"/>.
A disadvantage of these methods is that they often result in retaining
more WAL segments than required.
Similarly, <xref linkend="guc-hot-standby-feedback"/> without
a replication slot provides protection against relevant rows
being removed by vacuum, but provides no protection during any time period
when the standby is not connected.
</para>
<para>
Replication slots overcome these disadvantages by retaining only the number
of segments known to be needed.
On the other hand, replication slots can retain so
many WAL segments that they fill up the space allocated
for <literal>pg_wal</literal>;
<xref linkend="guc-max-slot-wal-keep-size"/> limits the size of WAL files
retained by replication slots.
</para>

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Tom: There seems to be something broken here.
Teodor: I'm in sackcloth and ashes... Fixed.
http://postgr.es/m/482D1632.8010507@sigaev.ru