Skip to content

Commit 4178e56

Browse files
committed
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>
1 parent 28d1178 commit 4178e56

10 files changed

Lines changed: 286 additions & 4 deletions

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,6 +1299,11 @@
12991299
'OC\\DB\\ConnectionFactory' => $baseDir . '/lib/private/DB/ConnectionFactory.php',
13001300
'OC\\DB\\DbDataCollector' => $baseDir . '/lib/private/DB/DbDataCollector.php',
13011301
'OC\\DB\\Exceptions\\DbalException' => $baseDir . '/lib/private/DB/Exceptions/DbalException.php',
1302+
'OC\\DB\\Middleware\\ConnectionActivityConnection' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityConnection.php',
1303+
'OC\\DB\\Middleware\\ConnectionActivityDriver' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityDriver.php',
1304+
'OC\\DB\\Middleware\\ConnectionActivityMiddleware' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityMiddleware.php',
1305+
'OC\\DB\\Middleware\\ConnectionActivityNotifier' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityNotifier.php',
1306+
'OC\\DB\\Middleware\\ConnectionActivityStatement' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityStatement.php',
13021307
'OC\\DB\\MigrationException' => $baseDir . '/lib/private/DB/MigrationException.php',
13031308
'OC\\DB\\MigrationService' => $baseDir . '/lib/private/DB/MigrationService.php',
13041309
'OC\\DB\\Migrator' => $baseDir . '/lib/private/DB/Migrator.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,6 +1332,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
13321332
'OC\\DB\\ConnectionFactory' => __DIR__ . '/../../..' . '/lib/private/DB/ConnectionFactory.php',
13331333
'OC\\DB\\DbDataCollector' => __DIR__ . '/../../..' . '/lib/private/DB/DbDataCollector.php',
13341334
'OC\\DB\\Exceptions\\DbalException' => __DIR__ . '/../../..' . '/lib/private/DB/Exceptions/DbalException.php',
1335+
'OC\\DB\\Middleware\\ConnectionActivityConnection' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityConnection.php',
1336+
'OC\\DB\\Middleware\\ConnectionActivityDriver' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityDriver.php',
1337+
'OC\\DB\\Middleware\\ConnectionActivityMiddleware' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityMiddleware.php',
1338+
'OC\\DB\\Middleware\\ConnectionActivityNotifier' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityNotifier.php',
1339+
'OC\\DB\\Middleware\\ConnectionActivityStatement' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityStatement.php',
13351340
'OC\\DB\\MigrationException' => __DIR__ . '/../../..' . '/lib/private/DB/MigrationException.php',
13361341
'OC\\DB\\MigrationService' => __DIR__ . '/../../..' . '/lib/private/DB/MigrationService.php',
13371342
'OC\\DB\\Migrator' => __DIR__ . '/../../..' . '/lib/private/DB/Migrator.php',

