On Sat, Mar 18, 2023 at 12:09 AM Andres Freund <andres@anarazel.de> wrote:
Hi,
As evidenced by the bug fixed in be504a3e974, vacuum_defer_cleanup_age is not heavily used - the bug was trivial to hit as soon as vacuum_defer_cleanup_age is set to a non-toy value. It complicates thinking about visibility horizons substantially, as vacuum_defer_cleanup_age can make them go backward substantially. Obviously it's also severely undertested.
I started writing a test for vacuum_defer_cleanup_age while working on the fix referenced above, but now I am wondering if said energy would be better spent removing vacuum_defer_cleanup_age alltogether.
vacuum_defer_cleanup_age was added as part of hot standby. Back then we did not yet have hot_standby_feedback. Now that that exists, vacuum_defer_cleanup_age doesn't seem like a good idea anymore. It's pessimisistic, i.e. always retains rows, even if none of the standbys has an old enough snapshot.
The only benefit of vacuum_defer_cleanup_age over hot_standby_feedback is that it provides a limit of some sort. But transactionids aren't producing dead rows in a uniform manner, so limiting via xid isn't particularly useful. And even if there are use cases, it seems those would be served better by introducing a cap on how much hot_standby_feedback can hold the horizon back.
I don't think I have the cycles to push this through in the next weeks, but if we agree removing vacuum_defer_cleanup_age is a good idea, it seems like a good idea to mark it as deprecated in 16?
+1. I haven't seen any (correct) use of this in many many years on my end at least.
And yes, having a cap on hot_standby_feedback would also be great.
