Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events - Mailing list pgsql-hackers

From Artur Zakirov
Subject Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
Date
Msg-id CAFt03JNo9=77=W6ggSjFRQ_h85BhrjNPArWd761k_KQye0HjCQ@mail.gmail.com
Whole thread Raw
In response to Re: BUG #15293: Stored Procedure Triggered by Logical Replication isUnable to use Notification Events  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
List pgsql-hackers
Hello hackers,

On Wed, Jul 14, 2021 at 11:30 AM Sergey Fedchenko <seregayoga@bk.ru> wrote:
>
> Hi all! I still can reproduce it on 14beta1 version. I adapted a patch I found in this thread
https://github.com/seregayoga/postgres/commit/338bc33f2cf77edde7c45bfdfb9f39a92ec57eb8. It solved this bug for me
(testedwith simple Go program using https://github.com/lib/pq). It would be nice to have this bug fixed. I’m not so
familiarwith postgres code base, but would glad to help with testing. 

In our company in one of our projects we ran into this bug too.

I attached the patch which fixes it in a different way. It calls
SignalBackends() in AtCommit_Notify(). It is possible to call SignalBackends()
outside of ProcessCompletedNotifies() after the commit
51004c7172b5c35afac050f4d5849839a06e8d3b, which removes necessity of checking
of SignalBackends()'s result.

Moving SignalBackends() to AtCommit_Notify() makes it possible to signal
backends by "non-normal" backends including logical replication workers. It
seems it is safe to call SignalBackends() in AtCommit_Notify() since
SignalBackends() doesn't raise any error except OOM.

Regarding Andres concern:
> what I am wondering is what happens if there's a background worker (like
> the replication worker, but it could be other things too) that queues
> notifications, but no normal backends are actually listening. As far as
> I can tell, in that case we'd continue to queue stuff into the slru, but
> wouldn't actually clean things up until a normal session gets around to
> it? Which might be a while, on e.g. a logical replica.

A backend which raises notifications calls asyncQueueAdvanceTail() sometimes,
which truncates pages up to new tail, which is equal to head if there are no
listening backends. But there will be a problem if there is a backend which
is listening but it doesn't process incoming notifications and doesn't update
its queue position. In that case asyncQueueAdvanceTail() is able to advance
tail only up to that backend's queue position.

--
Artur Zakirov
PostgreSQL Developer at Adjust

Attachment

pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: psql - add SHOW_ALL_RESULTS option
Next
From: Alexander Pyhalov
Date:
Subject: Re: Case expression pushdown