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

pdo_mysql with persistent connection #2315

Closed
jeff1985 opened this issue Feb 15, 2016 · 39 comments
Closed

pdo_mysql with persistent connection #2315

jeff1985 opened this issue Feb 15, 2016 · 39 comments

Comments

@jeff1985
Copy link

While trying to use doctrine with persistent pdo_mysql connections, it seemed that this is not supported.
To be clear, when you pass PDO::ATTR_PERSISTENT => true as config option for connection, an exception will be raised stating that General error: PDO::ATTR_STATEMENT_CLASS cannot be used with persistent PDO instances.

After some research, i found this answer on stackoverflow suggesting to pass a pre-created PDO instance as configuration parameter like this:

$conn = array('pdo' => $dbh);`
$this->entityManager = EntityManager::create($conn, $config);

It does work and my application is able use doctrine as usual. BUT it feels strange. My understanding is that this way we fool doctrine to use pdo without the statement class (which is by the way Doctrine\DBAL\Driver\PDOStatement). Can someone please point out, if there are any risks of using doctrine like that?

I also suggest to add the corresponding info regarding persistent connection to the docs.

@Ocramius
Copy link
Member

this way we fool doctrine

Passing in a PDO instance is the correct way to handle edge cases in the configuration DSL of the DriverManager, actually.

@jeff1985
Copy link
Author

Yes, but do you need a pdo instance configured to work with Doctrine\DBAL\Driver\PDOStatement? If you don't, then what is the purpose of this class?

@Ocramius
Copy link
Member

@jeff1985 doctrine wraps around a number of drivers, and many of them are not PDO-based, so we built internal abstractions to get rid of that impedance.

Doctrine\DBAL\Driver\PDOStatement would still be used.

@jeff1985
Copy link
Author

@Ocramius
OK, i understand, why you've built a wrapper class.

You write, that Doctrine\DBAL\Driver\PDOStatement would still be used. How do you mean that? The whole point of providing an own pdo instance is not to be using this class. Because custom statement class is not supported by PDO for persistent connections.

So the actual question is will using the plain \PDOStatement class break any of doctrine code? So far i can see the only change that this class does is to wrap \PDOException with Doctrine\DBAL\Driver\PDOException. As far as i can see, the code still uses \PDOException everywhere, so the change actually would not break anything.

@Ocramius
Copy link
Member

will using the plain \PDOStatement class break any of doctrine code?

You have to test that against doctrine's test suite. Doctrine sets the statement in PDOConnection::__construct, so it cannot be worked around at the moment.

I didn't know this:

Because custom statement class is not supported by PDO for persistent connections.

Is there a documentation about it somewhere? Sounds like a weird issue...

@jeff1985
Copy link
Author

@Ocramius

Doctrine sets the statement in PDOConnection::__construct, so it cannot be worked around at the moment.

This is not true. We are speaking of a workaround here. PDOConnection is created by PDOMySql\Driver::connect(). The connect() method won't get called, if you've provided a ready pdo instance.

Is there a documentation about it somewhere? Sounds like a weird issue...

It is documented in the php manual:

PDO::ATTR_STATEMENT_CLASS: Set user-supplied statement class derived from PDOStatement. Cannot be used with persistent PDO instances.

@mikemix
Copy link

mikemix commented Apr 4, 2016

👍 Can't use persistent connection in Doctrine :(

@jeff1985
Copy link
Author

@mikemix
What exactly do you mean? Did you read my fist message and the answer on stack overflow?

@mikemix
Copy link

mikemix commented Apr 29, 2016

I introduced a better solution in my app and used the tick() method in my Ratchet server which pings the connection from time to time.

@jeff1985
Copy link
Author

@mikemix
Sorry, but i do not understand. Are you using websockets instead of database? The topic here was how to use persistent connection with doctrine.

@mikemix
Copy link

mikemix commented Apr 29, 2016

I am using a Doctrine connection in a really long running socket app.

@jeff1985
Copy link
Author

@mikemix
so how is that relevant to the topic discussed here?

@mikemix
Copy link

mikemix commented Apr 29, 2016

After some time the server was useless because the connection to the database was lost?

@jeff1985
Copy link
Author

@mikemix
again, how is this relevant? This topic is not about lost connections!

@mikemix
Copy link

mikemix commented Apr 29, 2016

Prove me wrong I thought this topic was about making a persistent connection to the database with Doctrine.

@jeff1985
Copy link
Author

@mikemix
Yes, this topic is about making a persistent connection to the database with Doctrine. Your answers are lacking any details, but from what i understood, you've had timeout problems with your connection.
I don't think it is related to the kind of connection (persistent/non-persistent). Please open your own topic for this.

@alexander-schranz
Copy link

alexander-schranz commented Jun 3, 2016

the problem seems to be this line: https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/PDOConnection.php#L44

but I don't know if this is required to work, else something like this could fix it:

if (!isset($options[PDO::ATTR_PERSISTENT]) || !$options[PDO::ATTR_PERSISTENT]) {
    $this->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('Doctrine\DBAL\Driver\PDOStatement', array()));
}

symfony-splitter pushed a commit to illuminate/database that referenced this issue Dec 8, 2016
Doctrine DBAL currently does not work with persistent connections.
doctrine/dbal#2315
@odiaseo
Copy link

odiaseo commented Jan 10, 2017

You can specify a driverClass as part of the doctrine connection parameters then overwrite the connect method to create the PDO connection.

@qRoC
Copy link

qRoC commented Jan 13, 2017

@odiaseo, yes, but for Doctrine\DBAL\Cache\ResultCacheStatement need Doctrine\DBAL\Driver\Statement

@timmetj
Copy link

timmetj commented Jun 6, 2017

Still no solution how to use persistent connection? I also get the ResultCacheStatement error.

@Ocramius
Copy link
Member

Ocramius commented Jun 6, 2017

As I mentioned in the first response:

this way we fool doctrine

Passing in a PDO instance is the correct way to handle edge cases in the configuration DSL of the DriverManager, actually.

@timmetj
Copy link

timmetj commented Jun 6, 2017

Can you give an example please? Where to pass the PDO instance?
I have tried following:

$dbh = new PDO('mysql:host=localhost;dbname=test', $user, $pass, array(
    PDO::ATTR_PERSISTENT => true
    PDO::ATTR_STATEMENT_CLASS => array('Custom', array($dbh))
));
$conn = array('pdo' => $dbh);`
$this->entityManager = EntityManager::create($conn, $config);

