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

DatabaseMigrations trait exhausts database connections #10619

Closed
mfn opened this issue Oct 15, 2015 · 17 comments
Closed

DatabaseMigrations trait exhausts database connections #10619

mfn opened this issue Oct 15, 2015 · 17 comments

Comments

@mfn
Copy link
Contributor

mfn commented Oct 15, 2015

Using DatabaseMigrations with a lot of tests will exhaust all connections to the dabatase, i.e. they're not freed/returned to the system.

How to reproduce, as suggested from http://laravel.com/docs/5.1/testing#resetting-the-database-after-each-test :

  1. composer.phar create-project laravel/laravel --prefer-dist
  2. cd laravel
  3. Change .env to point to a valid database connecion
  4. Create a test script with many tests, using the trait; e.g. using this bash lines:
echo "<?php class ExhaustionTest extends TestCase {" > tests/ExhaustionTest.php
echo "use Illuminate\Foundation\Testing\DatabaseMigrations;" >> tests/ExhaustionTest.php
for i in $(seq 1 500); do echo "function test$i() {}" >>  tests/ExhaustionTest.php; done
echo "}" >> tests/ExhaustionTest.php
  1. Run the tests with vendor/bin/phpunit

After a some tests they start to fail with the following error (or similar, depending on your DB):
PDOException: SQLSTATE[08004] [1040] Too many connections

Here's an example trace:

1) ExhaustionTest::test143
PDOException: SQLSTATE[08004] [1040] Too many connections

/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/Connectors/Connector.php:55
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/Connectors/MySqlConnector.php:22
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/Connectors/ConnectionFactory.php:60
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/Connectors/ConnectionFactory.php:49
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/DatabaseManager.php:175
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/DatabaseManager.php:67
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/Migrations/DatabaseMigrationRepository.php:171
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/Migrations/DatabaseMigrationRepository.php:139
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/Migrations/Migrator.php:393
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/Console/Migrations/MigrateCommand.php:100
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Database/Console/Migrations/MigrateCommand.php:58
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Container/Container.php:503
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Console/Command.php:150
/home/vagrant/src/test/laravel/vendor/symfony/console/Command/Command.php:259
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Console/Command.php:136
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Console/Application.php:62
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php:159
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Testing/ApplicationTrait.php:299
/home/vagrant/src/test/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Testing/DatabaseMigrations.php:12

To fix this for my suite, I created a trait similar to this:

trait DatabaseMigrationsCleanupTrait
{
    use Illuminate\Foundation\Testing\DatabaseMigrations;

    /**
     * @after
     */
    public function disconnectAllDatabases()
    {
        /** @var \Illuminate\Database\Connection $connection */
        foreach (\DB::getConnections() as $connection) {
            $connection->disconnect();
        }
    }
}

I would open a PR if someone points me in the right direction where the fix would be appropriate.

I tried this, which also worked:

diff --git a/vendor/laravel/framework/src/Illuminate/Foundation/Testing/DatabaseMigrations.php b/vendor/laravel/framework/src/Illuminate/Foundation/Testing/DatabaseMigrations.php
index d53b34e..b4a3205 100644
--- a/vendor/laravel/framework/src/Illuminate/Foundation/Testing/DatabaseMigrations.php
+++ b/vendor/laravel/framework/src/Illuminate/Foundation/Testing/DatabaseMigrations.php
@@ -13,6 +13,9 @@ trait DatabaseMigrations

         $this->beforeApplicationDestroyed(function () {
             $this->artisan('migrate:rollback');
+            foreach (\DB::getConnections() as $connection) {
+                $connection->disconnect();
+            }
         });
     }
 }
@GrahamCampbell
Copy link
Member

This should happen anyway?

@GrahamCampbell
Copy link
Member

When the applications are destroyed the connections should be garbage collected. Have you turned off the garbage collector?

@GrahamCampbell
Copy link
Member

\DB::getConnections()

You definitely wouldn't want to do that. You'd want to call this->app->...

@mfn
Copy link
Contributor Author

mfn commented Oct 15, 2015

Have you turned off the garbage collector?

Not that I'm aware of. Tested with

