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
