diff --git a/src/Swoole/Database/DatabaseManager.php b/src/Swoole/Database/DatabaseManager.php index 695eb47..e05affd 100644 --- a/src/Swoole/Database/DatabaseManager.php +++ b/src/Swoole/Database/DatabaseManager.php @@ -109,6 +109,7 @@ protected function getPool($name) 'heartbeat' => -1, 'max_idle_time' => 60.0, 'max_lifetime' => DatabasePool::DEFAULT_MAX_LIFETIME, + 'ping_after_idle' => 30.0, ]; $this->pools[$name] = new DatabasePool( diff --git a/src/Swoole/Database/DatabasePool.php b/src/Swoole/Database/DatabasePool.php index 5b726ca..1a1f918 100644 --- a/src/Swoole/Database/DatabasePool.php +++ b/src/Swoole/Database/DatabasePool.php @@ -115,18 +115,34 @@ public function get() } } + $idleFor = $this->idleSeconds($connection); + $this->markBorrowed($connection); - // Check if connection is still valid - if (! $this->checkConnection($connection)) { - $connection = $this->reconnect($connection); + // Ping only connections that sat idle long enough for the server to + // have plausibly dropped them. Every pooled connection was released + // moments-to-seconds ago on a busy pool, and the ping is a full + // round-trip to the database (measured ~10-20ms from Cloud Run to the + // DB host) paid on every request. Connections created on demand have + // no idle timestamp yet and skip the ping. A connection that died while + // idle still gets caught: the first real query fails and the + // reconnector swaps in a fresh PDO. + if ($idleFor !== null && $idleFor >= $this->pingAfterIdleSeconds()) { + if (! $this->checkConnection($connection)) { + $connection = $this->reconnect($connection); + } } - // Defensive cleanup on checkout as well as release. If a previous - // request left PDO or Laravel transaction state dirty, never hand that - // connection to the next coroutine. + // release() fully resets every connection before re-pooling it and + // closes any connection whose reset fails, so pooled connections are + // clean by invariant and the checkout-side session SQL (two more + // round-trips per borrow) is redundant. The local transaction checks + // below cost no SQL; only a genuinely dirty connection - e.g. handed + // to us dirty by the factory - pays for a full reset. try { - $this->resetConnection($connection); + if ($this->hasDirtyTransactionState($connection)) { + $this->resetConnection($connection); + } } catch (Throwable $e) { error_log('❌ Dirty DB connection could not be reset on checkout: '.$e->getMessage()); $this->closeConnection($connection); @@ -450,6 +466,30 @@ protected function rollBackAbandonedTransactions($connection): void } } + /** + * Force a fresh connection into the same session state resetConnection() + * leaves recycled ones in. Checkout no longer re-runs the session SQL, + * so fresh and recycled connections must be indistinguishable: without + * this, a server whose global isolation level or autocommit differs from + * these values would hand out connections whose behavior depends on + * whether they happen to be fresh - nondeterministic snapshot and + * gap-lock semantics per request. + */ + protected function normalizeSession(Connection $connection): void + { + if (! in_array($connection->getDriverName(), ['mysql', 'mariadb'], true)) { + return; + } + + try { + $pdo = $connection->getPdo(); + $pdo->exec('SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ'); + $pdo->exec('SET autocommit = 1'); + } catch (Throwable $e) { + error_log('⚠️ Could not normalize fresh MySQL session: '.$e->getMessage()); + } + } + /** * Whether a connection is older than the pool's max_lifetime. * @@ -487,6 +527,8 @@ protected function createConnection() } if ($connection instanceof Connection) { + $this->normalizeSession($connection); + // Without a reconnector, Connection::reconnect() throws // LostConnectionException and the pool silently discards and // rebuilds the connection on every stale checkout. Swapping the @@ -511,6 +553,50 @@ protected function createConnection() } } + /** + * Seconds this connection has sat idle in the pool, or null when unknown + * (freshly created connections have no idle timestamp). + */ + protected function idleSeconds($connection): ?float + { + if (! is_object($connection)) { + return null; + } + + $since = $this->idleSince[spl_object_id($connection)] ?? null; + + return $since === null ? null : microtime(true) - $since; + } + + /** + * Idle age beyond which a checkout pings the server before handing the + * connection out. 0 or negative pings on every checkout of a pooled + * connection (freshly created ones never ping - they just connected). + */ + protected function pingAfterIdleSeconds(): float + { + return (float) ($this->config['ping_after_idle'] ?? 30.0); + } + + /** + * Local-only dirty check: no SQL, just the Laravel counter and the PDO + * driver flag. + */ + protected function hasDirtyTransactionState($connection): bool + { + if (! $connection instanceof Connection) { + return false; + } + + if ($connection->transactionLevel() > 0) { + return true; + } + + $pdo = $connection->getRawPdo(); + + return $pdo instanceof \PDO && $pdo->inTransaction(); + } + protected function markIdle($connection): void { if (is_object($connection)) { diff --git a/tests/Unit/DatabasePoolTest.php b/tests/Unit/DatabasePoolTest.php index df6f120..ba2cccf 100644 --- a/tests/Unit/DatabasePoolTest.php +++ b/tests/Unit/DatabasePoolTest.php @@ -277,6 +277,7 @@ public function test_get_reconnects_invalid_connection() 'min_connections' => 1, 'max_connections' => 1, 'wait_timeout' => 0.1, + 'ping_after_idle' => 0, ], [], 'mysql', $factory); $result = $pool->get(); @@ -376,6 +377,7 @@ public function test_prune_idle_connections_closes_available_connections_above_m // Illuminate\Database\DatabaseManager::configure() does. foreach ($connections as $connection) { $connection->shouldReceive('setReconnector')->once(); + $connection->shouldReceive('getDriverName')->andReturn('sqlite'); } $factory = Mockery::mock(ConnectionFactory::class); @@ -711,6 +713,163 @@ public function test_tracking_a_new_connection_heals_a_reused_object_id(): void $this->assertSame($fresh, $live->getValue($pool)[$reusedId]->get()); } + public function test_checkout_skips_ping_and_session_reset_for_recently_pooled_connection(): void + { + $this->skipIfNoSwooleCoroutine(); + + if (! extension_loaded('pdo_sqlite')) { + $this->markTestSkipped('PDO SQLite is required.'); + } + + $factory = Mockery::mock(ConnectionFactory::class); + $factory->shouldReceive('make') + ->once() + ->withAnyArgs() + ->andReturnUsing(fn () => new Connection(new CountingSqlitePdo('sqlite::memory:'), 'database', '', [])); + + \Swoole\Coroutine\run(function () use ($factory) { + $pool = new DatabasePool([ + 'min_connections' => 0, + 'max_connections' => 1, + 'wait_timeout' => 0.1, + 'max_lifetime' => 60.0, + 'ping_after_idle' => 30.0, + ], [], 'sqlite', $factory); + + $connection = $pool->get(); + $pool->release($connection); + + // Probe: resetConnection flushes the query log; a surviving entry + // proves the checkout skipped the redundant session reset. + $connection->enableQueryLog(); + $connection->logQuery('probe', [], 0); + $pings = $connection->getPdo()->queryCalls; + + $again = $pool->get(); + + $this->assertSame($connection, $again); + $this->assertSame($pings, $connection->getPdo()->queryCalls, 'A connection idle for milliseconds must not be pinged on checkout.'); + $this->assertCount(1, $connection->getQueryLog(), 'A clean pooled connection must not pay the session reset on checkout.'); + }); + } + + public function test_checkout_pings_connection_idle_beyond_threshold(): void + { + $this->skipIfNoSwooleCoroutine(); + + if (! extension_loaded('pdo_sqlite')) { + $this->markTestSkipped('PDO SQLite is required.'); + } + + $factory = Mockery::mock(ConnectionFactory::class); + $factory->shouldReceive('make') + ->once() + ->withAnyArgs() + ->andReturnUsing(fn () => new Connection(new CountingSqlitePdo('sqlite::memory:'), 'database', '', [])); + + \Swoole\Coroutine\run(function () use ($factory) { + $pool = new DatabasePool([ + 'min_connections' => 0, + 'max_connections' => 1, + 'wait_timeout' => 0.1, + 'max_lifetime' => 600.0, + 'max_idle_time' => 600.0, + 'ping_after_idle' => 30.0, + ], [], 'sqlite', $factory); + + $connection = $pool->get(); + $pool->release($connection); + + // Age the idle timestamp past the ping threshold. + $idle = new \ReflectionProperty(DatabasePool::class, 'idleSince'); + $entries = $idle->getValue($pool); + foreach ($entries as $id => $since) { + $entries[$id] = $since - 120.0; + } + $idle->setValue($pool, $entries); + + $pings = $connection->getPdo()->queryCalls; + $pool->get(); + + $this->assertGreaterThan($pings, $connection->getPdo()->queryCalls, 'A connection idle past ping_after_idle must be pinged before handout.'); + }); + } + + public function test_checkout_still_resets_a_dirty_pooled_connection(): void + { + $this->skipIfNoSwooleCoroutine(); + + if (! extension_loaded('pdo_sqlite')) { + $this->markTestSkipped('PDO SQLite is required.'); + } + + $pdo = new PDO('sqlite::memory:'); + $connection = new Connection($pdo, 'database', '', []); + + $factory = Mockery::mock(ConnectionFactory::class); + $factory->shouldReceive('make')->never(); + + \Swoole\Coroutine\run(function () use ($factory, $connection, $pdo) { + $pool = new DatabasePool([ + 'min_connections' => 0, + 'max_connections' => 1, + 'wait_timeout' => 0.1, + ], [], 'sqlite', $factory); + + // Plant a DIRTY connection straight into the channel, bypassing + // release() - simulating any path that re-pools without reset. + $connection->beginTransaction(); + $channel = new \ReflectionProperty(DatabasePool::class, 'channel'); + $channel->getValue($pool)->push($connection); + $this->setPoolCurrentConnections($pool, 1); + $live = new \ReflectionProperty(DatabasePool::class, 'liveConnections'); + $live->setValue($pool, [spl_object_id($connection) => \WeakReference::create($connection)]); + + $handed = $pool->get(); + + $this->assertSame($connection, $handed); + $this->assertSame(0, $handed->transactionLevel(), 'A dirty pooled connection must still be reset at checkout.'); + $this->assertFalse($pdo->inTransaction()); + }); + } + + public function test_fresh_mysql_connections_get_the_same_session_normalization_as_recycled_ones(): void + { + $this->skipIfNoSwooleCoroutine(); + + $pdo = Mockery::mock(PDO::class); + $pdo->shouldReceive('exec')->with('SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ')->once(); + $pdo->shouldReceive('exec')->with('SET autocommit = 1')->once(); + $pdo->shouldReceive('query')->andReturn(true); + $pdo->shouldReceive('inTransaction')->andReturn(false); + + $connection = Mockery::mock(Connection::class); + $connection->shouldReceive('getDriverName')->andReturn('mysql'); + $connection->shouldReceive('getPdo')->andReturn($pdo); + $connection->shouldReceive('getRawPdo')->andReturn($pdo); + $connection->shouldReceive('setReconnector')->once(); + $connection->shouldReceive('transactionLevel')->andReturn(0); + + $factory = Mockery::mock(ConnectionFactory::class); + $factory->shouldReceive('make')->once()->withAnyArgs()->andReturn($connection); + + $result = null; + + \Swoole\Coroutine\run(function () use ($factory, &$result) { + $pool = new DatabasePool([ + 'min_connections' => 0, + 'max_connections' => 1, + 'wait_timeout' => 0.1, + ], [], 'mysql', $factory); + + $result = $pool->get(); + }); + + $this->assertSame($connection, $result); + // Mockery's ->once() expectations on the two SETs are the assertions: + // checkout no longer homogenizes sessions, so creation must. + } + protected function newPoolWithoutConstructor(): DatabasePool { $reflection = new \ReflectionClass(DatabasePool::class); @@ -746,6 +905,23 @@ protected function skipIfNoSwooleCoroutine(): void } } +class CountingSqlitePdo extends PDO +{ + public int $queryCalls = 0; + + #[\ReturnTypeWillChange] + public function query(string $query, ?int $fetchMode = null, mixed ...$fetchModeArgs) + { + $this->queryCalls++; + + if ($fetchMode === null) { + return parent::query($query); + } + + return parent::query($query, $fetchMode, ...$fetchModeArgs); + } +} + class TestPdoSuccess { public function query(string $sql): bool