Обсуждение: Notifications within triggers seem to compromise performance

Hi there!

I guess we stumbled upon a performance issue with notifications sent within triggers (using PostgreSQL version 11.5)
and I'd like your opinion about this.

We want our app to maintain a data cache, so each instance of the app listens to some channels (one per table).
There are update triggers set up on the tables so each update yelds a notification to the appropriate channel.

It works fine and we love the feature, but it seems to come with a performance cost.
Since we set them up, we get query timeouts in our app (set to 200ms in the app).

To try and understand this, we set deadlock_timeout to 100ms and enabled log_lock_waits to get the following warnings in the log: process XXXXX still waiting for AccessExclusiveLock on object 0 of class 1262 of database 0 after YYY.YYY ms
A row update transaction on table A is waiting for another row update transaction on table B. Tables are only tied by an FK, the updated fields are not the ID or FK fields.

A quick google + source code search showed the PreCommit_Notify function is trying to acquire this lock.
My educated guess of what happens during a COMMIT is the following :
- pre-commit actions are taken, the "notification lock" is taken
- commit actions are performed (can take some time)
- post-commit actions are taken, the notification is enqueued and "notification lock" is released

Am I correct ?

Other transactions involving a notification are stuck waiting for previous transactions to finish, this can be a performance issue.

I understand the need for lock to be taken pre-commit to ensure notification order matches transaction order, but it in my case I don't really care about the order and the performance penalty is high.

We could think of several options there :
- different locks for different channels (implies different notification queues I guess)
- an argument to NOTIFY query not to guarantee notifications order (and thus take and release the lock in post-commit actions)

I believe the notify-in-trigger should be a pretty common usage pattern and so this finding may impact quite a few systems.

What do you think about this ?

Regards,

-- 
Grégoire de Turckheim

=?UTF-8?Q?Gr=c3=a9goire_de_Turckheim?= <gdeturckheim@scaleway.com> writes:
> I guess we stumbled upon a performance issue with notifications sent within triggers (using PostgreSQL version 11.5)
> and I'd like your opinion about this.

We made some performance improvements for NOTIFY just a couple months
ago, cf commits b10f40bf0, bb5ae8f6c, bca6e6435, 51004c717.  It would
be interesting to know how much those changes helped your use-case.

I'm quite disinclined to reduce the correctness guarantees around
NOTIFY for performance's sake.  That's the sort of thing that sounds
like a good idea until you find out that it subtly breaks your
application, and then you've got nothing.

            regards, tom lane

Le 28/10/2019 à 15:22, Tom Lane a écrit :
> =?UTF-8?Q?Gr=c3=a9goire_de_Turckheim?= <gdeturckheim@scaleway.com> writes:
>> I guess we stumbled upon a performance issue with notifications sent within triggers (using PostgreSQL version 11.5)
>> and I'd like your opinion about this.
> We made some performance improvements for NOTIFY just a couple months
> ago, cf commits b10f40bf0, bb5ae8f6c, bca6e6435, 51004c717.  It would
> be interesting to know how much those changes helped your use-case.
Thanks for your quick reply!

If my understanding of the problem is correct, there is no performance
issue with the notification itself.
The problem is the following: a system-wide lock is taken pre-commit, so
any other transaction with a NOTIFY will have to wait for other
transactions to complete before it can leave its own pre-commit stage.
Is this wording better to clarify my explanation attempt ? :)

In my case, ~90% of the data is in tables with triggered notifications,
all of this data updates become "single threaded", whatever the table it
is in.
>
> I'm quite disinclined to reduce the correctness guarantees around
> NOTIFY for performance's sake.  That's the sort of thing that sounds
> like a good idea until you find out that it subtly breaks your
> application, and then you've got nothing.
100% agreed, this is why my suggestion was to make it an option. From a
user perspective, it seems very complex to understand if this option is
to be used or not. I really don't know how to present such an option to
the user.

There also may be better ways to do it, I suggested different queues
(and thus locks) for different channels but I have no idea about the
cost of it.

Regards,

--
Grégoire de Turckheim

=?UTF-8?Q?Gr=c3=a9goire_de_Turckheim?= <gdeturckheim@scaleway.com> writes:
> Le 28/10/2019 à 15:22, Tom Lane a écrit :
>> We made some performance improvements for NOTIFY just a couple months
>> ago, cf commits b10f40bf0, bb5ae8f6c, bca6e6435, 51004c717.  It would
>> be interesting to know how much those changes helped your use-case.

> If my understanding of the problem is correct, there is no performance 
> issue with the notification itself.
> The problem is the following: a system-wide lock is taken pre-commit, so 
> any other transaction with a NOTIFY will have to wait for other 
> transactions to complete before it can leave its own pre-commit stage.

Right, but all commits are single-threaded at some granularity.
The big problem with NOTIFY is that it sits for a long time holding
that lock, if you have a lot of notify traffic.  The commits I mentioned
should improve that.

Anyway, as I said, it would be good to find out whether the already
finished fixes are enough to solve your problem, before we debate
whether more needs to be done.

            regards, tom lane

Le 28/10/2019 à 17:25, Tom Lane a écrit :
> =?UTF-8?Q?Gr=c3=a9goire_de_Turckheim?= <gdeturckheim@scaleway.com> writes:
>> Le 28/10/2019 à 15:22, Tom Lane a écrit :
>>> We made some performance improvements for NOTIFY just a couple months
>>> ago, cf commits b10f40bf0, bb5ae8f6c, bca6e6435, 51004c717.  It would
>>> be interesting to know how much those changes helped your use-case.
>> If my understanding of the problem is correct, there is no performance
>> issue with the notification itself.
>> The problem is the following: a system-wide lock is taken pre-commit, so
>> any other transaction with a NOTIFY will have to wait for other
>> transactions to complete before it can leave its own pre-commit stage.
> Right, but all commits are single-threaded at some granularity.
> The big problem with NOTIFY is that it sits for a long time holding
> that lock, if you have a lot of notify traffic.  The commits I mentioned
> should improve that.
>
> Anyway, as I said, it would be good to find out whether the already
> finished fixes are enough to solve your problem, before we debate
> whether more needs to be done.
Let's do it this way, I'll give it a try (might be long, this isn't
something we can easily upgrade) and get back to you.

Thanks!

--
Grégoire de Turckheim