‎lib/private/DB/Connection.php‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
use Doctrine\DBAL\Result;
4949
use Doctrine\DBAL\Schema\Schema;
5050
use Doctrine\DBAL\Statement;
51+
use OC\DB\Middleware\ConnectionActivityNotifier;
5152
use OC\DB\QueryBuilder\QueryBuilder;
5253
use OC\SystemConfig;
5354
use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -86,6 +87,8 @@ class Connection extends PrimaryReadReplicaConnection {
8687

8788
/** @var DbDataCollector|null */
8889
protected $dbDataCollector = null;
90+
/** Seconds the connection may sit idle before the next use re-verifies connectivity */
91+
private const CONNECTION_CHECK_INTERVAL = 30;
8992
private array $lastConnectionCheck = [];
9093

9194
protected ?float $transactionActiveSince = null;
@@ -122,6 +125,13 @@ public function __construct(
122125
parent::__construct($params, $driver, $config, $eventManager);
123126
$this->adapter = new $params['adapter']($this);
124127
$this->tablePrefix = $params['tablePrefix'];
128+
$activityNotifier = $params['activity_notifier'] ?? null;
129+
if ($activityNotifier instanceof ConnectionActivityNotifier) {
130+
// first-class callable syntax requires PHP 8.1, this branch still supports 8.0
131+
$activityNotifier->setListener(function (): void {
132+
$this->refreshLastConnectionCheck();
133+
});
134+
}
125135

126136
$this->systemConfig = \OC::$server->getSystemConfig();
127137
$this->clock = Server::get(ClockInterface::class);
@@ -160,7 +170,7 @@ public function connect($connectionName = null) {
160170
$status = parent::connect();
161171
$eventLogger->end('connect:db');
162172

163-
$this->lastConnectionCheck[$this->getConnectionName()] = time();
173+
$this->refreshLastConnectionCheck();
164174

165175
return $status;
166176
} catch (Exception $e) {
@@ -786,21 +796,32 @@ public function rollBack() {
786796
private function reconnectIfNeeded(): void {
787797
if (
788798
!isset($this->lastConnectionCheck[$this->getConnectionName()]) ||
789-
time() <= $this->lastConnectionCheck[$this->getConnectionName()] + 30 ||
799+
time() <= $this->lastConnectionCheck[$this->getConnectionName()] + self::CONNECTION_CHECK_INTERVAL ||
790800
$this->isTransactionActive()
791801
) {
792802
return;
793803
}
794804

795805
try {
796806
$this->_conn->query($this->getDriver()->getDatabasePlatform()->getDummySelectSQL());
797-
$this->lastConnectionCheck[$this->getConnectionName()] = time();
807+
$this->refreshLastConnectionCheck();
798808
} catch (ConnectionLost|\Exception $e) {
799809
$this->logger->warning('Exception during connectivity check, closing and reconnecting', ['exception' => $e]);
800810
$this->close();
801811
}
802812
}
803813

814+
/**
815+
* A successful round trip proves the connection is alive: pushing the idle
816+
* timer forward keeps the connectivity probe of reconnectIfNeeded() from
817+
* firing between adjacent operations, where its query would reset the
818+
* driver level last insert id on MySQL. Invoked for every driver level
819+
* execution via the ConnectionActivityMiddleware.
820+
*/
821+
private function refreshLastConnectionCheck(): void {
822+
$this->lastConnectionCheck[$this->getConnectionName()] = time();
823+
}
824+
804825
private function getConnectionName(): string {
805826
return $this->isConnectedToPrimary() ? 'primary' : 'replica';
806827
}

‎lib/private/DB/ConnectionFactory.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use Doctrine\DBAL\Configuration;
3333
use Doctrine\DBAL\DriverManager;
3434
use Doctrine\DBAL\Event\Listeners\OracleSessionInit;
35+
use OC\DB\Middleware\ConnectionActivityMiddleware;
3536
use OC\SystemConfig;
3637

3738
/**
@@ -144,10 +145,16 @@ public function getConnection(string $type, array $additionalConnectionParams):
144145
$eventManager->addEventSubscriber(new SQLiteSessionInit(true, $journalMode));
145146
break;
146147
}
148+
$configuration = new Configuration();
149+
$activityMiddleware = new ConnectionActivityMiddleware();
150+
$configuration->setMiddlewares([
151+
$activityMiddleware,
152+
]);
153+
$connectionParams['activity_notifier'] = $activityMiddleware->getNotifier();
147154
/** @var Connection $connection */
148155
$connection = DriverManager::getConnection(
149156
$connectionParams,
150-
new Configuration(),
157+
$configuration,
151158
$eventManager
152159
);
153160
return $connection;
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OC\DB\Middleware;
11+
12+
use Doctrine\DBAL\Driver\Connection;
13+
use Doctrine\DBAL\Driver\Middleware\AbstractConnectionMiddleware;
14+
use Doctrine\DBAL\Driver\PDO\Connection as PDOConnection;
15+
use Doctrine\DBAL\Driver\Result;
16+
use Doctrine\DBAL\Driver\Statement;
17+
18+
final class ConnectionActivityConnection extends AbstractConnectionMiddleware {
19+
public function __construct(
20+
private Connection $inner,
21+
private ConnectionActivityNotifier $notifier,
22+
) {
23+
parent::__construct($inner);
24+
}
25+
26+
/**
27+
* Kept working for consumers that reach the native PDO handle through the
28+
* deprecated accessor, like SQLiteSessionInit: forwarding is intentionally
29+
* preferred over migrating the callers, as those code paths get refactored
30+
* with the DBAL 4 upgrade anyway.
31+
*/
32+
public function getWrappedConnection(): \PDO {
33+
if (!$this->inner instanceof PDOConnection) {
34+
throw new \LogicException('The wrapped connection is not a PDO based connection');
35+
}
36+
return $this->inner->getWrappedConnection();
37+
}
38+
39+
#[\Override]
40+
public function prepare(string $sql): Statement {
41+
return new ConnectionActivityStatement(parent::prepare($sql), $this->notifier);
42+
}
43+
44+
#[\Override]
45+
public function query(string $sql): Result {
46+
$result = parent::query($sql);
47+
$this->notifier->notify();
48+
return $result;
49+
}
50+
51+
#[\Override]
52+
public function exec(string $sql): int {
53+
$result = parent::exec($sql);
54+
$this->notifier->notify();
55+
return $result;
56+
}
57+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OC\DB\Middleware;
11+
12+
use Doctrine\DBAL\Driver;
13+
use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware;
14+
15+
final class ConnectionActivityDriver extends AbstractDriverMiddleware {
16+
public function __construct(
17+
Driver $driver,
18+
private ConnectionActivityNotifier $notifier,
19+
) {
20+
parent::__construct($driver);
21+
}
22+
23+
#[\Override]
24+
public function connect(array $params) {
25+
return new ConnectionActivityConnection(parent::connect($params), $this->notifier);
26+
}
27+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OC\DB\Middleware;
11+
12+
use Doctrine\DBAL\Driver;
13+
use Doctrine\DBAL\Driver\Middleware;
14+
15+
/**
16+
* Doctrine middleware reporting every query and statement execution back to
17+
* the owning connection, so the idle timer of the connectivity check can be
18+
* refreshed (see \OC\DB\Connection::refreshLastConnectionCheck()). Working on
19+
* the driver level covers executions of prepared statements as well, which
20+
* bypass the executeQuery() and executeStatement() methods of the connection.
21+
*/
22+
final class ConnectionActivityMiddleware implements Middleware {
23+
private ConnectionActivityNotifier $notifier;
24+
25+
public function __construct() {
26+
$this->notifier = new ConnectionActivityNotifier();
27+
}
28+
29+
/**
30+
* Hand the notifier to the connection that wants to listen, e.g. via the
31+
* connection parameters.
32+
*/
33+
public function getNotifier(): ConnectionActivityNotifier {
34+
return $this->notifier;
35+
}
36+
37+
#[\Override]
38+
public function wrap(Driver $driver): Driver {
39+
return new ConnectionActivityDriver($driver, $this->notifier);
40+
}
41+
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OC\DB\Middleware;
11+
12+
/**
13+
* Relays driver level activity to a listener that can only be registered
14+
* after the middleware was created: middlewares are configured before the
15+
* DriverManager constructs the connection wrapper that wants to listen.
16+
*/
17+
final class ConnectionActivityNotifier {
18+
private ?\Closure $listener = null;
19+
20+
/**
21+
* @param \Closure():void $listener
22+
*/
23+
public function setListener(\Closure $listener): void {
24+
$this->listener = $listener;
25+
}
26+
27+
public function notify(): void {
28+
if ($this->listener !== null) {
29+
($this->listener)();
30+
}
31+
}
32+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OC\DB\Middleware;
11+
12+
use Doctrine\DBAL\Driver\Middleware\AbstractStatementMiddleware;
13+
use Doctrine\DBAL\Driver\Result;
14+
use Doctrine\DBAL\Driver\Statement;
15+
16+
final class ConnectionActivityStatement extends AbstractStatementMiddleware {
17+
public function __construct(
18+
Statement $statement,
19+
private ConnectionActivityNotifier $notifier,
20+
) {
21+
parent::__construct($statement);
22+
}
23+
24+
#[\Override]
25+
public function execute($params = null): Result {
26+
$result = parent::execute($params);
27+
$this->notifier->notify();
28+
return $result;
29+
}
30+
}

‎tests/lib/DB/ConnectionTest.php‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
use Doctrine\DBAL\Platforms\MySQLPlatform;
1616
use OC\DB\Adapter;
1717
use OC\DB\Connection;
18+
use OC\DB\ConnectionAdapter;
19+
use OCP\IDBConnection;
20+
use OCP\Server;
1821
use Test\TestCase;
1922

2023
/**
@@ -98,4 +101,58 @@ public function testClusterConnectsToPrimaryAndReplica(): void {
98101
$connection->ensureConnectedToReplica();
99102
}
100103

104+
public function testSuccessfulQueryResetsConnectivityCheckTimer(): void {
105+
$inner = $this->getInnerConnection();
106+
107+
// Ensure the connection is established before touching the timer
108+
$qb = $inner->getQueryBuilder();
109+
$qb->select('configvalue')->from('appconfig')->setMaxResults(1);
110+
$qb->executeQuery()->closeCursor();
111+
112+
$property = $this->backdateLastConnectionCheck($inner);
113+
$before = time();
114+
115+
$qb->executeQuery()->closeCursor();
116+
117+
// A connectivity probe firing between adjacent operations would reset
118+
// the driver level last insert id on MySQL
119+
self::assertGreaterThanOrEqual($before, max($property->getValue($inner)));
120+
}
121+
122+
public function testPreparedStatementExecutionResetsConnectivityCheckTimer(): void {
123+
$inner = $this->getInnerConnection();
124+
125+
$statement = $inner->prepare('SELECT `configvalue` FROM `*PREFIX*appconfig`', 1);
126+
127+
$property = $this->backdateLastConnectionCheck($inner);
128+
$before = time();
129+
130+
$statement->executeQuery()->free();
131+
132+
self::assertGreaterThanOrEqual($before, max($property->getValue($inner)));
133+
}
134+
135+
private function getInnerConnection(): Connection {
136+
$connection = Server::get(IDBConnection::class);
137+
if (!$connection instanceof ConnectionAdapter) {
138+
self::markTestSkipped('Test requires the real database connection');
139+
}
140+
141+
return $connection->getInner();
142+
}
143+
144+
/**
145+
* Make the connectivity check timer stale, but by less than the check
146+
* interval: the probe must not fire, so only actual query activity can
147+
* refresh the timer.
148+
*/
149+
private function backdateLastConnectionCheck(Connection $connection): \ReflectionProperty {
150+
$property = new \ReflectionProperty(Connection::class, 'lastConnectionCheck');
151+
// required on PHP 8.0, a no-op since 8.1
152+
$property->setAccessible(true);
153+
$property->setValue($connection, ['primary' => time() - 20, 'replica' => time() - 20]);
154+
155+
return $property;
156+
}
157+
101158
}

0 commit comments

Comments
 (0)