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/
