Skip to content

Commit 44692a2

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 c65d1f9 commit 44692a2

10 files changed

Lines changed: 263 additions & 3 deletions

lib/composer/composer/autoload_classmap.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,6 +1714,11 @@
17141714
'OC\\DB\\ConnectionFactory' => $baseDir . '/lib/private/DB/ConnectionFactory.php',
17151715
'OC\\DB\\DbDataCollector' => $baseDir . '/lib/private/DB/DbDataCollector.php',
17161716
'OC\\DB\\Exceptions\\DbalException' => $baseDir . '/lib/private/DB/Exceptions/DbalException.php',
1717+
'OC\\DB\\Middleware\\ConnectionActivityConnection' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityConnection.php',
1718+
'OC\\DB\\Middleware\\ConnectionActivityDriver' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityDriver.php',
1719+
'OC\\DB\\Middleware\\ConnectionActivityMiddleware' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityMiddleware.php',
1720+
'OC\\DB\\Middleware\\ConnectionActivityNotifier' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityNotifier.php',
1721+
'OC\\DB\\Middleware\\ConnectionActivityStatement' => $baseDir . '/lib/private/DB/Middleware/ConnectionActivityStatement.php',
17171722
'OC\\DB\\Middleware\\UtcTimezoneMiddleware' => $baseDir . '/lib/private/DB/Middleware/UtcTimezoneMiddleware.php',
17181723
'OC\\DB\\Middleware\\UtcTimezoneMiddlewareDriver' => $baseDir . '/lib/private/DB/Middleware/UtcTimezoneMiddlewareDriver.php',
17191724
'OC\\DB\\MigrationException' => $baseDir . '/lib/private/DB/MigrationException.php',

lib/composer/composer/autoload_static.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1755,6 +1755,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
17551755
'OC\\DB\\ConnectionFactory' => __DIR__ . '/../../..' . '/lib/private/DB/ConnectionFactory.php',
17561756
'OC\\DB\\DbDataCollector' => __DIR__ . '/../../..' . '/lib/private/DB/DbDataCollector.php',
17571757
'OC\\DB\\Exceptions\\DbalException' => __DIR__ . '/../../..' . '/lib/private/DB/Exceptions/DbalException.php',
1758+
'OC\\DB\\Middleware\\ConnectionActivityConnection' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityConnection.php',
1759+
'OC\\DB\\Middleware\\ConnectionActivityDriver' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityDriver.php',
1760+
'OC\\DB\\Middleware\\ConnectionActivityMiddleware' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityMiddleware.php',
1761+
'OC\\DB\\Middleware\\ConnectionActivityNotifier' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityNotifier.php',
1762+
'OC\\DB\\Middleware\\ConnectionActivityStatement' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/ConnectionActivityStatement.php',
17581763
'OC\\DB\\Middleware\\UtcTimezoneMiddleware' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/UtcTimezoneMiddleware.php',
17591764
'OC\\DB\\Middleware\\UtcTimezoneMiddlewareDriver' => __DIR__ . '/../../..' . '/lib/private/DB/Middleware/UtcTimezoneMiddlewareDriver.php',
17601765
'OC\\DB\\MigrationException' => __DIR__ . '/../../..' . '/lib/private/DB/MigrationException.php',

lib/private/DB/Connection.php

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use Doctrine\DBAL\Result;
2626
use Doctrine\DBAL\Schema\Schema;
2727
use Doctrine\DBAL\Statement;
28+
use OC\DB\Middleware\ConnectionActivityNotifier;
2829
use OC\DB\QueryBuilder\Partitioned\PartitionedQueryBuilder;
2930
use OC\DB\QueryBuilder\Partitioned\PartitionSplit;
3031
use OC\DB\QueryBuilder\QueryBuilder;
@@ -63,6 +64,8 @@ class Connection extends PrimaryReadReplicaConnection {
6364
protected int $queriesBuilt = 0;
6465
protected int $queriesExecuted = 0;
6566
protected ?DbDataCollector $dbDataCollector = null;
67+
/** Seconds the connection may sit idle before the next use re-verifies connectivity */
68+
private const CONNECTION_CHECK_INTERVAL = 30;
6669
private array $lastConnectionCheck = [];
6770

6871
protected ?float $transactionActiveSince = null;
@@ -122,6 +125,10 @@ 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+
$activityNotifier->setListener($this->refreshLastConnectionCheck(...));
131+
}
125132
$this->isShardingEnabled = isset($this->params['sharding']) && !empty($this->params['sharding']);
126133

127134
if ($this->isShardingEnabled) {
@@ -221,7 +228,7 @@ public function connect($connectionName = null) {
221228
$status = parent::connect();
222229
$eventLogger->end('connect:db');
223230

224-
$this->lastConnectionCheck[$this->getConnectionName()] = time();
231+
$this->refreshLastConnectionCheck();
225232

226233
return $status;
227234
} catch (Exception $e) {
@@ -902,21 +909,32 @@ public function rollBack() {
902909
private function reconnectIfNeeded(): void {
903910
if (
904911
!isset($this->lastConnectionCheck[$this->getConnectionName()])
905-
|| time() <= $this->lastConnectionCheck[$this->getConnectionName()] + 30
912+
|| time() <= $this->lastConnectionCheck[$this->getConnectionName()] + self::CONNECTION_CHECK_INTERVAL
906913
|| $this->isTransactionActive()
907914
) {
908915
return;
909916
}
910917

911918
try {
912919
$this->_conn->query($this->getDriver()->getDatabasePlatform()->getDummySelectSQL());
913-
$this->lastConnectionCheck[$this->getConnectionName()] = time();
920+
$this->refreshLastConnectionCheck();
914921
} catch (ConnectionLost|\Exception $e) {
915922
$this->logger->warning('Exception during connectivity check, closing and reconnecting', ['exception' => $e]);
916923
$this->close();
917924
}
918925
}
919926

927+
/**
928+
* A successful round trip proves the connection is alive: pushing the idle
929+
* timer forward keeps the connectivity probe of reconnectIfNeeded() from
930+
* firing between adjacent operations, where its query would reset the
931+
* driver level last insert id on MySQL. Invoked for every driver level
932+
* execution via the ConnectionActivityMiddleware.
933+
*/
934+
private function refreshLastConnectionCheck(): void {
935+
$this->lastConnectionCheck[$this->getConnectionName()] = time();
936+
}
937+
920938
private function getConnectionName(): string {
921939
return $this->isConnectedToPrimary() ? 'primary' : 'replica';
922940
}

lib/private/DB/ConnectionFactory.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Doctrine\DBAL\Configuration;
1313
use Doctrine\DBAL\DriverManager;
1414
use Doctrine\DBAL\Event\Listeners\OracleSessionInit;
15+
use OC\DB\Middleware\ConnectionActivityMiddleware;
1516
use OC\DB\Middleware\UtcTimezoneMiddleware;
1617
use OC\DB\QueryBuilder\Sharded\AutoIncrementHandler;
1718
use OC\DB\QueryBuilder\Sharded\ShardConnectionManager;
@@ -146,9 +147,12 @@ public function getConnection(string $type, array $additionalConnectionParams):
146147
break;
147148
}
148149
$configuration = new Configuration();
150+
$activityMiddleware = new ConnectionActivityMiddleware();
149151
$configuration->setMiddlewares([
150152
new UtcTimezoneMiddleware(),
153+
$activityMiddleware,
151154
]);
155+
$connectionParams['activity_notifier'] = $activityMiddleware->getNotifier();
152156
/** @var Connection $connection */
153157
$connection = DriverManager::getConnection(
154158
$connectionParams,
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
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\Result;
15+
use Doctrine\DBAL\Driver\Statement;
16+
17+
final class ConnectionActivityConnection extends AbstractConnectionMiddleware {
18+
public function __construct(
19+
Connection $connection,
20+
private ConnectionActivityNotifier $notifier,
21+
) {
22+
parent::__construct($connection);
23+
}
24+
25+
#[\Override]
26+
public function prepare(string $sql): Statement {
27+
return new ConnectionActivityStatement(parent::prepare($sql), $this->notifier);
28+
}
29+
30+
#[\Override]
31+
public function query(string $sql): Result {
32+
$result = parent::query($sql);
33+
$this->notifier->notify();
34+
return $result;
35+
}
36+
37+
#[\Override]
38+
public function exec(string $sql): int {
39+
$result = parent::exec($sql);
40+
$this->notifier->notify();
41+
return $result;
42+
}
43+
}
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: 55 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
#[\PHPUnit\Framework\Attributes\Group('DB')]
@@ -96,4 +99,56 @@ public function testClusterConnectsToPrimaryAndReplica(): void {
9699
$connection->ensureConnectedToReplica();
97100
}
98101

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

0 commit comments

Comments
 (0)