Skip to content

Commit 116c49e

Browse files
DerDreschnerbackportbot[bot]
authored andcommitted
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 <david.dreschner@nextcloud.com> [skip ci]
1 parent 28d1178 commit 116c49e

7 files changed

Lines changed: 255 additions & 2 deletions

lib/private/DB/Connection.php

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ public function connect($connectionName = null) {
160160
$status = parent::connect();
161161
$eventLogger->end('connect:db');
162162

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

165165
return $status;
166166
} catch (Exception $e) {
@@ -794,13 +794,24 @@ private function reconnectIfNeeded(): void {
794794

795795
try {
796796
$this->_conn->query($this->getDriver()->getDatabasePlatform()->getDummySelectSQL());
797-
$this->lastConnectionCheck[$this->getConnectionName()] = time();
797+
$this->refreshLastConnectionCheck();
798798
} catch (ConnectionLost|\Exception $e) {
799799
$this->logger->warning('Exception during connectivity check, closing and reconnecting', ['exception' => $e]);
800800
$this->close();
801801
}
802802
}
803803

804+
/**
805+
* A successful round trip proves the connection is alive: pushing the idle
806+
* timer forward keeps the connectivity probe of reconnectIfNeeded() from
807+
* firing between adjacent operations, where its query would reset the
808+
* driver level last insert id on MySQL. Invoked for every driver level
809+
* execution via the ConnectionActivityMiddleware.
810+
*/
811+
private function refreshLastConnectionCheck(): void {
812+
$this->lastConnectionCheck[$this->getConnectionName()] = time();
813+
}
814+
804815
private function getConnectionName(): string {
805816
return $this->isConnectedToPrimary() ? 'primary' : 'replica';
806817
}
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: 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
/**
@@ -98,4 +101,56 @@ 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+
$property->setValue($connection, ['primary' => time() - 20, 'replica' => time() - 20]);
152+
153+
return $property;
154+
}
155+
101156
}

0 commit comments

Comments
 (0)