From 116c49eb95644f2f4a623faa033fc8a29ff77b8a Mon Sep 17 00:00:00 2001 From: David Dreschner Date: Thu, 30 Jul 2026 15:11:12 +0200 Subject: [PATCH] fix(Database): Use real idle-timer to prevent `lastInsertId` being reset on MariaDB/MySQL fix(Database): Use real idle-timer to prevent `lastInsertId` being reset on MariaDB/MySQL The previous implementation of the idle timer runs on a strict 30 second interval and sends a dummy `SELECT` statement to keep the connection open. This generates issues with the `lastInsertId` on long-running tasks (like our CI pipeline), as the MariaDB documentation clearly states: > If the last query wasn't an INSERT or UPDATE statement or if the modified table does not have a column with the AUTO_INCREMENT attribute and LAST_INSERT_ID was not used, this function will return zero. Source: https://mariadb.com/docs/connectors/mariadb-connector-c/api-functions/mysql_insert_id To mitigate that, this commit now uses a real idle-timer per connection instead. Assisted-by: ClaudeCode:claude-fable-5 Signed-off-by: David Dreschner [skip ci] --- lib/private/DB/Connection.php | 15 ++++- .../ConnectionActivityConnection.php | 57 +++++++++++++++++++ .../Middleware/ConnectionActivityDriver.php | 27 +++++++++ .../ConnectionActivityMiddleware.php | 41 +++++++++++++ .../Middleware/ConnectionActivityNotifier.php | 32 +++++++++++ .../ConnectionActivityStatement.php | 30 ++++++++++ tests/lib/DB/ConnectionTest.php | 55 ++++++++++++++++++ 7 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 lib/private/DB/Middleware/ConnectionActivityConnection.php create mode 100644 lib/private/DB/Middleware/ConnectionActivityDriver.php create mode 100644 lib/private/DB/Middleware/ConnectionActivityMiddleware.php create mode 100644 lib/private/DB/Middleware/ConnectionActivityNotifier.php create mode 100644 lib/private/DB/Middleware/ConnectionActivityStatement.php diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index 3edfffa6907a1..8623405195b49 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -160,7 +160,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) { @@ -794,13 +794,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/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 3f06082ff0cc3..794acfa2ac8e5 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; /** @@ -98,4 +101,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; + } + }