From: Tom Lane <tgl@sss.pgh.pa.us>
To: Artur Zakirov <artur.zakirov@adjust.com>
Cc: Sergey Fedchenko <seregayoga@bk.ru>
Cc: Daniel Danzberger <daniel@dd-wrt.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Sergei Kornilov <sk@zsrv.org>
Cc: pgsql-hackers@postgresql.org, Peter Eisentraut <peter_e@gmx.net>
Cc: Marc Dean <marc.dean.jr@gmail.com>
Cc: Petr Jelinek <petr@2ndquadrant.com>
Cc: michael.paul.powers@gmail.com <michael.paul.powers@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Robert Welin <robert@vaion.com>
Subject: Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
Date: Sat, 11 Sep 2021 18:16:09 -0400
Message-ID: <170808.1631398569@sss.pgh.pa.us> (raw)
In-Reply-To: <CAFt03JMY=769Vst0nipzSTpiXG3HBf5m42cGxZqafwiO0uWDYA@mail.gmail.com>
References: <20190628212224.GA11418@alvherre.pgsql>
<923262c473b494c4aec825a360d6f0499c144552.camel@dd-wrt.com>
<1623669444.237579337@f317.i.mail.ru>
<CAFt03JNo9=77=W6ggSjFRQ_h85BhrjNPArWd761k_KQye0HjCQ@mail.gmail.com>
<3113945.1630956452@sss.pgh.pa.us>
<CAFt03JMY=769Vst0nipzSTpiXG3HBf5m42cGxZqafwiO0uWDYA@mail.gmail.com>
Artur Zakirov <artur.zakirov@adjust.com> writes:
> On Mon, Sep 6, 2021 at 9:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hm. So that forecloses back-patching this to earlier than v13.
>> On the other hand, given that we've been ignoring the bug for awhile,
>> maybe a fix that only works in v13+ is good enough. (Or maybe by now
>> it'd be safe to back-patch the v13-era async.c changes? Don't really
>> want to, though.)
> I think it would be better to back-patch the fix to older versions
> than v13. But considering that the new patch renames
> ProcessCompletedNotifies(), it can break existing applications which
> use background workers and NOTIFY. Users can be upset with the change,
> since they don't face the original bug, they just call
> ProcessCompletedNotifies() manually.
I've not looked at your new patch yet, but I did spend a little time
checking into whether it'd be sane to backpatch 51004c717, which
changed SignalBackends' API. (We'd also need the non-catalog parts
of 8b7ae5a82, presumably.) I came away feeling that the benefit
isn't worth the effort and risk. There was an awful lot of churn
in async.c since v12, and to make matters worse, some of it was
back-patched already while some wasn't. So 51004c717 doesn't apply
to v12 at all cleanly, and some of the merge problems are due to
code that's older than that commit but others are due to code that's
newer. It would take some work just to get a patch that applies,
and I would not have a lot of faith in the result. I think we're
better off just settling for a back-patch to v13. The delta in
async.c between v13 and HEAD is not very large, so we'd not be
taking too much risk in going that far back.
As far as ProcessCompletedNotifies is concerned, I think what we
could do in the back branches is leave it in place as an empty,
no-op function, so as not to break extensions that are following
the existing documentation by calling it.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, artur.zakirov@adjust.com, seregayoga@bk.ru, daniel@dd-wrt.com, andres@anarazel.de, sk@zsrv.org, peter_e@gmx.net, marc.dean.jr@gmail.com, petr@2ndquadrant.com, michael.paul.powers@gmail.com, alvherre@2ndquadrant.com, robert@vaion.com
Subject: Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
In-Reply-To: <170808.1631398569@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox