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

Symfony - persistent connections #5045

Merged
merged 2 commits into from
Sep 9, 2019

Conversation

jderusse
Copy link
Contributor

@jderusse jderusse commented Sep 9, 2019

Current TestSuite is failing because of number of connection to the database.
The solution is to use "persistent connection", but this is not currently supported by Doctrine (see doctrine/dbal#2315) because setting ATTR_PERSISTENT conflict with ATTR_STATEMENT_CLASS.

nb: This issue has been fixed in Doctrine 3 (not yet released nor installable)

This PR patch doctrine to support persistent connection in PHP/symfony

note: Drawback is: caching results won't works anymore. Which not an issue in our case because caching result is not permitted in the benchmark.

This PR also replaces calls to aliases by a call to the original method. Less function calls === more performances. Tell me if you prefer a dedicated PR

ping @raziel057 @kaznovac

Copy link
Contributor

@kaznovac kaznovac left a comment

Choose a reason for hiding this comment

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

LGTM

@raziel057
Copy link
Contributor

Sounds good to me.

@NateBrady23
Copy link
Member

@jderusse Remove the WIP from title when this is ready to merge

@jderusse jderusse changed the title WIP - Symfony - persistent connections Symfony - persistent connections Sep 9, 2019
@jderusse
Copy link
Contributor Author

jderusse commented Sep 9, 2019

Ready to merge @nbrady-techempower

@NateBrady23 NateBrady23 merged commit 8dfa8a2 into TechEmpower:master Sep 9, 2019
@jderusse jderusse deleted the symfony-persist branch May 2, 2020 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants