On Thu, Oct 2, 2025, at 18:39, Tom Lane wrote:
> I don't understand why you feel you need a bgworker. The existing
> code does not have any provision that guarantees a lost signal will
> eventually be re-sent --- it will be if there is continuing NOTIFY
> traffic, but not if all the senders suddenly go quiet. AFAIR
> we've had zero complaints about that in 25+ years. So I'm perfectly
> content to continue the approach of "check for laggards during
> NOTIFY". (This could be gated behind an overall check on how long the
> notify queue is, so that we don't expend the cycles when things are
> performing as-expected.) If you feel that that's not robust enough,
> you should split it out as a separate patch that's advertised as a
> robustness improvement not a performance improvement, and see if you
> can get anyone to bite.
Good point. I agree it's better to check for laggards during NOTIFY.
> The other thing I'm concerned about with this patch is the new shared
> hash table. I don't think we have anywhere near a good enough fix on
> how big it needs to be, and that is problematic because of the
> frozen-at-startup size of main shared memory. We could imagine
> inventing YA GUC to let the user tell us how big to make it,
> but I think there is now a better way: use a dshash table
> (src/backend/lib/dshash.c). That offers the additional win that we
> don't have to create it at all in an installation that never uses
> LISTEN/NOTIFY. We could also rethink whether we really need the
> NOTIFY_MULTICAST_THRESHOLD limit: rather than having two code paths,
> we could just say that all listeners are registered for every channel.
Thanks for guidance, I didn't know about dshash.
The patch is now using dshash. I've been looking at code in launcher.c
when implementing it. The function init_channel_hash() ended up being
very similar to launcher.c's logicalrep_launcher_attach_dshmem().
/Joel
