Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register the idle connection listener #1739

Conversation

alli83
Copy link
Contributor

@alli83 alli83 commented Dec 26, 2023

linked to symfony/symfony#53214

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.
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.

@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch from d1bd383 to 88471c3 Compare December 26, 2023 12:03
@ostrolucky ostrolucky added the Status: On Hold Most likely waiting for upstream resolution label Dec 30, 2023
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch from 88471c3 to 536a096 Compare January 3, 2024 06:35
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch 3 times, most recently from eaacc08 to cfda672 Compare March 12, 2024 04:02
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch 4 times, most recently from 1f57511 to a9fb4dc Compare March 20, 2024 03:18
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch 4 times, most recently from c883146 to 5192741 Compare March 20, 2024 03:35
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch from 5192741 to 8e0bbc6 Compare April 4, 2024 12:36
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch 2 times, most recently from 8b3a078 to 36a7031 Compare April 16, 2024 01:02
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch 3 times, most recently from 5117c16 to b4122a1 Compare April 17, 2024 07:35
Middleware/ConnectionKeepAlive.php Outdated Show resolved Hide resolved
Middleware/ConnectionKeepAlive.php Outdated Show resolved Hide resolved
Middleware/ConnectionKeepAlive.php Outdated Show resolved Hide resolved
Middleware/ConnectionKeepAlive.php Outdated Show resolved Hide resolved
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch 4 times, most recently from a8f1faf to 87ac9a4 Compare April 18, 2024 14:28
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch 12 times, most recently from 7ffcd85 to bb1034e Compare May 1, 2024 09:04
@nicolas-grekas nicolas-grekas changed the base branch from 2.11.x to 2.13.x May 1, 2024 09:54
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM thanks!
Maybe squash + sync the commit title and PR title?
For the psalm failure, something like this might do it in psalm.xml.dist:

        <UndefinedClass>
            <errorLevel type="suppress">
                <!-- [...] -->
                <referencedClass name=" use Symfony\Bridge\Doctrine\Middleware\IdleConnection\Listener"/>
            </errorLevel>
        </UndefinedClass>

@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch 5 times, most recently from a3c1c3f to ccf7ac5 Compare May 2, 2024 09:04
psalm.xml.dist Show resolved Hide resolved
src/DependencyInjection/DoctrineExtension.php Outdated Show resolved Hide resolved
src/Middleware/IdleConnectionMiddleware.php Show resolved Hide resolved
@ostrolucky ostrolucky removed the Status: On Hold Most likely waiting for upstream resolution label May 2, 2024
@alli83 alli83 force-pushed the long-runnning-process-doctrine-connection-listener branch from ccf7ac5 to 6bca630 Compare May 2, 2024 13:09
@nicolas-grekas nicolas-grekas force-pushed the long-runnning-process-doctrine-connection-listener branch from 6bca630 to 166c5f8 Compare May 3, 2024 09:49
* @param ArrayObject<string, int> $connectionExpiries
* @param array<string, int> $ttlByConnection
*/
public function __construct(ArrayObject $connectionExpiries, array $ttlByConnection)

Choose a reason for hiding this comment

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

Suggested change
public function __construct(ArrayObject $connectionExpiries, array $ttlByConnection)
public function __construct(ArrayObject $expiringConnections, array $ttlByConnection)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I'm not mistaken, this is a list of expiries and not connections. There were discussions regarding the naming of variables above, and it was decided to use connectionExpiries

@ostrolucky ostrolucky merged commit 4bb9f20 into doctrine:2.13.x May 5, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants