On Sat, Dec 27, 2025, at 13:40, Joel Jacobson wrote:
> On Fri, Dec 26, 2025, at 21:12, Joel Jacobson wrote:
>> On Tue, Nov 25, 2025, at 21:17, Tom Lane wrote:
>>> "Joel Jacobson" <[email protected]> writes:
>>>> It looks to me like it would be best with two boolean fields; one
>>>> boolean to stage the updates during PreCommit_Notify, that each
>>>> pendingActions could flip back and forth, and another boolean that
>>>> represents the current value, which we would overwrite with the staged
>>>> value during AtCommit_Notify.
>>>
>>> +1, I had a feeling that a single boolean wouldn't quite do it.
>>> (There are various ways we could define the states, but what
>>> you say above seems pretty reasonable.)
>>
>> I've implemented the two boolean approach and think it's good.

I've reworked the staging mechanism for LISTEN/UNLISTEN. The new design
tracks LISTEN state at three levels:

* pendingListenChannels: per-transaction pending changes
* listenChannelsHash: per-backend committed state cache
* channelHash: cluster-wide shared state

The first two are local hash tables, the third is a dshash in shared
memory.  PreCommit_Notify updates all three (doing any allocations
before clog commit for OOM safety), and AtCommit_Notify finalizes the
changes.

The previous version tried to track pending state in the shared
ListenerEntry itself using two booleans (staged/current).  This worked,
but I think the three-layer approach is cleaner.

The main benefit is that pendingListenChannels is now a hash table
instead of a simple List.  In the old design, LISTEN foo; UNLISTEN foo;
LISTEN foo would create three list entries that all had to be processed
at commit.  The new design collapses this to one hash entry storing the
final state, which we just apply at commit.

A nice bonus is that UNLISTEN became simpler.  In PreCommit_Notify it
just records the intent in the local pending hash.  The old design had
to acquire an exclusive lock on the shared dshash entry to flip the
staged boolean.  UNLISTEN ALL is similar -- it now just scans the
backend's own local hashes instead of the cluster-wide shared hash.

The tradeoff is one additional local hash table per transaction that
executes LISTEN/UNLISTEN.  This seems like a reasonable price for the
simpler logic.

I also renamed a few things for clarity: ChannelEntry is now
ChannelListeners (since it holds the array of listeners for a channel),
and channelHashtab is now channelSet (since it's just a set of channel
names, not a hash of channel-related data).

/Joel

Attachment: 0001-optimize_listen_notify-v33.patch
Description: Binary data

Attachment: 0002-optimize_listen_notify-v33.patch
Description: Binary data

Reply via email to