But this gives the PDO::ATTR_STATEMENT_CLASS cannot be used with persistent PDO instances
and without the statement de resultcache is failing

@Ocramius
Copy link
Member

Ocramius commented Jun 6, 2017

@timmetj the config above seems correct: that's indeed how it's supposed to be done.

Now that makes the limitation clear. I think this cannot be solved on the doctrine side, but needs to be reported as a bug in PHP/PDO core (if there isn't already an issue for this)

@drscre
Copy link

drscre commented Dec 6, 2017

The downside of explicitly passing \PDO instance is that your connection stops being lazy.

@mattboothman
Copy link

mattboothman commented Feb 20, 2018

Looks like when using the workaround, passing an instance of \PDO to DriverMapper, does have some side-effects:

When \PDO throws an exception, which will not be in instance of DriverException, DBAL Connection will catch \Exception and will pass it to DBALException::driverExceptionDuringQuery which will not pass the exception to the driver's convertException() method.

Exceptions thrown by \PDO will not get converted to DBAL related exceptions like UniqueConstraintViolationException, for example.

Am I correct in assuming this? Is there a work around?

@raziel057
Copy link

It would be interesting to find a way to define persistent connection when using Doctrine to avoid errors on benchmarks with high traffic.

See comments in TechEmpower/FrameworkBenchmarks#4293

@Ocramius
Copy link
Member

Ocramius commented Jan 10, 2019

Persistent connections are generally not endorsed, and are not the way real world applications written in PHP should be configured. Do not write benchmarks that do not represent a real world scenario just for the sake of winning at benchmarks.

@qRoC
Copy link

qRoC commented Jan 10, 2019

real world scenario

So one solution in "real world" is to use proxy for resolving this issue.

@raziel057
Copy link

It's your opinion (and it was mine because I generally use transactions and want to avoid specific issues that can appears with persistent connection) but it seems to not be true for other peoples as you can read here:
TechEmpower/FrameworkBenchmarks#4293 (comment)

Do you have technical information that prove it? Nothing seems to indicate it should not be used. Even here it's just say that it can be useful to improve performances (depending on the business you have). You can have sites with high traffic where transactions are not used in real world.

http://php.net/manual/en/features.persistent-connections.php

@qRoC
Copy link

qRoC commented Jan 10, 2019

@raziel057, yes 45krps. Impossible work without proxy(open connection, TIME_WAIT, CLOSE_WAIT, etc). ProxySQL fixes that problems, but has other issues.

@joanhey
Copy link

joanhey commented Jan 10, 2019

real world applications written in PHP

It isn't about PHP or any other language.
Improving performance by understanding more about the underlying network protocols.
Also from Java, node.js, ruby, python, ... all use persistent connections.

In a real application we possibly have the database in another server.
If we need to create a new TCP connection every time, it is really slow ( 3-way handshake, low start, ...).
And if the connection use SSL is very resource intensive and slower.
Creating new TCP connections can take a lot of resources such as CPU and memory usage. Keeping connections alive longer can reduce this usage.

About using transactions and others issues are things of the past.
oci8 extension don't have this problem.
And the new mysqlnd don't have this problem, http://php.net/manual/en/mysqli.persistconns.php and http://php.net/manual/en/mysqlnd.persist.php
I don't know about other db extensions.

@raziel057
Copy link

I tried to disable the user-supplied statement class in case we turn ON the persistent connection.

In PDOConnection:

if (!isset($options[PDO::ATTR_PERSISTENT]) || !$options[PDO::ATTR_PERSISTENT]) {
    $this->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('Doctrine\DBAL\Driver\PDOStatement', array()));
}

