[DoctrineBridge] Idle connection listener for long running runtime

[alli83]

Q	A
Branch?	7.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	Fix #51661
License	MIT

This pull request introduces a solution based on the RoadRunner bundle's Doctrine ORM/ODM middleware https://github.com/Baldinof/roadrunner-bundle/blob/3.x/src/Integration/Doctrine/DoctrineORMMiddleware.php#L22.
It checks the status of Doctrine connection, then if the connection is initialized and connected, it performs a 'ping' to check its viability. If the ping fails, it closes the connection.

linked to doctrine/DoctrineBundle#1739

[alexandre-daubois]

A few suggestions 🙂

[nicolas-grekas]

Hi, thanks for working on this topic.
Would there be a way to put that concern in Doctrine itself, by decorating the Connection object?
I'm concerned that the current approach will trigger one DB call per HTTP request while not all HTTP entpoints might use Doctrine, and also by the fact this only works for HTTP while recovering from stalled DB connections is also a concern for CLI workers.

[OskarStark]

Just reading the code, it adds a lot new stuff which looks like a new feature instead of a bugfix to me

[dunglas]

I agree that a decorator would be better. Also, would it be possible to catch errors and reconnect only in this case instead of triggering a ping? This would remove the performance hit in most cases.

@OskarStark the code probably needs to be simplified, but the underlying bug is annoying because it makes Symfony apps unstable when using FrankenPHP, RoadRunner (without the bundle), Swoole etc. The fix should be made at least in the current stable version.

[alli83]

@nicolas-grekas @dunglas ok I'll update it and try to decorate the Connection object

[OskarStark]

@OskarStark the code probably needs to be simplified, but the underlying bug is annoying because it makes Symfony apps unstable when using FrankenPHP, RoadRunner (without the bundle), Swoole etc. The fix should be made at least in the current stable version.

Thanks for the explanation Kevin 👍

[ostrolucky]

I don't like the approach, because as @nicolas-grekas mentioned, this will keep pinging the DB every request, even if normally request wouldn't need connection. Can't we make this DBAL middleware instead? Middleware would check if any query fails with "DB went away..." error, if so, reset everything and retry.

Also, bear in mind counterpart for messenger is at https://github.com/symfony/symfony/blob/412ea7afcb533b8a7a95e248d741b70fea505350/src/Symfony/Bridge/Doctrine/Messenger/DoctrinePingConnectionMiddleware.php, so this would be a good opportunity to extract common parts into bridge

[alli83]

I don't like the approach, because as @nicolas-grekas mentioned, this will keep pinging the DB every request, even if normally request wouldn't need connection. Can't we make this DBAL middleware instead? Middleware would check if any query fails with "DB went away..." error, if so, reset everything and retry.

Also, bear in mind counterpart for messenger is at https://github.com/symfony/symfony/blob/412ea7afcb533b8a7a95e248d741b70fea505350/src/Symfony/Bridge/Doctrine/Messenger/DoctrinePingConnectionMiddleware.php, so this would be a good opportunity to extract common parts into bridge

Ok, I'll update then

[alli83]

@ostrolucky
Before refactoring (taking into account DoctrinePingConnectionMiddleware), I'm not entirely sure I grasp the specific middleware you're referring to. Are you referring to those related to Doctrine\DBAL\Driver
Doctrine\DBAL\Driver\Connection

[ostrolucky]

I was referring to middlewares that implement Doctrine\DBAL\Driver\Middleware. This is how Symfony does stuff like logging

[KDederichs]

Any news on this/what's preventing it from being merged?

[nicolas-grekas]

I force-pushed to fix my last comments :)

[dunglas]

🎉

[nicolas-grekas]

Thank you @alli83.

[andersonamuller]

@nicolas-grekas Isn't Symfony\Bridge\Doctrine\Middleware\IdleConnection?

[nicolas-grekas]

Oops, PR welcome

[xabbuh]

fixed in 65ccca0