fix(db): Prevent two connections for single node databases

[ChristophWurst]

• Resolves [Bug]: Two database connections and missing transaction isolation level for read operations #45003

Summary

Two connections to one database node

Since #41998 our connection always extends Doctrine's PrimaryReadReplicaConnection. This baseclass requires a primary and at least one replica and always does the read-write split.
For simpler Nextcloud installations we only have one database node and no replica. We do not want two database connections.
This overwrites the \Doctrine\DBAL\Connections\PrimaryReadReplicaConnection::performConnect method so we always connect to the primary if there is no replica. If there is one, we do the Doctrine read-write split.

Missing transaction isolation level for replica

Nextcloud uses READ COMMITTED as transaction isolation level. Setting it for the session was missing for the replica connections. Reverted because it implicitly causes a connection to the primary just to set the session variable.

How to test - Single node

1. Set up nextcloud with a single database node (mysql, postgres, oracle)
2. Enable query logging
   I. 'log_query' => true',
   II. 'query_log_file' => '/var/www/nextcloud/data/query.log',
3. Send some requests

master: you see replica and primary connections prefixes for the logged queries. That indicates that Nextcloud opens two connections per request.
here: you only see primary connections. That indicates that Nextcloud only opens one connection per request.

How to test - Cluster

1. Set up nextcloud with a single database node (mysql, postgres, oracle)
2. Add a fake replica by using a different hostname
3. Enable query logging
   I. 'log_query' => true',
   II. 'query_log_file' => '/var/www/nextcloud/data/query.log',
4. Send some requests

master and here: Nextcloud opens two connections.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[ChristophWurst]

The downside of this approach is that the query logging will speak the truth for single database node setups and prefix queries with "primary" even if they would go to a replica on clustered setups.

[ChristophWurst]

/backport to stable29

[ChristophWurst]

The downside of this approach is that the query logging will speak the truth for single database node setups and prefix queries with "primary" even if they would go to a replica on clustered setups.

This feels tricky to achieve without breaking too much. The current approach works for single node and clustered databases. A unit test was added to ensure this doesn't happen again.

[ChristophWurst]

I have reverted the second part of setting the transaction isolation level for read replicas.

This PR now only fixes having two connections for one request connecting to a single node database.

[kesselb]

Let's try to rewrite this code sometime and make our connection class a decorator for the doctrine classes rather than extending it.

[ChristophWurst]

Squashed

[artonge]

You apparently need to mock detectDatabasePlatform

https://github.com/nextcloud/server/actions/runs/9265386150/job/25487375754?pr=45013

[ChristophWurst]

Fixed by passing an actual platform. Thanks!

[szaimen]

Hi, is this fixing the problem #45257?

[ChristophWurst]

I don't think so