-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Don't recommend specific tuning #7702
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are certainly specific settings. However, for certain smaller setups, it should work and for other setups, users might have a hint what parameters to change (e.g. increase buffer sizes). Having this spelled out in more detail, what parameters are required, and which ones have to be adjusted to the use case and how, that would be great.
If you just remove this information, users have nothing to start with. If the default configuration is good and performance is great, they won't change anything.
@tflidd Yeah but these parameters in the manual are just arbitrary and have no context. Most small deployments can basically function with their distro/packager/image defaults. I'm not against a deeper db tuning section, but that'd be a separate PR... And probably not in the general db config section where this stuff currently sits. :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase if needed due to passage of time, but LGTM.
/rebase |
As values vary a lot between setups, just include mandatory settings for databases?
85905b6
to
37db5bb
Compare
character_set_server = utf8mb4 | ||
collation_server = utf8mb4_general_ci | ||
transaction_isolation = READ-COMMITTED | ||
binlog_format = ROW | ||
innodb_large_prefix=on | ||
innodb_file_format=barracuda | ||
innodb_file_per_table=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
innodb_file_per_table=1 is important afaik. We need it for utf8mb4 as per https://github.com/nextcloud/documentation/blob/82ad366697c2ba982498bc71fea9d3a8e4215d39/admin_manual/configuration_database/mysql_4byte_support.rst.
Could you add it back and set a link to the mysql 4byte page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the docs are dated. AFAIK innodb_file_per_table=1
is the default in all the db versions we support:
https://dev.mysql.com/doc/refman/8.0/en/innodb-parameters.html#sysvar_innodb_file_per_table
Looks like on has been default since MySQL 5.6 and at least MariaDB Server 10.0.
It's also deprecated, at least in MariaDB, but that's a recent change and not relevant as long as we're supporting 10.x:
https://mariadb.com/kb/en/innodb-system-variables/#innodb_file_per_table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@solracsf mind to sign off the commits? |
As values vary a lot between setups, just include mandatory settings for databases?