Re: NOTIFY and pg_notify performance when deduplicating notifications

On Tue, Oct 9, 2018 at 2:17 PM <julien(at)jdemoor(dot)com> wrote:
> I just caught an error in my patch, it's fixed in the attachment. The 'never' and 'maybe' collapse modes were mixed up in one location.

Here's a partial review of this version, did not read the doc part
very carefully.

First of all, I agree that this is a desirable feature as, for a large
number of notiifications, the O(n^2) overhead quickly becomes very
noticeable.

I would expect the collapse mode to be an enum which is created from
the string early on during parsing and used for the rest of the code.
Instead the string is used all the way leading to string comparisons
in the notification dispatcher and to the need of hardcoding special
strings in various places, including the contrib module.

This comment in the beginning of async.c should also be updated:
* Duplicate notifications from the same transaction are sent out as one
* notification only. This is done to save work when for example a trigger

pg_notify_3args duplicates pg_notify, I would expect a helper function
to be extracted and called from both.

There are braces placed on the same line as the if, for example if
(strlen(collapse_mode) != 0) { which seems to not be the project's
style.

>
> I can't find a reasonable way to build a regression test that checks whether notifications are effectively deduplicated. The output of the LISTEN command lists the PID of the notifying backend for each notification, e.g. : 'Asynchronous notification "foobar" received from server process with PID 24917'. I can't just add this to async.out. I did test manually for all eight combinations : four collapse mode values (missing, empty string, 'maybe' and 'never'), both with NOTIFY and pg_notify().

One way could be to take inspiration from
src/test/isolation/specs/async-notify.spec and check that
pg_notification_queue_usage() does grow when repeating the same
payload with collapse_mode='never' (while for always it would grow).
But I'm not sure it's worth the effort.