laravel-mysql-spatial icon indicating copy to clipboard operation
laravel-mysql-spatial copied to clipboard

Connections not closing on Laravel Vapor

Open georgeboot opened this issue 4 years ago • 8 comments

When this package is installed together with doctrine/dbal, and deployed to Laravel Vapor, the queue worker will not close the database connection after completing a job.

Please see https://github.com/georgeboot/test-db-thing for a proof of concept.

I have already contacted the Laravel Vapor team but they say the issue must be within this package.

The issue only happens on Vapor and only for queue jobs. When running locally or in web request, nothing goes wrong.

georgeboot avatar Sep 04 '20 07:09 georgeboot

@georgeboot Did you find any solution for this?

johanlinander avatar Oct 20 '20 07:10 johanlinander

Nope, unfortunately not. We ended up removing doctrine/dbal and that works fine.

Contacted the the Vapor team again but same story...

georgeboot avatar Oct 20 '20 07:10 georgeboot

Here's a screenshot of before/after we removed this package from our Laravel Vapor environment:

spatial-db-connections

mattias-persson avatar Oct 23 '20 07:10 mattias-persson

any news here?

The spike happened when installing the package

image

We managed to extract elloquent / sql parsing features from the package, and that didnt increase connections

Casperhr avatar Dec 22 '20 05:12 Casperhr

Same issue here. When running the tests I receive the following error:

SQLSTATE[08004] [1040] Too many connections

The issue goes away when removing the package but since I need it that was not an option.

What I noticed is that the issue is located within \Grimzy\LaravelMysqlSpatial\Connectors\ConnectionFactory and it can be skipped by bypassing the $driver === 'mysql' condition like this:

/**
 * @param string       $driver
 * @param \Closure|PDO $connection
 * @param string       $database
 * @param string       $prefix
 * @param array        $config
 *
 * @return \Illuminate\Database\ConnectionInterface
 */
protected function createConnection($driver, $connection, $database, $prefix = '', array $config = [])
{
    if ($this->container->bound($key = "db.connection.{$driver}")) {
        return $this->container->make($key, [$connection, $database, $prefix, $config]);    // @codeCoverageIgnore
    }

    // Skip the condition and return immediately with `parent::createConnection()`.
    return parent::createConnection($driver, $connection, $database, $prefix, $config);

    if ($driver === 'mysql') {
        return new MysqlConnection($connection, $database, $prefix, $config);
    }

    return parent::createConnection($driver, $connection, $database, $prefix, $config);
}

It appears that everything is working fine at the moment and the tests are running successfully so I believe that the issue is within \Grimzy\LaravelMysqlSpatial\MysqlConnection.

So, for a quick (and hopefully temporary solution until this is addressed in laravel-mysql-spatial) you can:

  1. Create your own service provider that extends \Grimzy\LaravelMysqlSpatial\SpatialServiceProvider:
class BloomSpatialServiceProvider extends SpatialServiceProvider
{
    /**
     * Register the service provider.
     */
    public function register(): void
    {
        parent::register();

        $this->app->singleton('db.factory', function ($app) {
            // Use our `ConnectionFactory` in order to fix the DB connections issue.
            return new ConnectionFactory($app);
        });
    }
}
  1. Create your own ConnectionFactory that skips the check so that MysqlConnection is not executed:
use Closure;
use Grimzy\LaravelMysqlSpatial\Connectors\ConnectionFactory as GrimzyConnectionFactory;
use Illuminate\Database\ConnectionInterface;
use Illuminate\Database\Connectors\ConnectionFactory as IlluminateConnectionFactory;
use PDO;

class ConnectionFactory extends GrimzyConnectionFactory
{
    /**
     * @param string      $driver
     * @param Closure|PDO $connection
     * @param string      $database
     * @param string      $prefix
     * @param array       $config
     */
    protected function createConnection(
        $driver,
        $connection,
        $database,
        $prefix = '',
        array $config = []
    ): ConnectionInterface {
        if ($this->container->bound($key = "db.connection.{$driver}")) {
            return $this->container->make($key, [$connection, $database, $prefix, $config]);
        }

        // Call the parent's parent `createConnection`.
        return IlluminateConnectionFactory::createConnection(
            $driver,
            $connection,
            $database,
            $prefix,
            $config
        );
    }
}
  1. Use your own service provider in app.php instead of the one provided by laravel-mysql-spatial.

silvioiannone avatar Apr 07 '21 12:04 silvioiannone

Any chance for a package level fix here? 🤯

obrunsmann avatar Sep 16 '21 08:09 obrunsmann

We switched to using https://github.com/MatanYadaev/laravel-eloquent-spatial. That package offers similar functionality and doesn't plug in as deep into the connections as this package does.

georgeboot avatar Sep 16 '21 08:09 georgeboot

Wow thanks @georgeboot! Will try directly.

Update: Easy migration, very stable! Works as expected.

obrunsmann avatar Sep 16 '21 09:09 obrunsmann