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 Too many connections when running test suite #18471

Closed
ryrobbo opened this issue Mar 24, 2017 · 25 comments
Closed

PDO Too many connections when running test suite #18471

ryrobbo opened this issue Mar 24, 2017 · 25 comments

Comments

@ryrobbo
Copy link

ryrobbo commented Mar 24, 2017

  • Laravel Version: 5.4.16
  • PHP Version: 7.1
  • Database Driver & Version: MySQL

Description:

I have a test suite which now has 140 tests. Each of my tests uses the DatabaseMigrations trait along with a dedicated database solely for tests to run against.

When PHP Unit gets to the last test it fails with the following error:

Caused by
Doctrine\DBAL\Driver\PDOException: SQLSTATE[08004] [1040] Too many connections

I can run this test individually and it passes fine.

After finding this issue (#10619 (comment)) I tried the following but the problem still persists:

  • Added the PDO::ATTR_PERSISTENT => false option to my test database connection entry
  • Added the following code to the DatabaseMigrations trait:
$this->beforeApplicationDestroyed(function () {
    $this->artisan('migrate:rollback');

    foreach ($this->app->make('db')->getConnections() as $connection) {
        $connection->disconnect();
    }
});

Steps To Reproduce:

N/A

@Kyslik
Copy link
Contributor

Kyslik commented Mar 26, 2017

Its problem with max_connections.

Naive solution is to increase max_connections using (run query)

set global max_connections = 200;

To check current max_connections

show variables like "max_connections";

I tested max_connections = 10 and run 60 tests and it failed with

Caused by
PDOException: SQLSTATE[HY000] [1040] Too many connections

@sisve
Copy link
Contributor

sisve commented Mar 27, 2017

I believe the problem is that the connection should be reused (or a few of them pooled), not that the database has a too low max_connections value.

@Kyslik
Copy link
Contributor

Kyslik commented Mar 27, 2017

What exactly do you mean reused? Connection is closed after each test, and until its closed new one opens up...

I tested this using Innotop on my local machine (installed from homebrew), I have 69 test total, and each test class uses DatabaseMigrations trait and Innotop showed 69 connections. I guess PHPUnit is fast so connections are not closed ASAP.

I found this post about this issue.

public function tearDown()
{
    $this->beforeApplicationDestroyed(function () {
        DB::disconnect();
    });

    parent::tearDown();
}

After that max_connection stats were ±3.

@patinthehat
Copy link

patinthehat commented Apr 6, 2017

I had a similar problem not too long ago in Lumen, and solved it with:

    public function tearDown()
    {
        \DB::connection()->setPdo(null);
    }

@arubacao
Copy link
Contributor

arubacao commented Apr 8, 2017

I can also confirm this issue.
Manually closing database connections solved the issue.
Contrary to @ryrobbo i hook into the DatabaseTransactions Traits beforeApplicationDestroyed method.
Simply override the setUpTraits method in TestCase. Thereby you bypass overkill solutions like @patinthehat or @Kyslik since most likely not every test interacts with the db.

    protected function setUpTraits()
    {
        parent::setUpTraits();

        $uses = array_flip(class_uses_recursive(static::class));

        if (isset($uses[DatabaseTransactions::class])) {
            $database = $this->app->make('db');
            $this->beforeApplicationDestroyed(function () use ($database) {
                foreach ($this->connectionsToTransact() as $name) {
                    $database->connection($name)->disconnect();
                }
            });
        }
    }

@ryrobbo
Copy link
Author

ryrobbo commented Apr 10, 2017

@arubacao

Do you use the DatabaseMigration's trait in your tests?

Your solution looks like it's only applicable if you're using the DatabaseTransations trait.

@arubacao
Copy link
Contributor

arubacao commented Apr 11, 2017

@ryrobbo yes, that is correct.
I just tried this as well (without the code from the previous post):

if (isset($uses[DatabaseMigrations::class])) {
            $this->beforeApplicationDestroyed(function () {
                $this->app->make('db')->connection('mysql')->disconnect();
            });
        }

And it worked for me. I have to be explicit here, because i have a mongodb connection open which doesn't cause any problems.

@ryrobbo
Copy link
Author

ryrobbo commented Apr 28, 2017

@arubacao

I have modified my TestCase class as follows:

abstract class TestCase extends BaseTestCase
{
    use CreatesApplication;

    protected function setUpTraits()
    {
        parent::setUpTraits(); // TODO: Change the autogenerated stub

        $uses = array_flip(class_uses_recursive(static::class));

        if (isset($uses[DatabaseMigrations::class])) {
            $this->beforeApplicationDestroyed(function () {
                $this->app->make('db')->connection('mysql_test')->disconnect();
            });
        }
    }
}

Note: I am using an database connection called "mysql_test" to run my migrations during tests.

But I am still getting the too many connections error.

@themsaid themsaid removed the database label Jun 7, 2017
@coquer
Copy link

coquer commented Jun 13, 2017

The best way we find to fix this issue was to move the test to sqlite driver:

'testing' => [
     'driver' => 'sqlite',
    'database' => storage_path('testing/phpunit.sqlite'),
    'prefix'   => '',
 ]

and my phpunit.xml

    <php>
        <env name="APP_ENV" value="testing"/>
        <env name="DB_CONNECTION" value="testing"/>
        <env name="CACHE_DRIVER" value="array"/>
        <env name="SESSION_DRIVER" value="array"/>
        <env name="QUEUE_DRIVER" value="sync"/>
    </php>

With this we saved around 90% of time running our test cases.

@lindyhopchris
Copy link

Also confirm this issue on PostgresSQL 9.4

@jycr753 tip is good to use sqlite - we generally do that but in this instance we're trying to test specifically against Postgres as that is what is used in production so makes more sense to integration test with the "real" database.

@ryrobbo
Copy link
Author

ryrobbo commented Jun 15, 2017

For anyone coming across this issue now, I've followed @Kyslik 's advice and just manually set a high connections value (set global max_connections = 800;).

Not the perfect solution but it's allowing me to run all of my tests.

@coquer
Copy link

coquer commented Jun 18, 2017

@ryrobbo the problem with that is that it does become a exponential issue, before I made the jump to sqlite for testing I was setting the connections to 4000

@bryandatanerds
Copy link

We are using Doctrine via Connection::getDoctrineSchemaManager(), and it seems like the Doctrine connection (and hence the PDO) doesn't get cleaned up at the end of each of our unit tests, despite going \DB::purge(...). If we do the following instead, it solves this issue for us:

$connection->getDoctrineConnection()->close();
\DB::purge($connectionName ?: null);

Is this perhaps the issue you're facing as well?

And is it intentional to have to clean up a Doctrine connection like this?

(PHP 7.1, laravel/framework 5.4.27, doctrine/dbal 2.5.12, Postgres 9.5.7)

@Kyslik
Copy link
Contributor

Kyslik commented Jun 23, 2017

@jycr753 There is always possibility to use sqlite driver with :memory: database, which is perfect for testing.

'testing' => [
    'driver' => 'sqlite',
    'database' => ':memory:',
    'prefix' => '',
],

But remember to enable constrains checks https://sqlite.org/foreignkeys.html

@tonychuuy
Copy link

I'm getting this error in Laravel 5.5.2, I'm not using DatabaseTransactions trait, I have a dedicated database to run integration tests.

Laravel Version: 5.5.2
PHP Version: 7.1
Database Driver & Version: MySQL

Running a test suite of 234 tests.

@tonychuuy
Copy link

Adding this option in the testing database solve the problem in Laravel 5.5.2

'options'   => array(
    PDO::ATTR_PERSISTENT => env('DB_PERSISTENT', true),
),

@Kyslik
Copy link
Contributor

Kyslik commented Nov 5, 2017

@themsaid is this still an issue? #20340 was merged and people over there reported that it works.

I guess this can be closed.

@IlCallo
Copy link

IlCallo commented Nov 17, 2017

Still getting this error in L5.4 from a test suite of 148 tests and going up.

Laravel Version: 5.4.36
PHP Version: 7.1
Database Driver & Version: MySQL
PHPUnit 6.4.4

@avirdz solution worked, but it's obviously a workaround

I think this is related to the fact that I forced the application creation into TestCase constructor

public function __construct($name = null, array $data = [], $dataName = '')
{
    parent::__construct($name, $data, $dataName);

    $this->createApplication();
}

in order to use factories and access data already present in the database into dataProviders.
Too many connections error is fired only for the test group which uses dataProviders and for all tests inside that group, so I guess it reaches the maximum connections right when generating the dataProviders output.

Dunno if I should open a new issue or keep with this one.
Dunno also if this problem can be solved on the Laravel side: the cause is PHPUnit code design and workflow.

@Kyslik
Copy link
Contributor

Kyslik commented Nov 17, 2017

@IlCallo (great name btw, and I am being ironic...) L5.4 is not supported anymore.

@IlCallo
Copy link

IlCallo commented Nov 17, 2017

Of course, but the problem could be present also on 5.5, I'll check when upgrading.
And @avirdz said he experienced a "Too many connection" on 5.5.2

@GuidoHendriks
Copy link
Contributor

GuidoHendriks commented Dec 7, 2017

I'm currently experiencing this problem on 5.5.24.

Adding this to my config solved it:

'options'   => array(
    PDO::ATTR_PERSISTENT => env('DB_PERSISTENT', true),
),

@GuidoHendriks
Copy link
Contributor

Persistent DB did not solve it after all. Original solution from @ryrobbo did.

@mfn
Copy link
Contributor

mfn commented Dec 16, 2017

Many of things here sound similar to #18056 . The major difference being: the other is talking about DatabaseTransactions, this one only mentions DatabaseMigrations.

From what I read here, this issue suffers the same problem in the end: connections are not free'd back.

Can anyone affected here try and use the DatabaseTransactions together with the migrations and see if it "fixes" it? Afaik MySQL doesn't support DDLs in transactions but \Illuminate\Foundation\Testing\TestCase::setUpTraits first runs the migrations and then activates the transactions (which should close the connections).

@GuidoHendriks
Copy link
Contributor

I just noticed that both RefreshDatabase and DatabaseTransactions use similar methods called beginDatabaseTransaction. They do exactly the same, except for 1 thing: DatabaseTransactions not only rolls back, but disconnects as well. Where RefreshDatabase only rolls back.

https://github.com/laravel/framework/blob/5.5/src/Illuminate/Foundation/Testing/RefreshDatabase.php
https://github.com/laravel/framework/blob/5.5/src/Illuminate/Foundation/Testing/DatabaseTransactions.php

I can’t test it at this moment. But I’m pretty this is the problem. The solution by the issue starter fixes it, and that just adds the missing disconnect.

@nesk
Copy link
Contributor

nesk commented Dec 29, 2017

@GuidoHendriks was right, I've just replaced:

$database->connection($name)->rollBack();

by:

$connection = $database->connection($name);

$connection->rollBack();
$connection->disconnect();

in the RefreshDatabase trait, and now it works like a charm! I wrote a PR (#22569) to fix that.

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

No branches or pull requests