From: Andres Freund <andres@anarazel.de>
To: Sergei Kornilov <sk@zsrv.org>
To: pgsql-hackers@postgresql.org, Peter Eisentraut <peter_e@gmx.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
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: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
Date: Tue, 24 Jul 2018 12:27:40 -0700
Message-ID: <20180724192740.6yn2xregaz6yhpc2@alap3.anarazel.de> (raw)
In-Reply-To: <648481532456538@myt5-0fa3e541b8b9.qloud-c.yandex.net>
References: <153243441449.1404.2274116228506175596@wrigleys.postgresql.org>
<45136061532438385@iva3-a2a8aac410a1.qloud-c.yandex.net>
<20180724155829.zq22vov6d7erhxsx@alap3.anarazel.de>
<CAEU75wAdO7_t6MzSqWxua4Cca+KKpuHYYz6txz=VgETX--yYvw@mail.gmail.com>
<648481532456538@myt5-0fa3e541b8b9.qloud-c.yandex.net>
Hi Tom, Peter,
On 2018-07-24 21:22:18 +0300, Sergei Kornilov wrote:
> in fact, I've already tried to build fix. Adding ProcessCompletedNotifies to apply_handle_commit fixed this issue and i think this is right place. In src/backend/tcop/postgres.c we call ProcessCompletedNotifies similar way after commit. This change pass make check-world.
> So i attach my two line patch.
> diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
> index 6ca6cdc..e54bd90 100644
> --- a/src/backend/replication/logical/worker.c
> +++ b/src/backend/replication/logical/worker.c
> @@ -37,6 +37,7 @@
>
> #include "commands/tablecmds.h"
> #include "commands/trigger.h"
> +#include "commands/async.h"
>
> #include "executor/executor.h"
> #include "executor/nodeModifyTable.h"
> @@ -490,6 +491,7 @@ apply_handle_commit(StringInfo s)
> replorigin_session_origin_timestamp = commit_data.committime;
>
> CommitTransactionCommand();
> + ProcessCompletedNotifies();
> pgstat_report_stat(false);
>
> store_flush_position(commit_data.end_lsn);
That's probably reasonable for the back branches (although I'd put the
store_flush_position before).
But I wonder if we shouldn't actually move the signalling part of
ProcessCompletedNotifies() into CommitTransactionCommand() in v11. Given
that transactions can now commit without a ready command being sent, due
to the addition of procedures, that kind of seems necessary?
Greetings,
Andres Freund
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: andres@anarazel.de, sk@zsrv.org, peter_e@gmx.net, tgl@sss.pgh.pa.us, marc.dean.jr@gmail.com, petr@2ndquadrant.com, michael.paul.powers@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
In-Reply-To: <20180724192740.6yn2xregaz6yhpc2@alap3.anarazel.de>
* 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