On Wed, Oct 29, 2025, at 17:48, Tom Lane wrote:
> However ... that won't actually work, the reason being that
> asyncQueueProcessPageEntries() doesn't work directly from an SLRU page
> but from a local copy. Even if it were to modify the state of that
> copy, no other backend would see the effects.
>
> The reason it's like that is stated in the comments:
>
> * The current page must have been fetched into page_buffer from shared
> * memory. (We could access the page right in shared memory, but that
> * would imply holding the SLRU bank lock throughout this routine.)
>
> The patch proposed here likewise appears to involve holding an SLRU
> bank lock throughout what could be a significant number of
> TransactionIdDidCommit tests. That seems like it could result in a
> pretty bad "burp" in NOTIFY throughput. That problem is ameliorated
> by only doing it when VACUUM is trying to advance datfrozenxid, but
> still I wonder if we can't find a less concurrency-unfriendly answer.
...
> All of this is a problem mainly because of the presumption that
> holding an SLRU bank lock for a long time is bad. I wonder how
> dangerous that really is.
Ops. Sounds scary.
I don't know if others have looked at the v12-vacuum_notify_queue_cleanup
approach; if it's bad, it would be helpful to understand why.
/Joel