Skip to content

Commit

Permalink
fix: Initialize lastConnectionCheck after first connection
Browse files Browse the repository at this point in the history
We are checking whether the DB connection is alive once every 30 seconds. But when we are lacking the last check time, we are skipping the check and reconnect logic. This is causing the reconnect logic to never fire in those cases.

It seems to me that "those cases", are actually always the case, as upon initialization, we are not using the proper connection name to store the time.

In the `connect()` logic, when `$this->_conn` is null, `$this->getConnectionName()` is returning `replica`, so `$this->lastConnectionCheck` will be equal to `['replica' => time()];`

https://github.com/nextcloud/server/blob/60711ea4cfde6f53d0b18bcd7e166a34a43056a5/lib/private/DB/Connection.php#L215-L221

https://github.com/nextcloud/server/blob/60711ea4cfde6f53d0b18bcd7e166a34a43056a5/lib/private/DB/Connection.php#L891-L893

https://github.com/nextcloud/3rdparty/blob/2b6d7bf65ff242ea050e736925f752a38d8da220/doctrine/dbal/src/Connections/PrimaryReadReplicaConnection.php#L136-L139

Then, if the connection name ends up as being 'primary', the reconnect logic is skipped:

https://github.com/nextcloud/server/blob/60711ea4cfde6f53d0b18bcd7e166a34a43056a5/lib/private/DB/Connection.php#L874-L880

Follow-up of #41819

Signed-off-by: Louis Chemineau <[email protected]>
  • Loading branch information
artonge committed Feb 18, 2025
1 parent c71bc06 commit 10c0c78
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/private/DB/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -218,15 +218,15 @@ public function connect($connectionName = null) {
return parent::connect();
}

$this->lastConnectionCheck[$this->getConnectionName()] = time();

// Only trigger the event logger for the initial connect call
$eventLogger = Server::get(IEventLogger::class);
$eventLogger->start('connect:db', 'db connection opened');
/** @psalm-suppress InternalMethod */
$status = parent::connect();
$eventLogger->end('connect:db');

$this->lastConnectionCheck[$this->getConnectionName()] = time();

return $status;
} catch (Exception $e) {
// throw a new exception to prevent leaking info from the stacktrace
Expand Down

0 comments on commit 10c0c78

Please sign in to comment.