Skip to content
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

Add setup warning in case row_format=compressed is still used #34497

Closed
2 tasks
PVince81 opened this issue Oct 10, 2022 · 8 comments · Fixed by #48547
Closed
2 tasks

Add setup warning in case row_format=compressed is still used #34497

PVince81 opened this issue Oct 10, 2022 · 8 comments · Fixed by #48547

Comments

@PVince81
Copy link
Member

PVince81 commented Oct 10, 2022

Since #30129 any new install of NC >= 24 will have row_format=dynamic with MariaDB.

Also, we saw reports where having "compressed" might negatively affect performance.

We should look into the following:

  • add a setup check that verifies that all known tables have row_format=dynamic instead of compressed.
  • if possible: add an occ command that migrates the tables to the new format. To be verified whether it's a quick thing or whether it can take hours for lots of data

@icewind1991 @juliushaertl @nickvergessen @CarlSchwan @szaimen

@PVince81 PVince81 added enhancement 1. to develop Accepted and waiting to be taken care of labels Oct 10, 2022
@PVince81 PVince81 added this to the Nextcloud 25.0.2 milestone Oct 10, 2022
@nickvergessen
Copy link
Member

if possible: add an occ command that migrates the tables to the new format. To be verified whether it's a quick thing or whether it can take hours for lots of data

We have that already as a repair step and it's also reachable via occ db:convert-mysql-charset ?
https://github.com/nextcloud/server/blob/master/lib/private/Repair/Collation.php#L82

Also your initial statement is wrong, every new install will NOT have compressed anymore, but dynamic.

@PVince81
Copy link
Member Author

sorry, I typed it wrong. fixed now.

hmm, the command occ db:convert-mysql-charset seems a bit obscure.
but if it does the job then we could direct to it from the setup check then

@blizzz blizzz modified the milestones: Nextcloud 25.0.3, Nextcloud 26 Jan 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@skjnldsv skjnldsv removed this from the Nextcloud 27.0.2 milestone Aug 8, 2023
@ChristophWurst
Copy link
Member

My observation on the dev env that was installed circa 2016 and only upgraded but never reset so far:

image

At some point I must have switched to UTF8 MB4 but that was at a time where we still created tables with row format compressed.

The query in \OC\Repair\Collation::getAllNonUTF8BinTables only checks the COLLATION_NAME and CHARACTER_SET.

Would it make sense to add a setup check and repair step for the row format specifically?

MariaDB [nextclouddev]> SELECT COUNT(*), ROW_FORMAT FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME LIKE 'oc_%' GROUP BY ROW_FORMAT;
+----------+------------+
| COUNT(*) | ROW_FORMAT |
+----------+------------+
|       53 | Compact    |
|      316 | Compressed |
|       52 | Dynamic    |
+----------+------------+
3 rows in set (0,021 sec)

My personal production instance of Nextcloud 27 confirms that not all upgrade paths lead to a fully dynamic row format for all tables:

MariaDB [(none)]> SELECT COUNT(*), ROW_FORMAT FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME LIKE 'oc_%' GROUP BY ROW_FORMAT;
+----------+------------+
| COUNT(*) | ROW_FORMAT |
+----------+------------+
|      214 | Compressed |
|       19 | Dynamic    |
+----------+------------+
2 rows in set (0,008 sec)

@artonge
Copy link
Contributor

artonge commented Jun 20, 2024

@Altahrim can you take a look and tell us whether we should create a setup check or a repair step?
The question is, if the conversion from compressed to dynamic takes too much time, then a repair step could be an issue, but we at least need to tell the admin about it.

Example code for the future implementation:

@ChristophWurst
Copy link
Member

I would advice against a repair step. This migration takes very long and it roughly doubles the required disk space. Admins need to plan for both.

@Altahrim
Copy link
Collaborator

I would also vote for a setup check :)

@ChristophWurst
Copy link
Member

@sorbaugh
Copy link
Contributor

@Altahrim let's schedule this

@Altahrim Altahrim added 3. to review Waiting for reviews and removed 1. to develop Accepted and waiting to be taken care of labels Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ☑️ Done
Development

Successfully merging a pull request may close this issue.

9 participants