PostgreSQL comes with a builtin, performant, scalable and transactional pub/sub system calledLISTEN/NOTIFY.
This PR allows to leverage this mechanism when using the Messenger component with Postgres.

When the Postgres is used, workers are notified in real-time when a message is dispatched.
This reduces the latency, and prevents the worker from executing useless SQL queries. Basically, it allows to switch from a polling-based approach to an event-based one.

This patch can be used with all existing installation of Messenger, as long as the underlying DBMS is Postgres. For many (most ?) projects, it allows to get the benefits of using a queue system such as RabbitMQ or Pulsar without having to introduce new services to monitor, replicate or upgrade.

If PostgreSQL is used,LISTEN/NOTIFY is used automatically!

That's all!

It's also possible to configure how long the worker must wait for new messages:

framework:messenger:transports:async:dsn:'%env(MESSENGER_TRANSPORT_DSN)%'options:pgsql_get_notify:truepgsql_get_notify_timeout:500

Then you can use start the workers with something like:php bin/console messenger:consume --sleep=0

A demo app using this new feature is available in this repository:https://github.com/dunglas/demo-postgres-listen-notify

TODO:

Add tests

dunglas requested a review fromsroze as acode owner

January 27, 2020 17:25

carsonbot added Status: Needs Review Messenger Feature labels

Jan 27, 2020

dunglas force-pushed themessenger-pg-listen-notify branch 3 times, most recently from0171724 to5363561Compare

January 27, 2020 17:30

jderusse reviewed

Jan 27, 2020

View reviewed changes

src/Symfony/Component/Messenger/Transport/Doctrine/Connection.php OutdatedShow resolvedHide resolved

Copy link

MemberAuthor

dunglas commentedJan 27, 2020•
edited
Loading

An alternative implementation is to use a trigger instead of callingpg_notify directly from PHP. The benefit of using a trigger is that the system works even if another app or client insert a line in the Messenger's database. The drawback is that Doctrine DBAL doesn't "support" triggers, and we'll have to execute a custom query after the setup (not a big deal IMHO).

For the record, I first usedLISTEN/NOTIFY as a driver for the HA version of Mercure.rocks. For Mercure, I use a trigger, and it works perfectly well.

Edit: implementation added in5207f51

dunglas force-pushed themessenger-pg-listen-notify branch from51fc6ee to5207f51Compare

January 27, 2020 19:39

bendavies reviewed

Jan 28, 2020

View reviewed changes

src/Symfony/Component/Messenger/Transport/Doctrine/Connection.php OutdatedShow resolvedHide resolved

Ocramius suggested changes

Jan 28, 2020

View reviewed changes

src/Symfony/Component/Messenger/Transport/Doctrine/Connection.php Outdated

		@@ -35,6 +35,9 @@ class Connection
		'queue_name' => 'default',
		'redeliver_timeout' => 3600,
		'auto_setup' => true,
		'pgsql_get_notify' => false,

Copy link

Contributor

OcramiusJan 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This should probably be its own transport, not mixing PostgreSQL-specifics with DBAL abstractions

Copy link

MemberAuthor

dunglasJan 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I had the same feeling in the beginning, but it will require to duplicate most of the code of this transport. So I'm not sure that it is worth it from a maintenance point of view.
Now that the implementation is less intrusive than it was, I'll check if the PostgreSQL related code can be moved to a decorator.

Copy link

Contributor

OcramiusJan 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yeah, but that's not a big deal: more problematic is having generic SQL and PGSQL mixed within the same service, activated conditionally.

If the "enable norify" logic deals to a completely dedicated adapter, fixing specific bugs and maintenance becomes much easier, and so will debugging too.

Copy link

MemberAuthor

dunglasJan 28, 2020•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I isolated the PostgresSQL-specific code in a dedicated class, it's a bit cleaner, and while far from being perfect I think it will be good enough for now at least.

src/Symfony/Component/Messenger/Transport/Doctrine/Connection.php OutdatedShow resolvedHide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels

Jan 28, 2020

bendavies reviewed

Jan 28, 2020

View reviewed changes

