On 2023-Apr-22, Andres Freund wrote:
> On 2023-04-13 13:18:38 +0200, Alvaro Herrera 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.
>
> My rewrite of what? The creation of TransactionIdRetreatSafely() in
> be504a3e974?
I meant the code that used to call TransactionIdRetreatSafely() and that
you're changing in the proposed patch.
> I'm afraid we'll need TransactionIdRetreatSafely() again, when we convert more
> things to 64bit xids (lest they end up with the same bug as fixed by
> be504a3e974), so it's perhaps worth thinking about how to make it less
> confusing.
The one thing that IMO makes it less confusing is to have it return the
value rather than modifying it in place.
> > <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>
>
> It seems a bit confusing now, because "by retaining only the number of
> segments ..." now also should cover hs_feedback (due to merging), but doesn't.
Hah, ok.
> I think I'll push the version I had. Then we can separately word-smith the
> section? Unless somebody protests I'm gonna do that soon.
No objection.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/