@IljaN these are the changes that you made to get CI passing with MariaDb 10.6. The CI passes here with MariaDb 10.2, so the changes are backward-compatible on a newly-installed system.

1. can you please write a changelog for this?

For everyone:
2) what should we do about checking upgrades of existing oC10 systems that use MariaDb 10.2/3/4/5 ?
3) what else can we think of that might be a problem?

Do we know how long it might take to migrate for a user with a big database ?

Do we know how long it might take to migrate for a user with a big database ?

I couldn't find any info on this. So I guess we need to test. If it's only a metadata DDL change for MariaDB then it might not take long (depends). By the looks of it (just guessing) MariaDB used to store Columns (and indices?) in a compressed form and deprecated this now. Two scenarios I can think of:

• We only change the DDL and Maria is smart enough to convert it's internal structure on the fly / on use
• Database needs to be taken offline and converted/migrated where all data on disk is rewritten

In this PR migration step for ALTERING ROW_FORMAT for tables is missing.

Is the migration something which should be done during a normal ownCloud (minor) upgrade or does it need planing by the admin? There is also the option to set the row type globally so we might be able to remove it from table definitions?

nextcloud/server#25436 (comment)

@IljaN ok, maybe we then provide Repair command only for these that fail to upgrade ? For all new instances we just do not use ROW_FORMAT ? Plus we make sure when MariaDB upgrade is attempted we throw proper error with what to do?

@IljaN @mrow4a I would rather love to have a migration command (if the migrate process is quite short),
ROW_FORMAT DYNAMIC also has the advantage of a faster database.

From a user perspective, figuring out the problem, reading the docs, run the repair command is pretty laborious,
what do you think?

I'd vote for removing the row format from our code. From my point of view, we shouldn't need to care about it. We could suggest to use a specific format because of performance reasons, but I don't think we should enforce it. The problem we have now is that we enforce to use a specific format that is now deprecated.

Ideally, that should be fixed by a DBA, completely isolated from ownCloud: switch off ownCloud, convert the DB (on your own, with whatever tools the DB provides), switch on ownCloud.

I'd vote for removing the row format from our code

I agree, I will check if we can set the row_format=DEFAULT in the migration which is DYNAMIC in MariaDB

NOTE: changed to DEFAULT, after migration row_format is DYNAMIC in mariadb10.6, which is good

We could bump the oC10 version from 10.8.1 prealpha to 10.9.0 prealpha in this PR. That should trigger the migration to auto-run when any developer/QA person updates. That will allow someone to test by installing MariaDb 10.5 with oC 10.8.0 checked out, then checkout this branch (or head of master when this is merged) and php occ:upgrade and it should "just work".

@phil-davis good idea, still wondering if we should migrate or repair (which should be triggered), my favor is migrate

@phil-davis good idea, still wondering if we should migrate or repair (which should be triggered), my favor is migrate

I am happy whichever is decided - auto-migrate, or something for an admin to do explicitly.

I'm not sure ownCloud 10.x can work normally without having that row format in the DB, at least until the next core upgrade.

Assuming OC 10.x can work without problems (at least until the next core upgrade), I'd remove the migration.

If the admin keeps using mariaDB 10.5, the only changes we have is that we no longer require the compressed row format, so the repair step won't do anything.

If the admin wants to use mariaDB 10.6, he can change the row format at any time; there is no need to wait for the upgrade (assuming OC doesn't give any problem). The only thing he needs to be careful about is to update to 10.9 for the next upgrade. The upgrade itself on our side won't do anything special.
The only problematic scenario is that the admin, having mariaDB 10.6, updates to OC 10.8 or earlier. In that case, I expect the migration to take some time because I expect ownCloud to change the DB row format back to compressed (due to the repair steps). Anyway, I don't think we can do anything about this other than document that mariaDB 10.6 is "officially supported" from 10.9 onward.

For an update from mariaDB 10.5 + OC 10.8 to mariaDB 10.6 + OC 10.9, I think the best path is:

1. Change row format in mariaDB 10.5
2. Update mariaDB to 10.6
3. Update OC to 10.9
   ownCloud should be able to run after each step without problem. In theory, you should be able to stay for a while in step 1, and the same for step 2.

@jvillafanez

If the admin keeps using mariaDB 10.5, the only changes we have is that we no longer require the compressed row format, so the repair step won't do anything.

This is not quite correct, I will give the admin the possibility to change MariaDB to 10.6 easily.
Why do we want to enforce our users to search for a problem, which we then need to explain and document, so the user needs to run several queries and commands?
We can solve all this overhead for us and the admin, by a simple migration

If we have a migration, then it is currently going to run on every installation where $platformName == 'mysql'

I guess that will be every installation that uses "real" MySQL and that uses any supported MariaDb 10.2, 10.3 10.4 or 10.5.

Do installations with "real" MySQL need to do this change at all?
(Is a similar deprecation coming for MySQL?)

Do installations with "real" MySQL need to do this change at all?
(Is a similar deprecation coming for MySQL?)

MySQL is not affected.

@phil-davis according to @jvillafanez and also this comment nextcloud/server#25436 (comment)
I agree that row_format should be set by the DBA if needed and not by ownCloud, so I like to
pull right away

It seems to be a mismatch between the code and the expectations. The code seems to convert the table row format to the default one, however, the command's help suggests that it's possible to convert to other row formats, but this isn't the case.

@jvillafanez We might extend it in the future, but not for now.

@jvillafanez We might extend it in the future, but not for now.

That's ok, no problem with that.

To clarify a bit my points:

• Any content that is facing the users or admins needs to reflect the actual state of how the code behaves. In this case, the command only allows conversion to the default format, which means that the option should be removed (we aren't allowing the admin to choose), and the help message should be adjusted (the conversion is to the default row format of the DB, not anyone)
• Future plans should be documented in the code. The idea is that, after forgetting about this piece of code, you should be able to reconstruct your thought process and implement what is missing following the original plan. Depending on how far you want to implement those plans now, you might need more or less documentation. For this case, I think the options are:
  • Implement the code as if only the the default row format will be used. No real reference that additional row formats could be used in the future.
  • Implement the code as if only the the default row format will be used but having a list of allowed formats with "default" being the only one allowed (I think this is the current state). It's a good point to have some comments around to explain the future plans and what is left to do.
  • Assume that, somehow, the row format will be gotten from the admin, although it's currently hardcoded to use "default" now. The code will have to assume a variable row format, and it might be a good idea to play around a bit by changing the hardcoded value to ensure the code works properly. The idea is to reduce the amount of changes to be done once we start implementing the plans.

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

96.7% Coverage
0.0% Duplication