I have run my test suites (phpUnit / behat) on 2 projects that use Doctrine and tried manually the web application and at this stage I can't detect any issues.

@morozov
Copy link
Member

morozov commented Jan 10, 2019

FWIW, as part of #2958, we're not using a custom statement class at all. Could you check if your case works on develop w/o extra code changes?

@raziel057
Copy link

raziel057 commented Jan 11, 2019

I currently use the following doctrine packages:

composer show doctrine/*
doctrine/annotations              v1.6.0 Docblock Annotations Parser
doctrine/cache                    v1.7.1 Caching library offering an object-oriented API for many cache backends
doctrine/collections              v1.5.0 Collections Abstraction library
doctrine/common                   v2.8.1 Common Library for Doctrine projects
doctrine/data-fixtures            v1.3.1 Data Fixtures for all Doctrine Object Managers
doctrine/dbal                     v2.7.1 Database Abstraction Layer
doctrine/doctrine-bundle          1.9.1  Symfony DoctrineBundle
doctrine/doctrine-cache-bundle    1.3.3  Symfony Bundle for Doctrine Cache
doctrine/doctrine-fixtures-bundle v2.4.1 Symfony DoctrineFixturesBundle
doctrine/inflector                v1.3.0 Common String Manipulations with regard to casing and singular/plural rules.
doctrine/instantiator             1.1.0  A small, lightweight utility to instantiate objects in PHP without invoking their constructors
doctrine/lexer                    v1.0.1 Base library for a lexer that can be used in Top-Down, Recursive Descent Parsers.
doctrine/orm                      v2.6.1 Object-Relational-Mapper for PHP

In my composer json

"require" : {
		"php" : ">=7.1",
		"symfony/symfony" : "3.4.*",
		"doctrine/orm" : "^2.5",
		"doctrine/doctrine-bundle" : "^1.6",
                ...
	},

I tried to add "doctrine/dbal" : "dev-develop" and "doctrine/common" : "^2.9" but I still have dependency conflicts. I will check later. What is your opinion about the small modification in PDOConnection to be able to enable the persistent connection for high traffic sites (and benchmarks).

Currently it's not fair play because all PHP projects using Doctrine have bad results / errors in last runs because of not using a persistent connection (all others Frameworks use it):
https://www.techempower.com/benchmarks/#section=test&runid=58770c2f-a4bb-478f-8b4b-7ef77e5e25b6

It would be great to avoid to give this bad image for such a good solution.

@morozov
Copy link
Member

morozov commented Jan 14, 2019

What is your opinion about the small modification in PDOConnection to be able to enable the persistent connection for high traffic sites (and benchmarks).

As proposed in #2315 (comment), it's a breaking change. The statement objects instantiated by Doctrine\DBAL\Driver\Connection implementations are expected to implement the Doctrine\DBAL\Driver\Statement interface which will be broken (a plain PDOStatement will be returned).

Probably, certain non-breaking parts of #2958 (the changes in Doctrine\DBAL\Driver\PDOConnection and Doctrine\DBAL\Driver\PDOStatement) could be back-ported to master.

@morozov
Copy link
Member

morozov commented Jan 14, 2019

Actually, the fact that Doctrine\DBAL\Driver\PDOStatement no longer extends PDOStatement itself can be considered a breaking change. E.g. one won’t be able to call getAttribute() or setAttribute() on the statement object. Not sure if it's part the of the API or an implementation detail.

/cc @Ocramius

@morozov
Copy link
Member

morozov commented Jun 6, 2019

Resolved by #3515.

@morozov morozov closed this as completed Jun 6, 2019
@raziel057
Copy link

That's a very good news! Thanks

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests