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

Too many connections error during testing with 2 databases #23513

Closed
denitsa-md opened this issue Mar 13, 2018 · 12 comments
Closed

Too many connections error during testing with 2 databases #23513

denitsa-md opened this issue Mar 13, 2018 · 12 comments

Comments

@denitsa-md
Copy link

denitsa-md commented Mar 13, 2018

  • Laravel Version: 5.6.8
  • PHP Version: 7.2
  • Database Driver & Version: mysql 5.7.21

Description:

I am getting a Too many connections error during phpunit testing with a test suite of about 150 tests, starting after approximately half of the test have run.

Error stack:

/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:43
/vendor/laravel/framework/src/Illuminate/Database/Connectors/Connector.php:65
/vendor/laravel/framework/src/Illuminate/Database/Connectors/Connector.php:44
/vendor/laravel/framework/src/Illuminate/Database/Connectors/MySqlConnector.php:24
/vendor/laravel/framework/src/Illuminate/Database/Connectors/ConnectionFactory.php:183
/vendor/laravel/framework/src/Illuminate/Database/Connection.php:915
/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:108
/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:92
/vendor/laravel/framework/src/Illuminate/Foundation/Testing/RefreshDatabase.php:73

Steps To Reproduce:

I have 2 mysql connections, lots of tests making calls to model factories, and the following in a base test class extending from Illuminate\Foundation\Testing\TestCase:

    protected $connectionsToTransact = ['main', 'tenant'];

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

        parent::tearDown();
    }
@tomhatzer
Copy link

Ok before digging deeper into this, can you try and remove the () after $this->connectionsToTransact and see if this still happens?

@denitsa-md
Copy link
Author

denitsa-md commented Mar 13, 2018

@tomhatzer I've tried with $this->connectionsToTransact alone but no luck.

I've also tried hard-coding them like:

DB::disconnect('tenant');
DB::disconnect('main');

or like

DB::connection('main')->disconnect();
...

or like

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

or like

DB::connection('main')->setPdo(null);
...

among others.

@tomhatzer
Copy link

@denitsa-cm Ok good... Next question: When do you create the new database connections? Do you create them in a wrapper class that executes all your tests or do you create a new connection within every single test class?
Also, when do you close them? In a wrapper class after all tests are done or in every single test after being done?

@denitsa-md
Copy link
Author

denitsa-md commented Mar 13, 2018

@tomhatzer

  • I have a base TestCase, which all tests extend from that looks like this:
namespace Tests;

use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
use Illuminate\Support\Facades\DB;

abstract class TestCase extends BaseTestCase
{
    use CreatesApplication;

    protected $connectionsToTransact = ['main', 'tenant'];

    public function tearDown()
    {
        $this->beforeApplicationDestroyed(function () {
            foreach ($this->connectionsToTransact as $connection) {
                DB::connection($connection)->setPdo(null);
            }
        });

        parent::tearDown();
    }
  • I am overriding two of the methods in the RefreshDatabase trait in order to migrate 2 databases instead of just one (basically just adding the tenant database). My trait uses the default trait.

I manually use the trait in all my tests. (Simply because I like to be able to turn it on and off for a test).

<?php

namespace Tests;

use Illuminate\Contracts\Console\Kernel;
use Illuminate\Foundation\Testing\RefreshDatabase as BaseRefreshDatabase;
use Illuminate\Foundation\Testing\RefreshDatabaseState;

trait RefreshDatabase
{
    use BaseRefreshDatabase;

    /**
     * Refresh the in-memory database.
     *
     * @return void
     */
    protected function refreshInMemoryDatabase()
    {
        $this->artisan('migrate');

        $path = 'database' . DIRECTORY_SEPARATOR . 'migrations' . DIRECTORY_SEPARATOR . 'tenants';

        $this->artisan('migrate', [
            '--database' => 'testing',
            '--path' => $path
        ]);

        $this->app[Kernel::class]->setArtisan(null);
    }

    /**
     * Refresh a conventional test database.
     *
     * @return void
     */
    protected function refreshTestDatabase()
    {
        if (!RefreshDatabaseState::$migrated) {
            $this->artisan('migrate:fresh');

            $path = 'database' . DIRECTORY_SEPARATOR . 'migrations' . DIRECTORY_SEPARATOR . 'tenants';

            $this->artisan('migrate:fresh', [
                '--database' => config('database.tenant'),
                '--path' => $path
            ]);

            $this->app[Kernel::class]->setArtisan(null);

            RefreshDatabaseState::$migrated = true;
        }

        $this->beginDatabaseTransaction();
    }
}

I would assume the connections are open in $this->beginDatabaseTransaction and thought they were closed there as well (after reading #18471 (comment)). However, since it doesn't seem they get closed, I tried to do it in the tearDown() in the TestCase class (so after every test), which still doesn't work. Maybe I'm doing something wrong 🤔 Still relatively new to Laravel, so please excuse my ignorance. :)

@denitsa-md
Copy link
Author

denitsa-md commented Mar 13, 2018

@tomhatzer It is also worth noting that during tests I have calls to factories that look like this:

<?php

use Faker\Generator as Faker;

$factory->define(App\Vendor::class, function (Faker $faker) {
    return [
        'vendor_type_id' => function () {
            return factory(\App\VendorType::class)->create()->id;
        },
        'owner_id' => function () {
            return factory(\App\User::class)->create()->id;
        },
        'name' => $faker->name,
        'alternative_name' => $faker->name,
        'abbreviation' => $faker->name,
        'website' => $faker->url,
        'currency' => $faker->currencyCode,
        'active' => $faker->boolean,
        'sushi' => $faker->boolean,
        'ringgold_id' => $faker->slug
    ];
});

Note that a Vendor in this case is on a tenant connection (set in a BaseTenantModel), while a User is on a main connection. Not sure if that could be causing some weirdness.

P.S. Just tried replacing all calls from a tenant connection model factory to a User model factory on the main connection with a randomNumber() instead..same result.

@tomhatzer
Copy link

@denitsa-cm Ok I see...

Can you try this class as your base class?

namespace Tests;

use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
use Illuminate\Support\Facades\DB;

abstract class TestCase extends BaseTestCase
{
    use CreatesApplication;

    protected $connectionsToTransact = ['main', 'tenant'];

    public function tearDown()
    {
        $database = $this->app->make('db');
        $this->beforeApplicationDestroyed(function () use ($database) {
            foreach ($this->connectionsToTransact as $connection) {
                $conn = $database->connection($connection);
                $conn->rollBack();
                $conn->disconnect();
            }
        });

        parent::tearDown();
    }
}

This should rollback all changes and disconnect everything afterwards.

Do you get any log output from Laravel during the tests? Any errors or anything like that?

The basic problem is, as in the other issues belonging to database connections, that the connections don't get closed.
Do you have persistent connections enabled in your database server?
Something like this:

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

@denitsa-md
Copy link
Author

denitsa-md commented Mar 15, 2018

@tomhatzer Just tried it and ran phpunit full of hope but still no :(

Class looks like this:

use Illuminate\Foundation\Testing\TestCase as BaseTestCase;

abstract class TestCase extends BaseTestCase
{
    use CreatesApplication;

    protected $connectionsToTransact = ['main', 'tenant'];

    public function tearDown()
    {
        $database = $this->app->make('db');
        $this->beforeApplicationDestroyed(function () use ($database) {
            foreach ($this->connectionsToTransact as $connection) {
                $conn = $database->connection($connection);
                $conn->rollBack();
                $conn->disconnect();
            }
        });

        parent::tearDown();
    }
}

This has really been bothering me for a while :)

@tomhatzer
Copy link

@denitsa-cm do you possibly have a public repository with a little test implementation and example code? I don't think we'll get any further here with the current way.

@denitsa-md
Copy link
Author

@tomhatzer I have temporarily increased the database connections limit to be able to run the tests. I will try to reproduce it in a clean repository soon. The things is the error only happens with many tests and multiple database connections so I'll need some time to set it up. Will post here when I've done that! :) Thank you for your help!

@laurencei
Copy link
Contributor

If you can reproduce in an clean repo - please let us know

@indreka
Copy link

indreka commented Apr 6, 2021

This is still broken. In unittest teardown function, do
$connection = DB::connection();
parent::tearDown();
print_r($connection->getPdo());

Ideally Application->flush() should also force close all connections, so if there are any memory leaks that keep references to connection object, the actual db connection gets closed off. Or better yet, fix circular references/memory leaks so that connection objects are released.
We are using v7.30.4 which is not officially updated anymore.

Testing for memory leaks proof with xdebug extension should be easy:
inside tearDown() store connection class and pdo itself locally

$connection = DB::connection();
$pdo = $connection->getPdo();
parent::tearDown();
xdebug_debug_zval('connection');
xdebug_debug_zval('pdo');

if Application->flush() released everything properly and closed connections, xdebug should show that 'pdo' and 'connection' variables are objects, which have only 1 reference (local variables in function). If the number of references is 2 or more, then there is a memory/connection leak which makes tests unrunnable for big projects.

@Dylan-Buth
Copy link

This was one of the only github issues I've found referencing this issue, so in case this helps anyone else here's what we're doing. We assumed Laravel's teardown functionality would close these connections but monitoring our mysql container during a fairly large testsuite, at a certain point we get all failures and a blast of Too many connections from the container.

Also ran select host,count(host) from information_schema.processlist group by host in our testing database which return 152 (our max I guess) while running the tests initially, then ~7 while running them after the update.

Note: We're using https://tenancyforlaravel.com/

    protected function setUp(): void
    {
        parent::setUp();

        $testingTenant = OurTenancyModel::where('tenant', 'testing')->first();
        tenancy()->initialize($testingTenant);

        DB::connection('mysql')->beginTransaction();
        DB::connection('tenant')->beginTransaction();
    }

    protected function tearDown(): void
    {
        DB::connection('tenant')->rollBack();
        DB::connection('tenant')->disconnect(); // <-- This was what needed to be added. 
        DB::connection('mysql')->rollBack();
        DB::connection('mysql')->disconnect(); // <-- This was what needed to be added. 

        parent::tearDown();
    }

Hope this helps!

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

5 participants