Hi, On 2023-03-23 10:18:35 +0100, Daniel Gustafsson wrote: > > On 22 Mar 2023, at 18:00, Andres Freund <and...@anarazel.de> wrote: > > > It wasn't actually that much work to write a patch to remove > > vacuum_defer_cleanup_age, see the attached. > > - and <xref linkend="guc-vacuum-defer-cleanup-age"/> provide protection > against > + provides 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 > > Isn't "the latter" in the kept part of the sentence referring to the guc we're > removing here?
You're right. That paragraph generally seems a bit off. In HEAD: <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, whereas replication slots retain 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> <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. </para> Replication slots alone don't prevent row removal, that requires hot_standby_feedback to be used as well. A minimal rephrasing would be: <para> 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. </para> > - * It's possible that slots / vacuum_defer_cleanup_age backed up the > - * horizons further than oldest_considered_running. Fix. > + * It's possible that slots backed up the horizons further than > + * oldest_considered_running. Fix. > > While not the fault of this patch, can't we use the opportunity to expand > "Fix." to a short "Fix this by ..." sentence? Or remove "Fix." perhaps, > either > of those would improve the comment IMHO. Perhaps unsurprisingly, given that I wrote that comment, I don't see a problem with the "Fix."... Greetings, Andres Freund