Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events

Artur Zakirov <artur.zakirov@adjust.com> writes:
> [ v2-0001-signal-backends-on-commit.patch ]

I had an epiphany while looking at this.  Now that PostgresMain
calls ProcessNotifyInterrupt at the same place it calls
ProcessCompletedNotifies (which it does since 790026972), we don't
actually need ProcessCompletedNotifies to read self-notifies either.
If we merely set notifyInterruptPending, ProcessNotifyInterrupt will
handle that just fine.  With the other changes already under discussion,
this means ProcessCompletedNotifies can go away entirely.

That's not only less code, but fewer cycles: in cases where we have both
self-notifies and inbound notify signals, the existing code starts two
transactions and runs asyncQueueReadAllNotifications twice, but there's
no need to do it more than once.  Self-notifies become less of a special
case on the sending side too, since we can just treat that as signalling
ourselves --- though it still seems worthwhile to optimize that by
setting notifyInterruptPending directly instead of invoking kill().

Hence, I present the attached, which also tweaks things to avoid an
extra pq_flush in the end-of-command code path, and improves the
comments to discuss the issue of NOTIFYs sent by procedures.

There is still a loose end we ought to think about: what to do when
someone issues LISTEN in a background worker.  With the code as
it stands, or with this patch, the worker will block cleanout of
the async SLRU since it will never read any messages.  (With
the code as it stands, a bgworker author can ameliorate that by
calling ProcessCompletedNotifies, but this patch is going to either
eliminate ProcessCompletedNotifies or turn it into a no-op.  In
any case, we still have a problem if an ill-considered trigger
issues LISTEN in a replication worker.)

I'm inclined to think we should flat-out reject LISTEN in any process
that is not attached to a frontend, at least until somebody takes the
trouble to add infrastructure that would let it be useful.  I've not
done that here though; I'm not quite sure what we should test for.

			regards, tom lane