Repository navigation
[Bug]: Two database connections and missing transaction isolation level for read operations #45003
Description
⚠️ This issue respects the following points: ⚠️
- This is a bug, not a question or a configuration/webserver/proxy issue.
- This issue is not already reported on Github OR Nextcloud Community Forum (I've searched it).
- Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
- I agree to follow Nextcloud's Code of Conduct.
Bug description
I am capturing all queries of my MariaDB installation and noticed a strange query pattern for simple Nextcloud requests:
-- nc_admin@localhost on nextclouddev using Socket
SELECT `appid`, `configkey`, `configvalue`, `type` FROM `oc_appconfig` WHERE `lazy` = 0;
SELECT `uid`, `displayname`, `password` FROM `oc_users` WHERE `uid_lower` = 'admin'
SELECT `gu`.`gid`, `g`.`displayname` FROM `oc_group_user` `gu` LEFT JOIN `oc_groups` `g` ON `gu`.`gid` = `g`.`gid` WHERE `uid` = 'admin'
SELECT `provider_id`, `enabled` FROM `oc_twofactor_providers` WHERE `uid` = 'admin'
SELECT `appid`, `configkey`, `configvalue` FROM `oc_preferences` WHERE `userid` = 'admin'
-- nc_admin@localhost on nextclouddev using Socket
SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED
SET SESSION AUTOCOMMIT=1
START TRANSACTION
SELECT * FROM `oc_authtoken` WHERE (`uid` = 'admin') AND (`version` = 2) LIMIT 1000
COMMIT
...Steps to reproduce
- Set up Nextcloud with a single node database
- Send requests
Expected behavior
Single connection and read queries use READ COMMITTED too, not the global database default.
Installation method
None
Nextcloud Server version
29
Operating system
None
PHP engine version
None
Web server
None
Database engine version
None
Is this bug present after an update or on a fresh install?
None
Are you using the Nextcloud Server Encryption module?
None
What user-backends are you using?
- Default user-backend (database)
- LDAP/ Active Directory
- SSO - SAML
- Other
Configuration report
No response
List of activated Apps
No response
Nextcloud Signing status
No response
Nextcloud Logs
No response
Additional info
Related to #41998.
Activity
ChristophWurst commentedon Apr 24, 2024
Stack traces
so it's the Doctrine primary/replica connection class that switches to the primary once we start a transaction. The class doesn't factor in that we only have primary and no replica and still opens a separate connection.
We set the primary parameters as the first replica if there is no replica config. Doctrine requires this for the Primary/Replica class, else it throws
Uncaught InvalidArgumentException: You have to configure at least one replica. in nextcloud/3rdparty/doctrine/dbal/src/Connections/PrimaryReadReplicaConnection.php:117.ChristophWurst commentedon Apr 24, 2024
I think the best fix would be to only subclass PrimaryReadReplicaConnection when db replicas are actually available and use the simple Doctrine Connection instead.
kesselb commentedon Apr 24, 2024
Good finding 👍
joshtrichards commentedon May 16, 2024
We've got some other bugs in this area of code too:
occ db:convert-typebroken sincePrimaryReadReplicaConnectionsupport added #45257Both of the above have the same underlying cause. #45257 is particularly nasty since it not only doesn't copy the source database, but then it clears out the (wrong) entire database schema (!).
We need to at a minimum clean up the parameter value merging for #45257 (or whatever is the appropriate approach with the new replica stuff now being in mainline). Ideally before next maintenance release (which admittedly is cutting it close and I'm not offering up a fix since I haven't had the time to look deep enough at that area of code so far and doubt I'll have the time to do in the next day or so).