diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0426b1b8976d8..7843724239581 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1656,6 +1656,11 @@ 'OC\\DB\\ConnectionFactory' => $baseDir . '/lib/private/DB/ConnectionFactory.php', 'OC\\DB\\DbDataCollector' => $baseDir . '/lib/private/DB/DbDataCollector.php', 'OC\\DB\\Exceptions\\DbalException' => $baseDir . '/lib/private/DB/Exceptions/DbalException.php', + 'OC\\DB\\Middleware\\ConnectionActivityConnection' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityConnection.php', + 'OC\\DB\\Middleware\\ConnectionActivityDriver' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityDriver.php', + 'OC\\DB\\Middleware\\ConnectionActivityMiddleware' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityMiddleware.php', + 'OC\\DB\\Middleware\\ConnectionActivityNotifier' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityNotifier.php', + 'OC\\DB\\Middleware\\ConnectionActivityStatement' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityStatement.php', 'OC\\DB\\Middleware\\UtcTimezoneMiddleware' => $baseDir . '/lib/private/DB/Middleware/UtcTimezoneMiddleware.php', 'OC\\DB\\Middleware\\UtcTimezoneMiddlewareDriver' => $baseDir . '/lib/private/DB/Middleware/UtcTimezoneMiddlewareDriver.php', 'OC\\DB\\MigrationException' => $baseDir . '/lib/private/DB/MigrationException.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 7d647c336321a..b13049e46fe3c 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1697,6 +1697,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\DB\\ConnectionFactory' => __DIR__ . '/../../..' . '/lib/private/DB/ConnectionFactory.php', 'OC\\DB\\DbDataCollector' => __DIR__ . '/../../..' . '/lib/private/DB/DbDataCollector.php', 'OC\\DB\\Exceptions\\DbalException' => __DIR__ . '/../../..' . '/lib/private/DB/Exceptions/DbalException.php', + 'OC\\DB\\Middleware\\ConnectionActivityConnection' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityConnection.php', + 'OC\\DB\\Middleware\\ConnectionActivityDriver' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityDriver.php', + 'OC\\DB\\Middleware\\ConnectionActivityMiddleware' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityMiddleware.php', + 'OC\\DB\\Middleware\\ConnectionActivityNotifier' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityNotifier.php', + 'OC\\DB\\Middleware\\ConnectionActivityStatement' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityStatement.php', 'OC\\DB\\Middleware\\UtcTimezoneMiddleware' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/UtcTimezoneMiddleware.php', 'OC\\DB\\Middleware\\UtcTimezoneMiddlewareDriver' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/UtcTimezoneMiddlewareDriver.php', 'OC\\DB\\MigrationException' => __DIR__ . '/../../..' . '/lib/private/DB/MigrationException.php', diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index 27be1b327e2c6..6d084834e9ddd 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -24,6 +24,7 @@ use Doctrine\DBAL\Result; use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Statement; +use OC\DB\Middleware\ConnectionActivityNotifier; use OC\DB\QueryBuilder\Partitioned\PartitionedQueryBuilder; use OC\DB\QueryBuilder\Partitioned\PartitionSplit; use OC\DB\QueryBuilder\QueryBuilder; @@ -62,6 +63,8 @@ class Connection extends PrimaryReadReplicaConnection { protected int $queriesBuilt = 0; protected int $queriesExecuted = 0; protected ?DbDataCollector $dbDataCollector = null; + /** Seconds the connection may sit idle before the next use re-verifies connectivity */ + private const CONNECTION_CHECK_INTERVAL = 30; private array $lastConnectionCheck = []; protected ?float $transactionActiveSince = null; @@ -121,6 +124,10 @@ public function __construct( parent::__construct($params, $driver, $config, $eventManager); $this->adapter = new $params['adapter']($this); $this->tablePrefix = $params['tablePrefix']; + $activityNotifier = $params['activity_notifier'] ?? null; + if ($activityNotifier instanceof ConnectionActivityNotifier) { + $activityNotifier->setListener($this->refreshLastConnectionCheck(...)); + } $this->isShardingEnabled = isset($this->params['sharding']) && !empty($this->params['sharding']); if ($this->isShardingEnabled) { @@ -220,7 +227,7 @@ public function connect($connectionName = null) { $status = parent::connect(); $eventLogger->end('connect:db'); - $this->lastConnectionCheck[$this->getConnectionName()] = time(); + $this->refreshLastConnectionCheck(); return $status; } catch (Exception $e) { @@ -902,7 +909,7 @@ public function rollBack() { private function reconnectIfNeeded(): void { if ( !isset($this->lastConnectionCheck[$this->getConnectionName()]) - || time() <= $this->lastConnectionCheck[$this->getConnectionName()] + 30 + || time() <= $this->lastConnectionCheck[$this->getConnectionName()] + self::CONNECTION_CHECK_INTERVAL || $this->isTransactionActive() ) { return; @@ -910,13 +917,24 @@ private function reconnectIfNeeded(): void { try { $this->_conn->query($this->getDriver()->getDatabasePlatform()->getDummySelectSQL()); - $this->lastConnectionCheck[$this->getConnectionName()] = time(); + $this->refreshLastConnectionCheck(); } catch (ConnectionLost|\Exception $e) { $this->logger->warning('Exception during connectivity check, closing and reconnecting', ['exception' => $e]); $this->close(); } } + /** + * A successful round trip proves the connection is alive: pushing the idle + * timer forward keeps the connectivity probe of reconnectIfNeeded() from + * firing between adjacent operations, where its query would reset the + * driver level last insert id on MySQL. Invoked for every driver level + * execution via the ConnectionActivityMiddleware. + */ + private function refreshLastConnectionCheck(): void { + $this->lastConnectionCheck[$this->getConnectionName()] = time(); + } + private function getConnectionName(): string { return $this->isConnectedToPrimary() ? 'primary' : 'replica'; } diff --git a/lib/private/DB/ConnectionFactory.php b/lib/private/DB/ConnectionFactory.php index 3d5de0d7c8483..70d8294f7f5d0 100644 --- a/lib/private/DB/ConnectionFactory.php +++ b/lib/private/DB/ConnectionFactory.php @@ -11,6 +11,7 @@ use Doctrine\DBAL\Configuration; use Doctrine\DBAL\DriverManager; use Doctrine\DBAL\Event\Listeners\OracleSessionInit; +use OC\DB\Middleware\ConnectionActivityMiddleware; use OC\DB\Middleware\UtcTimezoneMiddleware; use OC\DB\QueryBuilder\Sharded\AutoIncrementHandler; use OC\DB\QueryBuilder\Sharded\ShardConnectionManager; @@ -145,9 +146,12 @@ public function getConnection(string $type, array $additionalConnectionParams): break; } $configuration = new Configuration(); + $activityMiddleware = new ConnectionActivityMiddleware(); $configuration->setMiddlewares([ new UtcTimezoneMiddleware(), + $activityMiddleware, ]); + $connectionParams['activity_notifier'] = $activityMiddleware->getNotifier(); /** @var Connection $connection */ $connection = DriverManager::getConnection( $connectionParams, diff --git a/lib/private/DB/Middleware/ConnectionActivityConnection.php b/lib/private/DB/Middleware/ConnectionActivityConnection.php new file mode 100644 index 0000000000000..bb4e554d7d252 --- /dev/null +++ b/lib/private/DB/Middleware/ConnectionActivityConnection.php @@ -0,0 +1,57 @@ +inner instanceof PDOConnection) { + throw new \LogicException('The wrapped connection is not a PDO based connection'); + } + return $this->inner->getWrappedConnection(); + } + + #[\Override] + public function prepare(string $sql): Statement { + return new ConnectionActivityStatement(parent::prepare($sql), $this->notifier); + } + + #[\Override] + public function query(string $sql): Result { + $result = parent::query($sql); + $this->notifier->notify(); + return $result; + } + + #[\Override] + public function exec(string $sql): int { + $result = parent::exec($sql); + $this->notifier->notify(); + return $result; + } +} diff --git a/lib/private/DB/Middleware/ConnectionActivityDriver.php b/lib/private/DB/Middleware/ConnectionActivityDriver.php new file mode 100644 index 0000000000000..84ab1d484dfeb --- /dev/null +++ b/lib/private/DB/Middleware/ConnectionActivityDriver.php @@ -0,0 +1,27 @@ +notifier); + } +} diff --git a/lib/private/DB/Middleware/ConnectionActivityMiddleware.php b/lib/private/DB/Middleware/ConnectionActivityMiddleware.php new file mode 100644 index 0000000000000..2a35a3d778164 --- /dev/null +++ b/lib/private/DB/Middleware/ConnectionActivityMiddleware.php @@ -0,0 +1,41 @@ +notifier = new ConnectionActivityNotifier(); + } + + /** + * Hand the notifier to the connection that wants to listen, e.g. via the + * connection parameters. + */ + public function getNotifier(): ConnectionActivityNotifier { + return $this->notifier; + } + + #[\Override] + public function wrap(Driver $driver): Driver { + return new ConnectionActivityDriver($driver, $this->notifier); + } +} diff --git a/lib/private/DB/Middleware/ConnectionActivityNotifier.php b/lib/private/DB/Middleware/ConnectionActivityNotifier.php new file mode 100644 index 0000000000000..c6dfc3ff2f1b2 --- /dev/null +++ b/lib/private/DB/Middleware/ConnectionActivityNotifier.php @@ -0,0 +1,32 @@ +listener = $listener; + } + + public function notify(): void { + if ($this->listener !== null) { + ($this->listener)(); + } + } +} diff --git a/lib/private/DB/Middleware/ConnectionActivityStatement.php b/lib/private/DB/Middleware/ConnectionActivityStatement.php new file mode 100644 index 0000000000000..62e9a4cf35e08 --- /dev/null +++ b/lib/private/DB/Middleware/ConnectionActivityStatement.php @@ -0,0 +1,30 @@ +notifier->notify(); + return $result; + } +} diff --git a/tests/lib/DB/ConnectionTest.php b/tests/lib/DB/ConnectionTest.php index 20348862b7da9..a3100bfbea0e6 100644 --- a/tests/lib/DB/ConnectionTest.php +++ b/tests/lib/DB/ConnectionTest.php @@ -15,6 +15,9 @@ use Doctrine\DBAL\Platforms\MySQLPlatform; use OC\DB\Adapter; use OC\DB\Connection; +use OC\DB\ConnectionAdapter; +use OCP\IDBConnection; +use OCP\Server; use Test\TestCase; #[\PHPUnit\Framework\Attributes\Group('DB')] @@ -96,4 +99,56 @@ public function testClusterConnectsToPrimaryAndReplica(): void { $connection->ensureConnectedToReplica(); } + public function testSuccessfulQueryResetsConnectivityCheckTimer(): void { + $inner = $this->getInnerConnection(); + + // Ensure the connection is established before touching the timer + $qb = $inner->getQueryBuilder(); + $qb->select('configvalue')->from('appconfig')->setMaxResults(1); + $qb->executeQuery()->closeCursor(); + + $property = $this->backdateLastConnectionCheck($inner); + $before = time(); + + $qb->executeQuery()->closeCursor(); + + // A connectivity probe firing between adjacent operations would reset + // the driver level last insert id on MySQL + self::assertGreaterThanOrEqual($before, max($property->getValue($inner))); + } + + public function testPreparedStatementExecutionResetsConnectivityCheckTimer(): void { + $inner = $this->getInnerConnection(); + + $statement = $inner->prepare('SELECT `configvalue` FROM `*PREFIX*appconfig`', 1); + + $property = $this->backdateLastConnectionCheck($inner); + $before = time(); + + $statement->executeQuery()->free(); + + self::assertGreaterThanOrEqual($before, max($property->getValue($inner))); + } + + private function getInnerConnection(): Connection { + $connection = Server::get(IDBConnection::class); + if (!$connection instanceof ConnectionAdapter) { + self::markTestSkipped('Test requires the real database connection'); + } + + return $connection->getInner(); + } + + /** + * Make the connectivity check timer stale, but by less than the check + * interval: the probe must not fire, so only actual query activity can + * refresh the timer. + */ + private function backdateLastConnectionCheck(Connection $connection): \ReflectionProperty { + $property = new \ReflectionProperty(Connection::class, 'lastConnectionCheck'); + $property->setValue($connection, ['primary' => time() - 20, 'replica' => time() - 20]); + + return $property; + } + }