Обсуждение: BUG #16481: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
BUG #16481: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
От
PG Bug reporting form
Дата:
The following bug has been logged on the website:
Bug reference: 16481
Logged by: Fabio Vianello
Email address: fabio.vianello@salvagninigroup.com
PostgreSQL version: 12.3
Operating system: Windows 10
Description:
About the bug BUG #15293, on PostgreSQL version 10.4 and 11.2 as describe
below, we found the same issue on the PostgreSQL version 12.3.
Do you think to solve the issue?
Is it a feature?
Becasue in the documentation we didn't found any constraint that says that
we can not use NOTIFY/LISTEN on logical replication tables.
"When using logical replication a stored procedure executed on the replica
is
unable to use NOTIFY to send messages to other listeners. The stored
procedure does execute as expected but the pg_notify() doesn't appear to
have any effect. If an insert is run on the replica side the trigger
executes the stored procedure as expected and the NOTIFY correctly
notifies
listeners.
Steps to Reproduce:
Set up Master:
CREATE TABLE test (id SERIAL PRIMARY KEY, msg TEXT NOT NULL);
CREATE PUBLICATION testpub FOR TABLE test;
Set up Replica:
CREATE TABLE test (id SERIAL PRIMARY KEY, msg TEXT NOT NULL);
CREATE SUBSCRIPTION testsub CONNECTION 'host=192.168.0.136 user=test
password=test' PUBLICATION testpub;
CREATE OR REPLACE FUNCTION notify_channel() RETURNS trigger AS $$
BEGIN
RAISE LOG 'Notify Triggered';
PERFORM pg_notify('testchannel', 'Testing');
RETURN NEW;
END;
$$ LANGUAGE 'plpgsql';
DROP TRIGGER queue_insert ON TEST;
CREATE TRIGGER queue_insert AFTER INSERT ON test FOR EACH ROW EXECUTE
PROCEDURE notify_channel();
ALTER TABLE test ENABLE ALWAYS TRIGGER queue_insert;
LISTEN testchannel;
Run the following insert on the master:
INSERT INTO test (msg) VALUES ('test');
In postgresql-10-main.log I get the following:
2018-07-24 07:45:15.705 EDT [6701] LOG: 00000: Notify Triggered
2018-07-24 07:45:15.705 EDT [6701] CONTEXT: PL/pgSQL function
notify_channel() line 3 at RAISE
2018-07-24 07:45:15.705 EDT [6701] LOCATION: exec_stmt_raise,
pl_exec.c:3337
But no listeners receive the message. However if an insert is run directly
on the replica:
INSERT INTO test VALUES (99999, 'test');
INSERT 0 1
Asynchronous notification "testchannel" with payload "Testing" received
from
server process with PID 6701.
Asynchronous notification "testchannel" with payload "Testing" received
from
server process with PID 6701.
Asynchronous notification "testchannel" with payload "Testing" received
from
server process with PID 6701.
Asynchronous notification "testchannel" with payload "Testing" received
from
server process with PID 6701.
Asynchronous notification "testchannel" with payload "Testing" received
from
server process with PID 6701.
Asynchronous notification "testchannel" with payload "Testing" received
from
server process with PID 9992.
Backed up notifications are received for previous NOTIFY's."
Re: BUG #16481: Stored Procedure Triggered by Logical Replicationis Unable to use Notification Events
От
Kyotaro Horiguchi
Дата:
Hello.
It seems to me a bug.
At Fri, 05 Jun 2020 11:05:14 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in
> The following bug has been logged on the website:
>
> Bug reference: 16481
> Logged by: Fabio Vianello
> Email address: fabio.vianello@salvagninigroup.com
> PostgreSQL version: 12.3
> Operating system: Windows 10
> Description:
>
> About the bug BUG #15293, on PostgreSQL version 10.4 and 11.2 as describe
> below, we found the same issue on the PostgreSQL version 12.3.
The HEAD behaves the same way.
> Is it a feature?
> Becasue in the documentation we didn't found any constraint that says that
> we can not use NOTIFY/LISTEN on logical replication tables.
>
> "When using logical replication a stored procedure executed on the replica
> is
> unable to use NOTIFY to send messages to other listeners. The stored
> procedure does execute as expected but the pg_notify() doesn't appear to
> have any effect. If an insert is run on the replica side the trigger
> executes the stored procedure as expected and the NOTIFY correctly
> notifies
> listeners.
The message is actually queued, but logical replication worker doesn't
signal that to listener backends. If any ordinary session sent a
message to the same listener after that, both messages would be shown
at once.
That can be fixed by calling ProcessCompletedNotifies() in
apply_handle_commit. The function has a code to write out
notifications to connected clients but it doesn't nothing on logical
replication workers.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From b8df8c0cf6ae6c2bcd78ffd7d9bd629f51ab3bee Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>
Date: Mon, 8 Jun 2020 16:07:41 +0900
Subject: [PATCH] Signal notifications from logical replication workers
Notifications need to be signaled to listeners but logical replication
worker forgets to do that. Fix that by signaling notifications after
committing a transaction.
---
src/backend/replication/logical/worker.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index a752a1224d..28ae89c574 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -33,6 +33,7 @@
#include "catalog/pg_inherits.h"
#include "catalog/pg_subscription.h"
#include "catalog/pg_subscription_rel.h"
+#include "commands/async.h"
#include "commands/tablecmds.h"
#include "commands/trigger.h"
#include "executor/executor.h"
@@ -517,6 +518,9 @@ apply_handle_commit(StringInfo s)
pgstat_report_stat(false);
store_flush_position(commit_data.end_lsn);
+
+ /* Send out notify signals */
+ ProcessCompletedNotifies();
}
else
{
--
2.18.2
Re: BUG #16481: Stored Procedure Triggered by Logical Replicationis Unable to use Notification Events
От
Kyotaro Horiguchi
Дата:
Hello.
It seems to me a bug.
At Fri, 05 Jun 2020 11:05:14 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in
> The following bug has been logged on the website:
>
> Bug reference: 16481
> Logged by: Fabio Vianello
> Email address: fabio.vianello@salvagninigroup.com
> PostgreSQL version: 12.3
> Operating system: Windows 10
> Description:
>
> About the bug BUG #15293, on PostgreSQL version 10.4 and 11.2 as describe
> below, we found the same issue on the PostgreSQL version 12.3.
The HEAD behaves the same way.
> Is it a feature?
> Becasue in the documentation we didn't found any constraint that says that
> we can not use NOTIFY/LISTEN on logical replication tables.
>
> "When using logical replication a stored procedure executed on the replica
> is
> unable to use NOTIFY to send messages to other listeners. The stored
> procedure does execute as expected but the pg_notify() doesn't appear to
> have any effect. If an insert is run on the replica side the trigger
> executes the stored procedure as expected and the NOTIFY correctly
> notifies
> listeners.
The message is actually queued, but logical replication worker doesn't
signal that to listener backends. If any ordinary session sent a
message to the same listener after that, both messages would be shown
at once.
That can be fixed by calling ProcessCompletedNotifies() in
apply_handle_commit. The function has a code to write out
notifications to connected clients but it doesn't nothing on logical
replication workers.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From b8df8c0cf6ae6c2bcd78ffd7d9bd629f51ab3bee Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>
Date: Mon, 8 Jun 2020 16:07:41 +0900
Subject: [PATCH] Signal notifications from logical replication workers
Notifications need to be signaled to listeners but logical replication
worker forgets to do that. Fix that by signaling notifications after
committing a transaction.
---
src/backend/replication/logical/worker.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index a752a1224d..28ae89c574 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -33,6 +33,7 @@
#include "catalog/pg_inherits.h"
#include "catalog/pg_subscription.h"
#include "catalog/pg_subscription_rel.h"
+#include "commands/async.h"
#include "commands/tablecmds.h"
#include "commands/trigger.h"
#include "executor/executor.h"
@@ -517,6 +518,9 @@ apply_handle_commit(StringInfo s)
pgstat_report_stat(false);
store_flush_position(commit_data.end_lsn);
+
+ /* Send out notify signals */
+ ProcessCompletedNotifies();
}
else
{
--
2.18.2
Hi Kyotaro Horiguchi, thanks for you helps. We have a question about the bug. Why there isn't any solution in the HEAD? This bug last since 10.4 version and I can't understand why it is not fixed in the HEAD yet. BR. Fabio Vianello. -----Original Message----- From: Kyotaro Horiguchi [mailto:horikyota.ntt@gmail.com] Sent: lunedì 8 giugno 2020 10:28 To: Vianello Fabio <fabio.vianello@salvagninigroup.com>; pgsql-bugs@lists.postgresql.org; pgsql-hackers@lists.postgresql.org Subject: Re: BUG #16481: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events Hello. It seems to me a bug. At Fri, 05 Jun 2020 11:05:14 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in > The following bug has been logged on the website: > > Bug reference: 16481 > Logged by: Fabio Vianello > Email address: fabio.vianello@salvagninigroup.com > PostgreSQL version: 12.3 > Operating system: Windows 10 > Description: > > About the bug BUG #15293, on PostgreSQL version 10.4 and 11.2 as > describe below, we found the same issue on the PostgreSQL version 12.3. The HEAD behaves the same way. > Is it a feature? > Becasue in the documentation we didn't found any constraint that says > that we can not use NOTIFY/LISTEN on logical replication tables. > > "When using logical replication a stored procedure executed on the > replica is unable to use NOTIFY to send messages to other listeners. > The stored procedure does execute as expected but the pg_notify() > doesn't appear to have any effect. If an insert is run on the replica > side the trigger executes the stored procedure as expected and the > NOTIFY correctly notifies listeners. The message is actually queued, but logical replication worker doesn't signal that to listener backends. If any ordinarysession sent a message to the same listener after that, both messages would be shown at once. That can be fixed by calling ProcessCompletedNotifies() in apply_handle_commit. The function has a code to write out notificationsto connected clients but it doesn't nothing on logical replication workers. regards. -- Kyotaro Horiguchi NTT Open Source Software Center SALVAGNINI ITALIA S.p.A. Via Guido Salvagnini, 51 - IT - 36040 Sarego (VI) T. +39 0444 725111 | F. +39 0444 43 6404 Società a socio unico - Attività direz. e coord.: Salvagnini Holding S.p.A. Clicca qui<https://www.salvagninigroup.com/company-information> per le informazioni societarie salvagninigroup.com<https://www.salvagninigroup.com> | salvagnini.it<http://www.salvagnini.it> Le informazioni trasmesse sono destinate esclusivamente alla persona o alla società in indirizzo e sono da intendersi confidenzialie riservate. Ogni trasmissione, inoltro, diffusione o altro uso di queste informazioni a persone o società differentidal destinatario è proibita. Se avete ricevuto questa comunicazione per errore, per favore contattate il mittentee cancellate le informazioni da ogni computer. Questa casella di posta elettronica è riservata esclusivamente all’invioed alla ricezione di messaggi aziendali inerenti all’attività lavorativa e non è previsto né autorizzato l’utilizzoper fini personali. Pertanto i messaggi in uscita e quelli di risposta in entrata verranno trattati quali messaggiaziendali e soggetti alla ordinaria gestione disposta con proprio disciplinare dall’azienda e, di conseguenza, eventualmenteanche alla lettura da parte di persone diverse dall’intestatario della casella. Any information herein transmitted only concerns the person or the company named in the address and is deemed to be confidential.It is strictly forbidden to transmit, post, forward or otherwise use said information to anyone other than therecipient. If you have received this message by mistake, please contact the sender and delete any relevant informationfrom your computer. This mailbox is only meant for sending and receiving messages pertaining business mattersand any other use for personal purposes is forbidden and unauthorized. Therefore, any email sent and received willbe handled as ordinary business messages and subject to the company's own rules, and may thus be read also by peopleother than the user named in the mailbox address.
Hi Kyotaro Horiguchi, thanks for you helps. We have a question about the bug. Why there isn't any solution in the HEAD? This bug last since 10.4 version and I can't understand why it is not fixed in the HEAD yet. BR. Fabio Vianello. -----Original Message----- From: Kyotaro Horiguchi [mailto:horikyota.ntt@gmail.com] Sent: lunedì 8 giugno 2020 10:28 To: Vianello Fabio <fabio.vianello@salvagninigroup.com>; pgsql-bugs@lists.postgresql.org; pgsql-hackers@lists.postgresql.org Subject: Re: BUG #16481: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events Hello. It seems to me a bug. At Fri, 05 Jun 2020 11:05:14 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in > The following bug has been logged on the website: > > Bug reference: 16481 > Logged by: Fabio Vianello > Email address: fabio.vianello@salvagninigroup.com > PostgreSQL version: 12.3 > Operating system: Windows 10 > Description: > > About the bug BUG #15293, on PostgreSQL version 10.4 and 11.2 as > describe below, we found the same issue on the PostgreSQL version 12.3. The HEAD behaves the same way. > Is it a feature? > Becasue in the documentation we didn't found any constraint that says > that we can not use NOTIFY/LISTEN on logical replication tables. > > "When using logical replication a stored procedure executed on the > replica is unable to use NOTIFY to send messages to other listeners. > The stored procedure does execute as expected but the pg_notify() > doesn't appear to have any effect. If an insert is run on the replica > side the trigger executes the stored procedure as expected and the > NOTIFY correctly notifies listeners. The message is actually queued, but logical replication worker doesn't signal that to listener backends. If any ordinarysession sent a message to the same listener after that, both messages would be shown at once. That can be fixed by calling ProcessCompletedNotifies() in apply_handle_commit. The function has a code to write out notificationsto connected clients but it doesn't nothing on logical replication workers. regards. -- Kyotaro Horiguchi NTT Open Source Software Center SALVAGNINI ITALIA S.p.A. Via Guido Salvagnini, 51 - IT - 36040 Sarego (VI) T. +39 0444 725111 | F. +39 0444 43 6404 Società a socio unico - Attività direz. e coord.: Salvagnini Holding S.p.A. Clicca qui<https://www.salvagninigroup.com/company-information> per le informazioni societarie salvagninigroup.com<https://www.salvagninigroup.com> | salvagnini.it<http://www.salvagnini.it> Le informazioni trasmesse sono destinate esclusivamente alla persona o alla società in indirizzo e sono da intendersi confidenzialie riservate. Ogni trasmissione, inoltro, diffusione o altro uso di queste informazioni a persone o società differentidal destinatario è proibita. Se avete ricevuto questa comunicazione per errore, per favore contattate il mittentee cancellate le informazioni da ogni computer. Questa casella di posta elettronica è riservata esclusivamente all’invioed alla ricezione di messaggi aziendali inerenti all’attività lavorativa e non è previsto né autorizzato l’utilizzoper fini personali. Pertanto i messaggi in uscita e quelli di risposta in entrata verranno trattati quali messaggiaziendali e soggetti alla ordinaria gestione disposta con proprio disciplinare dall’azienda e, di conseguenza, eventualmenteanche alla lettura da parte di persone diverse dall’intestatario della casella. Any information herein transmitted only concerns the person or the company named in the address and is deemed to be confidential.It is strictly forbidden to transmit, post, forward or otherwise use said information to anyone other than therecipient. If you have received this message by mistake, please contact the sender and delete any relevant informationfrom your computer. This mailbox is only meant for sending and receiving messages pertaining business mattersand any other use for personal purposes is forbidden and unauthorized. Therefore, any email sent and received willbe handled as ordinary business messages and subject to the company's own rules, and may thus be read also by peopleother than the user named in the mailbox address.
On Mon, 8 Jun 2020 at 05:27, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
That can be fixed by calling ProcessCompletedNotifies() in
apply_handle_commit. The function has a code to write out
notifications to connected clients but it doesn't nothing on logical
replication workers.
This bug was already reported some time ago (#15293) but it slipped through the
cracks. I don't think you should simply call ProcessCompletedNotifies [1].
cracks. I don't think you should simply call ProcessCompletedNotifies [1].
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, 8 Jun 2020 at 05:27, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
That can be fixed by calling ProcessCompletedNotifies() in
apply_handle_commit. The function has a code to write out
notifications to connected clients but it doesn't nothing on logical
replication workers.
This bug was already reported some time ago (#15293) but it slipped through the
cracks. I don't think you should simply call ProcessCompletedNotifies [1].
cracks. I don't think you should simply call ProcessCompletedNotifies [1].
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: BUG #16481: Stored Procedure Triggered by Logical Replicationis Unable to use Notification Events
От
Kyotaro Horiguchi
Дата:
Hello, Euler. At Mon, 8 Jun 2020 07:51:18 -0300, Euler Taveira <euler.taveira@2ndquadrant.com> wrote in > On Mon, 8 Jun 2020 at 05:27, Kyotaro Horiguchi <horikyota.ntt@gmail.com> > wrote: > > > > > That can be fixed by calling ProcessCompletedNotifies() in > > apply_handle_commit. The function has a code to write out > > notifications to connected clients but it doesn't nothing on logical > > replication workers. > > > > > This bug was already reported some time ago (#15293) but it slipped through > the > cracks. I don't think you should simply call ProcessCompletedNotifies [1]. Yeah, Thanks for pointing that. I faintly thought of a similar thing to the discussion there. Just calling ProcessCompletedNotifies in apply_handle_commit is actually wrong. We can move only SignalBackends() to AtCommit_Notify since asyncQueueAdvanceTail() is no longer dependent on the result of SignalBackends, but anyway we need to call asyncQueueAdvanceTail in AtCommit_Notify and AtAbort_Notify since otherwise the queue cannot be shorten while running logical replication. This can slightly defers tail-advancing but I think it wouldn't be a significant problem. > [1] https://www.postgresql.org/message-id/13844.1532468610%40sss.pgh.pa.us regards. -- Kyotaro Horiguchi NTT Open Source Software Center From c3aa3e584cf57632284dc9b282dd635c418f3084 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com> Date: Tue, 9 Jun 2020 14:01:34 +0900 Subject: [PATCH v2] Fix notification signaling Notifications are signaled at command loop. That prevents logical replication apply loop from signaling properly. To fix, send signal in AtCommit_Notify instead of the top-level command loop. Discussion: https://www.postgresql.org/message-id/13844.1532468610%40sss.pgh.pa.us Discussion: https://www.postgresql.org/message-id/20200608.172730.68580977059033.horikyota.ntt%40gmail.com --- src/backend/commands/async.c | 103 +++++++++++++++++++++-------------- 1 file changed, 63 insertions(+), 40 deletions(-) diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 71b7577afc..590ad7fcc8 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -449,7 +449,7 @@ static void asyncQueueNotificationToEntry(Notification *n, AsyncQueueEntry *qe); static ListCell *asyncQueueAddEntries(ListCell *nextNotify); static double asyncQueueUsage(void); static void asyncQueueFillWarning(void); -static void SignalBackends(void); +static bool SignalBackends(void); static void asyncQueueReadAllNotifications(void); static bool asyncQueueProcessPageEntries(volatile QueuePosition *current, QueuePosition stop, @@ -976,7 +976,8 @@ PreCommit_Notify(void) * * This is called at transaction commit, after committing to clog. * - * Update listenChannels and clear transaction-local state. + * Update listenChannels and clear transaction-local state. Send signals + * for notifications to other backends to process them. */ void AtCommit_Notify(void) @@ -1021,6 +1022,29 @@ AtCommit_Notify(void) /* And clean up */ ClearPendingActionsAndNotifies(); + + /* signal our notifications to other backends */ + if (backendHasSentNotifications) + { + /* + * No use reading the queue at idle time later if this backend is not a + * listener. + */ + if (!SignalBackends()) + backendHasSentNotifications = false; + + /* + * If it's time to try to advance the global tail pointer, do that. We + * need do this here in case where many transactions are committed + * without returning to the top-level loop, like logical replication + * apply loop. + */ + if (backendTryAdvanceTail) + { + backendTryAdvanceTail = false; + asyncQueueAdvanceTail(); + } + } } /* @@ -1196,10 +1220,8 @@ Exec_UnlistenAllCommit(void) * * This is called from postgres.c just before going idle at the completion * of a transaction. If we issued any notifications in the just-completed - * transaction, send signals to other backends to process them, and also - * process the queue ourselves to send messages to our own frontend. - * Also, if we filled enough queue pages with new notifies, try to advance - * the queue tail pointer. + * transaction, process the queue ourselves to send messages to our own + * frontend. * * The reason that this is not done in AtCommit_Notify is that there is * a nonzero chance of errors here (for example, encoding conversion errors @@ -1208,17 +1230,11 @@ Exec_UnlistenAllCommit(void) * to ensure that a transaction's self-notifies are delivered to the frontend * before it gets the terminating ReadyForQuery message. * - * Note that we send signals and process the queue even if the transaction - * eventually aborted. This is because we need to clean out whatever got - * added to the queue. - * * NOTE: we are outside of any transaction here. */ void ProcessCompletedNotifies(void) { - MemoryContext caller_context; - /* Nothing to do if we didn't send any notifications */ if (!backendHasSentNotifications) return; @@ -1230,43 +1246,32 @@ ProcessCompletedNotifies(void) */ backendHasSentNotifications = false; - /* - * We must preserve the caller's memory context (probably MessageContext) - * across the transaction we do here. - */ - caller_context = CurrentMemoryContext; - if (Trace_notify) elog(DEBUG1, "ProcessCompletedNotifies"); - /* - * We must run asyncQueueReadAllNotifications inside a transaction, else - * bad things happen if it gets an error. - */ - StartTransactionCommand(); - - /* Send signals to other backends */ - SignalBackends(); - if (listenChannels != NIL) { + MemoryContext caller_context; + + /* + * We must preserve the caller's memory context (probably + * MessageContext) across the transaction we do here. + */ + caller_context = CurrentMemoryContext; + + /* + * We must run asyncQueueReadAllNotifications inside a transaction, + * else bad things happen if it gets an error. + */ + StartTransactionCommand(); + /* Read the queue ourselves, and send relevant stuff to the frontend */ asyncQueueReadAllNotifications(); - } + CommitTransactionCommand(); - /* - * If it's time to try to advance the global tail pointer, do that. - */ - if (backendTryAdvanceTail) - { - backendTryAdvanceTail = false; - asyncQueueAdvanceTail(); + MemoryContextSwitchTo(caller_context); } - CommitTransactionCommand(); - - MemoryContextSwitchTo(caller_context); - /* We don't need pq_flush() here since postgres.c will do one shortly */ } @@ -1655,13 +1660,16 @@ asyncQueueFillWarning(void) * advance their queue position pointers, allowing the global tail to advance. * * Since we know the BackendId and the Pid the signaling is quite cheap. + * + * Returns true if this backend is listening to notifications. */ -static void +static bool SignalBackends(void) { int32 *pids; BackendId *ids; int count; + bool am_listener = false; /* * Identify backends that we need to signal. We don't want to send @@ -1684,7 +1692,10 @@ SignalBackends(void) Assert(pid != InvalidPid); if (pid == MyProcPid) + { + am_listener = true; continue; /* never signal self */ + } pos = QUEUE_BACKEND_POS(i); if (QUEUE_BACKEND_DBOID(i) == MyDatabaseId) { @@ -1729,6 +1740,8 @@ SignalBackends(void) pfree(pids); pfree(ids); + + return am_listener; } /* @@ -1752,6 +1765,16 @@ AtAbort_Notify(void) /* And clean up */ ClearPendingActionsAndNotifies(); + + /* + * We can reach here having some notifications queued in this + * transaction. Advance tail pointer in that case. + */ + if (backendTryAdvanceTail) + { + backendTryAdvanceTail = false; + asyncQueueAdvanceTail(); + } } /* -- 2.18.2
Re: BUG #16481: Stored Procedure Triggered by Logical Replicationis Unable to use Notification Events
От
Kyotaro Horiguchi
Дата:
Hello, Euler. At Mon, 8 Jun 2020 07:51:18 -0300, Euler Taveira <euler.taveira@2ndquadrant.com> wrote in > On Mon, 8 Jun 2020 at 05:27, Kyotaro Horiguchi <horikyota.ntt@gmail.com> > wrote: > > > > > That can be fixed by calling ProcessCompletedNotifies() in > > apply_handle_commit. The function has a code to write out > > notifications to connected clients but it doesn't nothing on logical > > replication workers. > > > > > This bug was already reported some time ago (#15293) but it slipped through > the > cracks. I don't think you should simply call ProcessCompletedNotifies [1]. Yeah, Thanks for pointing that. I faintly thought of a similar thing to the discussion there. Just calling ProcessCompletedNotifies in apply_handle_commit is actually wrong. We can move only SignalBackends() to AtCommit_Notify since asyncQueueAdvanceTail() is no longer dependent on the result of SignalBackends, but anyway we need to call asyncQueueAdvanceTail in AtCommit_Notify and AtAbort_Notify since otherwise the queue cannot be shorten while running logical replication. This can slightly defers tail-advancing but I think it wouldn't be a significant problem. > [1] https://www.postgresql.org/message-id/13844.1532468610%40sss.pgh.pa.us regards. -- Kyotaro Horiguchi NTT Open Source Software Center From c3aa3e584cf57632284dc9b282dd635c418f3084 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com> Date: Tue, 9 Jun 2020 14:01:34 +0900 Subject: [PATCH v2] Fix notification signaling Notifications are signaled at command loop. That prevents logical replication apply loop from signaling properly. To fix, send signal in AtCommit_Notify instead of the top-level command loop. Discussion: https://www.postgresql.org/message-id/13844.1532468610%40sss.pgh.pa.us Discussion: https://www.postgresql.org/message-id/20200608.172730.68580977059033.horikyota.ntt%40gmail.com --- src/backend/commands/async.c | 103 +++++++++++++++++++++-------------- 1 file changed, 63 insertions(+), 40 deletions(-) diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 71b7577afc..590ad7fcc8 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -449,7 +449,7 @@ static void asyncQueueNotificationToEntry(Notification *n, AsyncQueueEntry *qe); static ListCell *asyncQueueAddEntries(ListCell *nextNotify); static double asyncQueueUsage(void); static void asyncQueueFillWarning(void); -static void SignalBackends(void); +static bool SignalBackends(void); static void asyncQueueReadAllNotifications(void); static bool asyncQueueProcessPageEntries(volatile QueuePosition *current, QueuePosition stop, @@ -976,7 +976,8 @@ PreCommit_Notify(void) * * This is called at transaction commit, after committing to clog. * - * Update listenChannels and clear transaction-local state. + * Update listenChannels and clear transaction-local state. Send signals + * for notifications to other backends to process them. */ void AtCommit_Notify(void) @@ -1021,6 +1022,29 @@ AtCommit_Notify(void) /* And clean up */ ClearPendingActionsAndNotifies(); + + /* signal our notifications to other backends */ + if (backendHasSentNotifications) + { + /* + * No use reading the queue at idle time later if this backend is not a + * listener. + */ + if (!SignalBackends()) + backendHasSentNotifications = false; + + /* + * If it's time to try to advance the global tail pointer, do that. We + * need do this here in case where many transactions are committed + * without returning to the top-level loop, like logical replication + * apply loop. + */ + if (backendTryAdvanceTail) + { + backendTryAdvanceTail = false; + asyncQueueAdvanceTail(); + } + } } /* @@ -1196,10 +1220,8 @@ Exec_UnlistenAllCommit(void) * * This is called from postgres.c just before going idle at the completion * of a transaction. If we issued any notifications in the just-completed - * transaction, send signals to other backends to process them, and also - * process the queue ourselves to send messages to our own frontend. - * Also, if we filled enough queue pages with new notifies, try to advance - * the queue tail pointer. + * transaction, process the queue ourselves to send messages to our own + * frontend. * * The reason that this is not done in AtCommit_Notify is that there is * a nonzero chance of errors here (for example, encoding conversion errors @@ -1208,17 +1230,11 @@ Exec_UnlistenAllCommit(void) * to ensure that a transaction's self-notifies are delivered to the frontend * before it gets the terminating ReadyForQuery message. * - * Note that we send signals and process the queue even if the transaction - * eventually aborted. This is because we need to clean out whatever got - * added to the queue. - * * NOTE: we are outside of any transaction here. */ void ProcessCompletedNotifies(void) { - MemoryContext caller_context; - /* Nothing to do if we didn't send any notifications */ if (!backendHasSentNotifications) return; @@ -1230,43 +1246,32 @@ ProcessCompletedNotifies(void) */ backendHasSentNotifications = false; - /* - * We must preserve the caller's memory context (probably MessageContext) - * across the transaction we do here. - */ - caller_context = CurrentMemoryContext; - if (Trace_notify) elog(DEBUG1, "ProcessCompletedNotifies"); - /* - * We must run asyncQueueReadAllNotifications inside a transaction, else - * bad things happen if it gets an error. - */ - StartTransactionCommand(); - - /* Send signals to other backends */ - SignalBackends(); - if (listenChannels != NIL) { + MemoryContext caller_context; + + /* + * We must preserve the caller's memory context (probably + * MessageContext) across the transaction we do here. + */ + caller_context = CurrentMemoryContext; + + /* + * We must run asyncQueueReadAllNotifications inside a transaction, + * else bad things happen if it gets an error. + */ + StartTransactionCommand(); + /* Read the queue ourselves, and send relevant stuff to the frontend */ asyncQueueReadAllNotifications(); - } + CommitTransactionCommand(); - /* - * If it's time to try to advance the global tail pointer, do that. - */ - if (backendTryAdvanceTail) - { - backendTryAdvanceTail = false; - asyncQueueAdvanceTail(); + MemoryContextSwitchTo(caller_context); } - CommitTransactionCommand(); - - MemoryContextSwitchTo(caller_context); - /* We don't need pq_flush() here since postgres.c will do one shortly */ } @@ -1655,13 +1660,16 @@ asyncQueueFillWarning(void) * advance their queue position pointers, allowing the global tail to advance. * * Since we know the BackendId and the Pid the signaling is quite cheap. + * + * Returns true if this backend is listening to notifications. */ -static void +static bool SignalBackends(void) { int32 *pids; BackendId *ids; int count; + bool am_listener = false; /* * Identify backends that we need to signal. We don't want to send @@ -1684,7 +1692,10 @@ SignalBackends(void) Assert(pid != InvalidPid); if (pid == MyProcPid) + { + am_listener = true; continue; /* never signal self */ + } pos = QUEUE_BACKEND_POS(i); if (QUEUE_BACKEND_DBOID(i) == MyDatabaseId) { @@ -1729,6 +1740,8 @@ SignalBackends(void) pfree(pids); pfree(ids); + + return am_listener; } /* @@ -1752,6 +1765,16 @@ AtAbort_Notify(void) /* And clean up */ ClearPendingActionsAndNotifies(); + + /* + * We can reach here having some notifications queued in this + * transaction. Advance tail pointer in that case. + */ + if (backendTryAdvanceTail) + { + backendTryAdvanceTail = false; + asyncQueueAdvanceTail(); + } } /* -- 2.18.2
Just to help those who find themselves in the same situation, there is a simple application-level workaround which
consistsin listening to the notifications in the replica when they are issued by the master and vice versa.
We are programming in .NET and we use a code like this:
Code in the replica side:
var csb = new NpgsqlConnectionStringBuilder
{
Host = "master",
Database = "MasterDB",
Port = 5432,
Username = "postgres",
Password = "XXXXXXX",
};
var connection = new NpgsqlConnection(csb.ConnectionString);
connection.Open();
using (var command = new NpgsqlCommand("listen \"Table\"", connection))
{
command.ExecuteNonQuery();
}
connection.Notification += PostgresNotification;
So you can listen from the replica every changed raised by a trigger on the master from the replica side on the table
"Table".
CREATE TRIGGER master_trigger
AFTER INSERT OR DELETE OR UPDATE
ON public."TABLE"
FOR EACH ROW
EXECUTE PROCEDURE public.master_notice();
ALTER TABLE public."Tabele"
ENABLE ALWAYS TRIGGER master_trigger;
CREATE FUNCTION public. master_notice ()
RETURNS trigger
LANGUAGE 'plpgsql'
COST 100
VOLATILE NOT LEAKPROOF
AS $BODY$
BEGIN
PERFORM pg_notify('Table', Cast(NEW."ID" as text));
RETURN NEW;
END;
$BODY$;
ALTER FUNCTION public.incupdate1_notice()
OWNER TO postgres;
I hope that help someone, because the bug last from "years". I tried in version 10 11 and 12, so it is present since
2017-10-05and I can't see any solution on 13 beta.
Best Regards.
Fabio.
-----Original Message-----
From: Vianello Fabio
Sent: lunedì 8 giugno 2020 11:14
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org; pgsql-hackers@lists.postgresql.org
Subject: RE: BUG #16481: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
Hi Kyotaro Horiguchi, thanks for you helps.
We have a question about the bug. Why there isn't any solution in the HEAD?
This bug last since 10.4 version and I can't understand why it is not fixed in the HEAD yet.
BR.
Fabio Vianello.
-----Original Message-----
From: Kyotaro Horiguchi [mailto:horikyota.ntt@gmail.com]
Sent: lunedì 8 giugno 2020 10:28
To: Vianello Fabio <fabio.vianello@salvagninigroup.com>; pgsql-bugs@lists.postgresql.org;
pgsql-hackers@lists.postgresql.org
Subject: Re: BUG #16481: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
Hello.
It seems to me a bug.
At Fri, 05 Jun 2020 11:05:14 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in
> The following bug has been logged on the website:
>
> Bug reference: 16481
> Logged by: Fabio Vianello
> Email address: fabio.vianello@salvagninigroup.com
> PostgreSQL version: 12.3
> Operating system: Windows 10
> Description:
>
> About the bug BUG #15293, on PostgreSQL version 10.4 and 11.2 as
> describe below, we found the same issue on the PostgreSQL version 12.3.
The HEAD behaves the same way.
> Is it a feature?
> Becasue in the documentation we didn't found any constraint that says
> that we can not use NOTIFY/LISTEN on logical replication tables.
>
> "When using logical replication a stored procedure executed on the
> replica is unable to use NOTIFY to send messages to other listeners.
> The stored procedure does execute as expected but the pg_notify()
> doesn't appear to have any effect. If an insert is run on the replica
> side the trigger executes the stored procedure as expected and the
> NOTIFY correctly notifies listeners.
The message is actually queued, but logical replication worker doesn't signal that to listener backends. If any
ordinarysession sent a message to the same listener after that, both messages would be shown at once.
That can be fixed by calling ProcessCompletedNotifies() in apply_handle_commit. The function has a code to write out
notificationsto connected clients but it doesn't nothing on logical replication workers.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
SALVAGNINI ITALIA S.p.A.
Via Guido Salvagnini, 51 - IT - 36040 Sarego (VI)
T. +39 0444 725111 | F. +39 0444 43 6404
Società a socio unico - Attività direz. e coord.: Salvagnini Holding S.p.A.
Clicca qui<https://www.salvagninigroup.com/company-information> per le informazioni societarie
salvagninigroup.com<https://www.salvagninigroup.com> | salvagnini.it<http://www.salvagnini.it>
Le informazioni trasmesse sono destinate esclusivamente alla persona o alla società in indirizzo e sono da intendersi
confidenzialie riservate. Ogni trasmissione, inoltro, diffusione o altro uso di queste informazioni a persone o società
differentidal destinatario è proibita. Se avete ricevuto questa comunicazione per errore, per favore contattate il
mittentee cancellate le informazioni da ogni computer. Questa casella di posta elettronica è riservata esclusivamente
all’invioed alla ricezione di messaggi aziendali inerenti all’attività lavorativa e non è previsto né autorizzato
l’utilizzoper fini personali. Pertanto i messaggi in uscita e quelli di risposta in entrata verranno trattati quali
messaggiaziendali e soggetti alla ordinaria gestione disposta con proprio disciplinare dall’azienda e, di conseguenza,
eventualmenteanche alla lettura da parte di persone diverse dall’intestatario della casella.
Any information herein transmitted only concerns the person or the company named in the address and is deemed to be
confidential.It is strictly forbidden to transmit, post, forward or otherwise use said information to anyone other than
therecipient. If you have received this message by mistake, please contact the sender and delete any relevant
informationfrom your computer. This mailbox is only meant for sending and receiving messages pertaining business
mattersand any other use for personal purposes is forbidden and unauthorized. Therefore, any email sent and received
willbe handled as ordinary business messages and subject to the company's own rules, and may thus be read also by
peopleother than the user named in the mailbox address.
Just to help those who find themselves in the same situation, there is a simple application-level workaround which
consistsin listening to the notifications in the replica when they are issued by the master and vice versa.
We are programming in .NET and we use a code like this:
Code in the replica side:
var csb = new NpgsqlConnectionStringBuilder
{
Host = "master",
Database = "MasterDB",
Port = 5432,
Username = "postgres",
Password = "XXXXXXX",
};
var connection = new NpgsqlConnection(csb.ConnectionString);
connection.Open();
using (var command = new NpgsqlCommand("listen \"Table\"", connection))
{
command.ExecuteNonQuery();
}
connection.Notification += PostgresNotification;
So you can listen from the replica every changed raised by a trigger on the master from the replica side on the table
"Table".
CREATE TRIGGER master_trigger
AFTER INSERT OR DELETE OR UPDATE
ON public."TABLE"
FOR EACH ROW
EXECUTE PROCEDURE public.master_notice();
ALTER TABLE public."Tabele"
ENABLE ALWAYS TRIGGER master_trigger;
CREATE FUNCTION public. master_notice ()
RETURNS trigger
LANGUAGE 'plpgsql'
COST 100
VOLATILE NOT LEAKPROOF
AS $BODY$
BEGIN
PERFORM pg_notify('Table', Cast(NEW."ID" as text));
RETURN NEW;
END;
$BODY$;
ALTER FUNCTION public.incupdate1_notice()
OWNER TO postgres;
I hope that help someone, because the bug last from "years". I tried in version 10 11 and 12, so it is present since
2017-10-05and I can't see any solution on 13 beta.
Best Regards.
Fabio.
-----Original Message-----
From: Vianello Fabio
Sent: lunedì 8 giugno 2020 11:14
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org; pgsql-hackers@lists.postgresql.org
Subject: RE: BUG #16481: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
Hi Kyotaro Horiguchi, thanks for you helps.
We have a question about the bug. Why there isn't any solution in the HEAD?
This bug last since 10.4 version and I can't understand why it is not fixed in the HEAD yet.
BR.
Fabio Vianello.
-----Original Message-----
From: Kyotaro Horiguchi [mailto:horikyota.ntt@gmail.com]
Sent: lunedì 8 giugno 2020 10:28
To: Vianello Fabio <fabio.vianello@salvagninigroup.com>; pgsql-bugs@lists.postgresql.org;
pgsql-hackers@lists.postgresql.org
Subject: Re: BUG #16481: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
Hello.
It seems to me a bug.
At Fri, 05 Jun 2020 11:05:14 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in
> The following bug has been logged on the website:
>
> Bug reference: 16481
> Logged by: Fabio Vianello
> Email address: fabio.vianello@salvagninigroup.com
> PostgreSQL version: 12.3
> Operating system: Windows 10
> Description:
>
> About the bug BUG #15293, on PostgreSQL version 10.4 and 11.2 as
> describe below, we found the same issue on the PostgreSQL version 12.3.
The HEAD behaves the same way.
> Is it a feature?
> Becasue in the documentation we didn't found any constraint that says
> that we can not use NOTIFY/LISTEN on logical replication tables.
>
> "When using logical replication a stored procedure executed on the
> replica is unable to use NOTIFY to send messages to other listeners.
> The stored procedure does execute as expected but the pg_notify()
> doesn't appear to have any effect. If an insert is run on the replica
> side the trigger executes the stored procedure as expected and the
> NOTIFY correctly notifies listeners.
The message is actually queued, but logical replication worker doesn't signal that to listener backends. If any
ordinarysession sent a message to the same listener after that, both messages would be shown at once.
That can be fixed by calling ProcessCompletedNotifies() in apply_handle_commit. The function has a code to write out
notificationsto connected clients but it doesn't nothing on logical replication workers.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
SALVAGNINI ITALIA S.p.A.
Via Guido Salvagnini, 51 - IT - 36040 Sarego (VI)
T. +39 0444 725111 | F. +39 0444 43 6404
Società a socio unico - Attività direz. e coord.: Salvagnini Holding S.p.A.
Clicca qui<https://www.salvagninigroup.com/company-information> per le informazioni societarie
salvagninigroup.com<https://www.salvagninigroup.com> | salvagnini.it<http://www.salvagnini.it>
Le informazioni trasmesse sono destinate esclusivamente alla persona o alla società in indirizzo e sono da intendersi
confidenzialie riservate. Ogni trasmissione, inoltro, diffusione o altro uso di queste informazioni a persone o società
differentidal destinatario è proibita. Se avete ricevuto questa comunicazione per errore, per favore contattate il
mittentee cancellate le informazioni da ogni computer. Questa casella di posta elettronica è riservata esclusivamente
all’invioed alla ricezione di messaggi aziendali inerenti all’attività lavorativa e non è previsto né autorizzato
l’utilizzoper fini personali. Pertanto i messaggi in uscita e quelli di risposta in entrata verranno trattati quali
messaggiaziendali e soggetti alla ordinaria gestione disposta con proprio disciplinare dall’azienda e, di conseguenza,
eventualmenteanche alla lettura da parte di persone diverse dall’intestatario della casella.
Any information herein transmitted only concerns the person or the company named in the address and is deemed to be
confidential.It is strictly forbidden to transmit, post, forward or otherwise use said information to anyone other than
therecipient. If you have received this message by mistake, please contact the sender and delete any relevant
informationfrom your computer. This mailbox is only meant for sending and receiving messages pertaining business
mattersand any other use for personal purposes is forbidden and unauthorized. Therefore, any email sent and received
willbe handled as ordinary business messages and subject to the company's own rules, and may thus be read also by
peopleother than the user named in the mailbox address.