fix(Database): Use real idle-timer to prevent lastInsertId being reset on MariaDB/MySQL

DerDreschner · backportbot[bot] · commit 20c2dfefe222 · 2026-07-30T16:33:14.000Z
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 <david.dreschner@nextcloud.com> [skip ci]
diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php
@@ -23,6 +23,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;
@@ -131,6 +132,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) {
@@ -229,7 +234,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) {
@@ -898,13 +903,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
@@ -0,0 +1,57 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\DB\Middleware;
+
+use Doctrine\DBAL\Driver\Connection;
+use Doctrine\DBAL\Driver\Middleware\AbstractConnectionMiddleware;
+use Doctrine\DBAL\Driver\PDO\Connection as PDOConnection;
+use Doctrine\DBAL\Driver\Result;
+use Doctrine\DBAL\Driver\Statement;
+
+final class ConnectionActivityConnection extends AbstractConnectionMiddleware {
+	public function __construct(
+		private Connection $inner,
+		private ConnectionActivityNotifier $notifier,
+	) {
+		parent::__construct($inner);
+	}
+
+	/**
+	 * Kept working for consumers that reach the native PDO handle through the
+	 * deprecated accessor, like SQLiteSessionInit: forwarding is intentionally
+	 * preferred over migrating the callers, as those code paths get refactored
+	 * with the DBAL 4 upgrade anyway.
+	 */
+	public function getWrappedConnection(): \PDO {
+		if (!$this->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
@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\DB\Middleware;
+
+use Doctrine\DBAL\Driver;
+use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware;
+
+final class ConnectionActivityDriver extends AbstractDriverMiddleware {
+	public function __construct(
+		Driver $driver,
+		private ConnectionActivityNotifier $notifier,
+	) {
+		parent::__construct($driver);
+	}
+
+	#[\Override]
+	public function connect(array $params) {
+		return new ConnectionActivityConnection(parent::connect($params), $this->notifier);
+	}
+}
diff --git a/lib/private/DB/Middleware/ConnectionActivityMiddleware.php b/lib/private/DB/Middleware/ConnectionActivityMiddleware.php
@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\DB\Middleware;
+
+use Doctrine\DBAL\Driver;
+use Doctrine\DBAL\Driver\Middleware;
+
+/**
+ * Doctrine middleware reporting every query and statement execution back to
+ * the owning connection, so the idle timer of the connectivity check can be
+ * refreshed (see \OC\DB\Connection::refreshLastConnectionCheck()). Working on
+ * the driver level covers executions of prepared statements as well, which
+ * bypass the executeQuery() and executeStatement() methods of the connection.
+ */
+final class ConnectionActivityMiddleware implements Middleware {
+	private ConnectionActivityNotifier $notifier;
+
+	public function __construct() {
+		$this->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
@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\DB\Middleware;
+
+/**
+ * Relays driver level activity to a listener that can only be registered
+ * after the middleware was created: middlewares are configured before the
+ * DriverManager constructs the connection wrapper that wants to listen.
+ */
+final class ConnectionActivityNotifier {
+	private ?\Closure $listener = null;
+
+	/**
+	 * @param \Closure():void $listener
+	 */
+	public function setListener(\Closure $listener): void {
+		$this->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
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\DB\Middleware;
+
+use Doctrine\DBAL\Driver\Middleware\AbstractStatementMiddleware;
+use Doctrine\DBAL\Driver\Result;
+use Doctrine\DBAL\Driver\Statement;
+
+final class ConnectionActivityStatement extends AbstractStatementMiddleware {
+	public function __construct(
+		Statement $statement,
+		private ConnectionActivityNotifier $notifier,
+	) {
+		parent::__construct($statement);
+	}
+
+	#[\Override]
+	public function execute($params = null): Result {
+		$result = parent::execute($params);
+		$this->notifier->notify();
+		return $result;
+	}
+}
diff --git 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;
+	}
+
 }
