-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug]: db:convert-type uses systemConfig instead of command line parameters for connection to new target database #45257
Comments
Logging the Parameter FromDB
ToDB
It is notable that in the second case (which should be the postgres-database configured via the CLI arguments), the host, dbname and port in the top-level array are set correctly from the CLI, Both the |
Possibly related: #45097 I don't have time to look tonight, but your analysis seems sound (and, given the added evidence from the log output, even more so). Cc: @juliushaertl |
I can verify but can't contribute logs. I used the --clear-schema option in my attempt and my original MariaDB DB got deleted. Good thing I have backups. |
I modified https://github.com/nextcloud/server/blob/master/core/Command/Db/ConvertType.php#L247
and seems all converted Also have to remove mysql.utf8mb4 parameter from config.php to perform conversion to remove client_encoding from params of DbFactory |
Got hit by this a week ago. Is that particular Nextcloud instance completely screwed? |
Yes, unfortunately. 😢 The end result is the parameters get swapped so it likely dropped the source database. Fortunately, recovery should be a simple matter of restoring from backup (just the db portion): https://docs.nextcloud.com/server/latest/admin_manual/maintenance/restore.html#restore-database (or whatever other mechanism you utilize in your environment for database backup/recovery) |
@Delagen Mind pushing that as PR? I know there may be additional changes needed, but it'll start things in motion. |
Needed until #45257 is addressed to prevent data loss Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards I don't mind that my changes solve the problem correctly. It only makes possible for me successful conversion from MySQL to Postgres |
I have just experienced this problem on a new installation. |
Needed until #45257 is addressed to prevent data loss Signed-off-by: Josh <josh.t.richards@gmail.com>
Would not be it the simplest solution, if the ConnectionFactory::getConnection() method would first check if the |
Needed until #45257 is addressed to prevent data loss Signed-off-by: Josh <josh.t.richards@gmail.com>
Hi, is this #45013 fixing the problem? |
I just looked into it a bit and so far it looks like the databases get mixed up somewhere in the script. When I hardcoded the |
Possible patch candidate, but haven't had time to verify and test this yet: diff --git a/core/Command/Db/ConvertType.php b/core/Command/Db/ConvertType.php
index 333f29625f6..078fd47965b 100644
--- a/core/Command/Db/ConvertType.php
+++ b/core/Command/Db/ConvertType.php
@@ -227,15 +227,22 @@ class ConvertType extends Command implements CompletionAwareInterface {
protected function getToDBConnection(InputInterface $input, OutputInterface $output) {
$type = $input->getArgument('type');
$connectionParams = $this->connectionFactory->createConnectionParams();
- $connectionParams = array_merge($connectionParams, [
+
+ $overrideConnectionParams = [
'host' => $input->getArgument('hostname'),
'user' => $input->getArgument('username'),
'password' => $input->getOption('password'),
'dbname' => $input->getArgument('database'),
- ]);
+ ];
if ($input->getOption('port')) {
- $connectionParams['port'] = $input->getOption('port');
+ $overrideConnectionParams['port'] = $input->getOption('port');
}
+
+ $connectionParams = array_merge($connectionParams, [
+ 'primary' => $overrideConnectionParams,
+ 'replica' => [$overrideConnectionParams],
+ ]);
+
return $this->connectionFactory->getConnection($type, $connectionParams);
}
|
Hi @intersectRaven @ksmonkey123 @Delagen @HammyHavoc @chimpboy @rotdrop are you able to test the above patch? Thanks in advance! :) |
@szaimen I'll be probably playing around with a new (old) server this evening. I'll back up my database and try the patch out. I'll know practically immediately as I get the duplicate entry error right at the start if it tries to write into the original DB. |
I tried it out, it doesn't work. I get stuck on |
The conversion script gets stuck on this line server/core/Command/Db/ConvertType.php Line 210 in 34f5cba
|
Bug description
The OCC-Command
db:convert-type
seems to ignore the DB properties (type, user, host, database, port, password) provided through the CLI and instead operates on the database configured inconfig.php
I believe this may be related to the commit 79c4986 as this reworked the parameter handling in
ConnectionFactory.php
.If I read this correctly, the default parameters are now always generated based on the system config instead of the passed arguments.
Running
db:convert-type --clean-schema
drops the tables in the "old" database.Steps to reproduce
php occ db:convert-type --password=<PASSWORD> --port=5432 pgsql nextcloud_app postgres nextcloud
(note: the hostpostgres
is a Kubernetes service name)oc_accounts
, I assume because it tries to migrate the content of the old database into itself.Expected behavior
db:convert-type
Installation method
Community Docker image
Nextcloud Server version
29
Operating system
Debian/Ubuntu
PHP engine version
None
Web server
None
Database engine version
MySQL
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?
Configuration report
List of activated Apps
Nextcloud Signing status
Nextcloud Logs
No response
Additional info
nextcloud:29.0.0
mysql:8.4.0
postgres:16.3
The text was updated successfully, but these errors were encountered: