Re: Optimize LISTEN/NOTIFY - Mailing list pgsql-hackers

On Oct 8, 2025, at 22:53, Joel Jacobson <joel@compiler.org> wrote:

1
```
+ channels = GetPendingNotifyChannels();
+
LWLockAcquire(NotifyQueueLock, LW_EXCLUSIVE);
- for (ProcNumber i = QUEUE_FIRST_LISTENER; i != INVALID_PROC_NUMBER; i 
= QUEUE_NEXT_LISTENER(i))
+ foreach(lc, channels)
```

I don’t see where “channels” is freed. GetPendingNotifyChannels() 
creates a list of Nodes, both the list and Nodes the list points to 
should be freed.

Per suggestion from Tom Lane I reverted back GetPendingNotifyChannels(),
so this comment is not applicable any longer.

5
```
/*
@@ -1865,6 +2087,7 @@ asyncQueueReadAllNotifications(void)
LWLockAcquire(NotifyQueueLock, LW_SHARED);
/* Assert checks that we have a valid state entry */
Assert(MyProcPid == QUEUE_BACKEND_PID(MyProcNumber));
+ QUEUE_BACKEND_WAKEUP_PENDING(MyProcNumber) = false;
```

This piece of code originally only read the shared memory, so it can 
use LW_SHARED lock mode, but now it writes to the shared memory, do we 
need to change the lock mode to “exclusive”?

No, LW_SHARED is sufficient here, since the backend only modifies its own state,
and no other backend could do that, without holding an exclusive lock.

6
```
+static inline void
+ChannelHashPrepareKey(ChannelHashKey *key, Oid dboid, const char *channel)
+{
+ memset(key, 0, sizeof(ChannelHashKey));
+ key->dboid = dboid;
+ strlcpy(key->channel, channel, NAMEDATALEN);
+}
```

Do we really need the memset()? If “channel” is of length NAMEDATALEN, 
then it still results in a non-0 terminated key->channel; if channel is 
shorter than NAMEDATALEN, strlcpy will auto add a tailing ‘\0’. I think 
previous code should have ensured length of channel should be less than 
NAMEDATALEN.

Yes, I think we need memset, since I fear that when the hash table keys
are compared, every byte of the struct might be inspected, so without
zero-initializing it, there could be unused bytes after the null
terminator, that could then cause logically identical keys to be wrongly
considered different.

I haven't checked the implementation though, but my gut feeling says
it's better to be a bit paranoid here.