src/Symfony/Component/Messenger/Transport/Doctrine/Connection.php Outdated

		@@ -227,6 +270,28 @@ public function setup(): void
		$this->driverConnection->getConfiguration()->setFilterSchemaAssetsExpression($assetFilter);
		}

		if ($this->usePgsqlNotify) {
		$sql = sprintf(<<<'SQL'

Copy link

Contributor

bendaviesJan 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

wrap in begin...commit?

Copy link

MemberAuthor

dunglasJan 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

What would be the benefit?

Copy link

Contributor

bendaviesJan 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

isexec transactional? you want both statements to fail or both to succeed.

Copy link

MemberAuthor

dunglasJan 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm not sure,pg_query is transnational, but I cannot find the information for exec.

dunglas force-pushed themessenger-pg-listen-notify branch 2 times, most recently froma131f83 to2be2850Compare

January 28, 2020 17:28

gquemener mentioned this pull request

Jan 30, 2020

Make use of Postgres pub/sub ?prooph/pdo-event-store#86

Open

chalasr approved these changes

Jan 30, 2020

View reviewed changes

Copy link

Member

chalasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

with minor comment

src/Symfony/Component/Messenger/Bridge/Doctrine/Transport/Connection.phpShow resolvedHide resolved

src/Symfony/Component/Messenger/Bridge/Doctrine/Transport/PostgreSqlConnection.phpShow resolvedHide resolved

carsonbot added Status: Reviewed and removed Status: Needs Work labels

Jan 30, 2020

nicolas-grekas added this to thenext milestone

Jan 31, 2020

dunglas force-pushed themessenger-pg-listen-notify branch froma32d12f tob976720Compare

January 31, 2020 18:15

fabpot approved these changes

Feb 3, 2020

View reviewed changes

Copy link

Member

fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM, minor comment.

src/Symfony/Component/Messenger/Bridge/Doctrine/Transport/PostgreSqlConnection.php

		* file that was distributed with this source code.
		*/

		declare(strict_types=1);

Copy link

Member

fabpotFeb 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Should be removed

[Messenger] Add support for PostgreSQL LISTEN/NOTIFY

01f33c3

fabpot force-pushed themessenger-pg-listen-notify branch fromb976720 to01f33c3Compare

February 4, 2020 10:43

Copy link

Member

fabpot commentedFeb 4, 2020

Thank you@dunglas.

fabpot mentioned this pull request

Feb 4, 2020

[Messenger] Add support for PostgreSQL LISTEN/NOTIFYsymfony/symfony-docs#13039

Closed

fabpot closed this ine3fa80a

Feb 4, 2020

fabpot merged commit01f33c3 intosymfony:master

Feb 4, 2020

dunglas deleted the messenger-pg-listen-notify branch

February 5, 2020 00:40

lyrixx mentioned this pull request

Feb 5, 2020

[Messenger] Made final class really final#35600

Merged

chalasr added a commit that referenced this pull request

Feb 5, 2020

minor#35600[Messenger] Made final class really final (lyrixx)

0b8adc9

This PR was merged into the 5.1-dev branch.Discussion----------[Messenger] Made final class really final| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       || License       | MIT| Doc PR        |introduced in#35485 (so no BC break)Commits-------cd8a386 [Messenger] Made final class really final

tacman mentioned this pull request

Feb 24, 2020

Support queued messages via Symfony Messenger?FriendsOfSymfony/FOSElasticaBundle#1586

Closed

nicolas-grekas modified the milestones:next,5.1

May 4, 2020

fabpot mentioned this pull request

May 5, 2020

Release v5.1.0-BETA1#36713

Merged

gubler mentioned this pull request

Jul 8, 2020

Error setting up Doctrine transport for Messenger with MS SQL Server#37532

Closed

t-richard mentioned this pull request

Feb 28, 2021

[Messenger] Add options for PostgreSQL LISTEN/NOTIFY supportsymfony/symfony-docs#15043

Merged

Nyholm mentioned this pull request

Mar 5, 2021

[Messenger] Don't lock tables or start transactions#40376

Merged

fabpot added a commit that referenced this pull request

Mar 5, 2021

bug#40376[Messenger] Don't lock tables or start transactions (Nyholm)

9978ba8

This PR was merged into the 5.2 branch.Discussion----------[Messenger] Don't lock tables or start transactions| Q             | A| ------------- | ---| Branch?       | 5.2| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       || License       | MIT| Doc PR        |I was so sure my PR#40336 fixed the doctrine-messenger issue once and for all. But it had a very silent bug..This has been tricky to debug and find because the `PDO` behaves differently in PHP 7.4 compared to PHP 8.| Scenario | Command | Is executed in transaction | Method that calls `PostgreSqlConnection::getTriggerSql()` || --| -- | -- | -- || A | `messenger:setup-transports` | No | `setup()`| B| `doctrine:schema:create` | No | `getExtraSetupSqlForTable()`| C | `doctrine:migration:diff` | Yes by default, but it can be configured | `getExtraSetupSqlForTable()`PR#40055 fixed scenario C on PHP 8, but that also broke scenario B on PHP 7.4 and PHP 8.In PR#40336 I was wrong claiming:> We don't need COMMIT because the transaction will commit itself when we close the connection.The result was the we removed all the errors messages from the 3 scenarios. But scenario B will produce some SQL that is actually never committed. IE it will silently fail.I've been trying to figure out a good solution to how or when to start a transaction. I tried out@fbourigault [suggestion](#40336 (comment)) but that would be the same fix as#40055.---------------We need a transaction because the SQL includes a `LOCK TABLE`, however, I cannot see a strict need for it. This PR removes `LOCK TABLE` and all transaction juggling. It all seams to work.I would be happy to get thorough feedback on this PR so we can end the chapter of constantly adding bugs to this part of the component.@dunglas, you added `LOCK TABLE` in your initial version of this class in#35485, could you share some knowledge if this is a good or bad idea?Commits-------26061a1 Dont lock tables or start transactions

wouterj added a commit to symfony/symfony-docs that referenced this pull request

Apr 7, 2021

feature#15043[Messenger] Add options for PostgreSQL LISTEN/NOTIFY s…

d74f702

…upport (t-richard)This PR was squashed before being merged into the 5.2 branch.Discussion----------[Messenger] Add options for PostgreSQL LISTEN/NOTIFY support<!--If your pull request fixes a BUG, use the oldest maintained branch that containsthe bug (seehttps://symfony.com/releases for the list of maintained branches).If your pull request documents a NEW FEATURE, use the same Symfony branch wherethe feature was introduced (and `5.x` for features of unreleased versions).-->Documentssymfony/symfony#35485Closes#13039Commits-------55edd48 [Messenger] Add options for PostgreSQL LISTEN/NOTIFY support