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

From: Artur Zakirov <artur(dot)zakirov(at)adjust(dot)com>
To: Sergey Fedchenko <seregayoga(at)bk(dot)ru>
Cc: Daniel Danzberger <daniel(at)dd-wrt(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Sergei Kornilov <sk(at)zsrv(dot)org>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, Marc Dean <marc(dot)dean(dot)jr(at)gmail(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, "michael(dot)paul(dot)powers(at)gmail(dot)com" <michael(dot)paul(dot)powers(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Welin <robert(at)vaion(dot)com>
Subject: Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
Date: 2021-07-22 09:04:07
Message-ID: CAFt03JNo9=77=W6ggSjFRQ_h85BhrjNPArWd761k_KQye0HjCQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Hello hackers,

On Wed, Jul 14, 2021 at 11:30 AM Sergey Fedchenko <seregayoga(at)bk(dot)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 (tested with simple Go program using https://github.com/lib/pq). It would be nice to have this bug fixed. I’m not so familiar with 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 Content-Type Size
0001-signal-backends-on-commit.patch text/x-patch 5.0 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Pawel Kudzia 2021-07-22 09:07:16 Re: IRe: BUG #16792: silent corruption of GIN index resulting in SELECTs returning non-matching rows
Previous Message Heikki Linnakangas 2021-07-22 07:25:43 Re: IRe: BUG #16792: silent corruption of GIN index resulting in SELECTs returning non-matching rows

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Pyhalov 2021-07-22 09:13:54 Re: Case expression pushdown
Previous Message Fabien COELHO 2021-07-22 09:00:41 Re: psql - add SHOW_ALL_RESULTS option