$ php -v 
PHP 5.6.14-1+deb.sury.org~trusty+1 (cli)
Copyright (c) 1997-2015 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2015 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2015, by Zend Technologies

$ php -i|grep enable_gc
zend.enable_gc => On => On

from https://launchpad.net/~ondrej/+archive/ubuntu/php5 for Ubuntu 14.04 LTS

You definitely wouldn't want to do that. You'd want to call this->app->...

Yeah, I thought so ... :-)

The underlying PDO does not support explicit disconnects and relies on being unrefed, maybe there's a dangling reference kept somewhere.

@pulkitjalan
Copy link
Contributor

Having the same issue with the DatabaseTransactions trait.

$ php -v
PHP 5.5.9-1ubuntu4.11 (cli) (built: Jul  2 2015 15:23:08) 
Copyright (c) 1997-2014 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2014 Zend Technologies
    with Zend OPcache v7.0.3, Copyright (c) 1999-2014, by Zend Technologies
    with Xdebug v2.2.3, Copyright (c) 2002-2013, by Derick Rethans

$ php -i | grep enable_gc
zend.enable_gc => On => On

The above fix solves the issue for me.

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

@mfn
Copy link
Contributor Author

mfn commented Oct 26, 2015

I've switched my project from using the DatbaseMigrations to DatabaseTransactions and have seen the same problem.

I replaced it with my own trait, not only because of this bug, but also because the included DatabaseTransactions only covers the default connections and not others:

<?php

trait MultiDatabaseTransactionsTrait
{
    /**
     * @before
     */
    public function beginDatabaseTransaction()
    {
        /** @var \Illuminate\Database\DatabaseManager $db */
        $db = $this->app->make('db');
        $names = array_keys($this->app['config']['database.connections']);

        foreach ($names as $name) {
            $db->connection($name)->beginTransaction();
        }

        $this->beforeApplicationDestroyed(function () use ($db, $names) {
            foreach ($names as $name) {
                $connection = $db->connection($name);
                $connection->rollBack();
                $connection->disconnect();
            }
        });
    }
}

@mfn
Copy link
Contributor Author

mfn commented Dec 2, 2015

Why closed? This issue isn't solved?

@RomainLanz
Copy link

Look at the PR linked above #10731

@pulkitjalan
Copy link
Contributor

That PR did not fix the issue and does not work. The pr was closed not merged.

@pulkitjalan
Copy link
Contributor

This issue is still not solved.

@RomainLanz
Copy link

Did you read the PR?

cc @GrahamCampbell

@GrahamCampbell
Copy link
Member

Being marked as a no-fix, or technical limitation.

@GrahamCampbell
Copy link
Member

If someone has a really good fix, we're open to PRs. ;)

@knabity
Copy link

knabity commented Feb 2, 2016

I'm having problems with this too. Where is the better solution?

@mfn
Copy link
Contributor Author

mfn commented Feb 3, 2016

@KNabityMailServicesLC just being an observer here, but as I see that the DatabaseTransaction code hasn't changed substantially since I created this issue, I think my #10619 (comment) still holds.

I can confirm that it works insofar as the codebase is heavily developed on and the tests run locally and in our CI infrastructure many many times a day without known problems.

@cythrawll
Copy link

So I seemed to solve this myself by setting persistent connections on in the configuration environment. I make it an env(), and set it to true in phpunit.xml. Not the best solution, but it seems to fix the issue.

@lehnen
Copy link

lehnen commented Mar 11, 2016

@cythrawll: Saved my day ;)

     'mysql' => [
            'driver'    => 'mysql',
            'host'      => env('DB_HOST'    , 'localhost'),
            'database'  => env('DB_DATABASE', 'forge'),
            'username'  => env('DB_USERNAME', 'forge'),
            'password'  => env('DB_PASSWORD', ''),
            'charset'   => 'utf8',
            'collation' => 'utf8_unicode_ci',
            'prefix'    => '',
            'strict'    => false,
            'options'   => array(
                PDO::ATTR_PERSISTENT => env('DB_PERSISTENT', false),
            ),
        ]

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

7 participants