Add support for mariadb server config parameter #3763
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request update the MariaDB add-on to version 2.7.2, introducing new optional configuration parameters, Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
mariadb/config.yaml (1)
26-27: Approve the addition of theinnodb_buffer_pool_sizeparameter.The addition of the
innodb_buffer_pool_sizeparameter aligns with the PR objective and will allow users to optimize the performance of their MariaDB database by adjusting the buffer pool size. The default value of128Mis a reasonable starting point, but users with larger databases may need to adjust this value based on their specific requirements.Consider adding guidance in the documentation to help users determine the appropriate value for their use case. Factors to consider include:
- The size of the database
- The available memory on the system
- The expected workload and query patterns
Additionally, ensure that the documentation clearly explains the impact of this parameter on performance and provides instructions for monitoring and adjusting the value as needed.
Also applies to: 43-44
mariadb/DOCS.md (1)
79-79: Explanation looks good! Consider a minor rephrasing.The explanation provides clear context and rationale for introducing the new
parameters.innodb_buffer_pool_sizeparameter. It helpfully mentions the RAM availability caveat and includes a reference link to the related migration issues.Consider rephrasing the first sentence slightly to improve clarity:
-During Home Assistant schema updates some people experienced [errors][migration-issues] on large databases. +Some users have experienced [errors][migration-issues] during Home Assistant schema updates on large databases.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- mariadb/CHANGELOG.md (1 hunks)
- mariadb/DOCS.md (2 hunks)
- mariadb/config.yaml (3 hunks)
- mariadb/rootfs/etc/s6-overlay/s6-rc.d/mariadb-core/run (1 hunks)
Additional context used
Path-based instructions (3)
mariadb/config.yaml (6)
Pattern
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern
*/**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern
*/**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern
*/**(html|markdown|md): - Use sentence-style capitalization also in headings.
Pattern
*/**(html|markdown|md): do not comment on HTML used for icons
Pattern
*/**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.mariadb/CHANGELOG.md (6)
Pattern
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern
*/**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern
*/**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern
*/**(html|markdown|md): - Use sentence-style capitalization also in headings.
Pattern
*/**(html|markdown|md): do not comment on HTML used for icons
Pattern
*/**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.mariadb/DOCS.md (6)
Pattern
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern
*/**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern
*/**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern
*/**(html|markdown|md): - Use sentence-style capitalization also in headings.
Pattern
*/**(html|markdown|md): do not comment on HTML used for icons
Pattern
*/**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.
Markdownlint
mariadb/CHANGELOG.md
5-5: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (6)
mariadb/rootfs/etc/s6-overlay/s6-rc.d/mariadb-core/run (4)
7-8: LGTM!Initializing an empty string to hold the concatenated parameters is a good approach. The variable name
param_stringclearly conveys its purpose.
10-15: LGTM!The code segment correctly fetches the parameters using
bashio::configand processes them with awhileloop. The format--parameteris a common convention for command line parameters.
17-17: LGTM!Removing the trailing whitespace from
param_stringusingsedis a good practice to ensure clean and consistent strings.
20-22: LGTM!The changes to the logging statement and the execution command for
mysqldare beneficial:
- Including the command line parameters in the logging statement enhances visibility into the startup process.
- Modifying the execution command to include
param_stringallows for greater flexibility in the service's configuration at runtime.mariadb/config.yaml (1)
2-2: LGTM!The version update to 2.7.2 aligns with the PR objective and is expected to be backward compatible.
mariadb/DOCS.md (1)
77-78: LGTM!The new optional configuration parameter
parameters.innodb_buffer_pool_sizeis introduced correctly without breaking existing configurations. The parameter name is clear and follows the existing naming convention.
|
@coderabbitai review |
There was a problem hiding this comment.
Hello @agners
I saw that you were the last committer in the MariaDB addon, I'm not sure if anything else is to be done with this pull request for acceptance? The recommended edits by CodeRabbit were pushed, but seems there is no update to the PR.
It worked in dev container: |
Configure parameters the same way as in matter-server addon
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
mariadb/rootfs/etc/s6-overlay/s6-rc.d/mariadb-core/run (1)
16-16: Execution command looks good, consider adding quotes for robustness.The modified execution command correctly incorporates the additional arguments into the MariaDB startup process. The use of
execis appropriate for process management in this context.For improved robustness, consider quoting the
${MARIADB_DATA}variable:exec mysqld --datadir="${MARIADB_DATA}" --user=root "${extra_args[@]}" < /dev/nullThis ensures correct handling of the data directory path even if it contains spaces or special characters.
mariadb/DOCS.md (1)
77-83: Improve clarity and formatting in the new section.The new section on
mariadb_server_argsis informative but could be improved for clarity and adherence to guidelines:
Use bold for UI strings:
Change "Option:mariadb_server_args(optional)" to "Option: mariadb_server_args (optional)"Be more direct in the instruction:
Change "Defining the recommended parameters can help if there is RAM available." to "Define the recommended parameters if sufficient RAM is available."Provide more context for the example:
Add a brief explanation of what the example parameter does.Apply these changes to improve the section:
-### Option: `mariadb_server_args` (optional) +### Option: **mariadb_server_args** (optional) Some users have experienced [errors][migration-issues] during Home Assistant schema updates on large databases. -Defining the recommended parameters can help if there is RAM available. +Define the recommended parameters if sufficient RAM is available. -Example: `--innodb_buffer_pool_size=512M` +Example: `--innodb_buffer_pool_size=512M` (Increases the InnoDB buffer pool size to 512 MB)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- mariadb/DOCS.md (2 hunks)
- mariadb/config.yaml (2 hunks)
- mariadb/rootfs/etc/s6-overlay/s6-rc.d/mariadb-core/run (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
mariadb/DOCS.md (6)
Pattern
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern
*/**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern
*/**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern
*/**(html|markdown|md): - Use sentence-style capitalization also in headings.
Pattern
*/**(html|markdown|md): do not comment on HTML used for icons
Pattern
*/**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.mariadb/config.yaml (6)
Pattern
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern
*/**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern
*/**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern
*/**(html|markdown|md): - Use sentence-style capitalization also in headings.
Pattern
*/**(html|markdown|md): do not comment on HTML used for icons
Pattern
*/**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.
🔇 Additional comments (3)
mariadb/config.yaml (2)
2-2: Version bump to 2.7.2The version update from 2.7.1 to 2.7.2 aligns with the PR objectives to add support for MariaDB server config parameters. This minor version increment suggests the introduction of new features or improvements without breaking changes.
41-42: New configuration option:mariadb_server_argsThe addition of the
mariadb_server_argsoption is a good implementation to address the PR objectives. It allows users to specify additional MariaDB server arguments, including theinnodb_buffer_pool_sizeparameter.Some considerations:
- The implementation is flexible, allowing for multiple arguments to be specified.
- Using an array of strings is a good choice for maintaining readability in the YAML configuration.
- The
?indicates that this option is optional, which is appropriate for advanced configuration settings.To ensure this new option is properly integrated, let's verify its usage in the add-on script:
mariadb/DOCS.md (1)
Line range hint
77-111: LGTM with minor suggestions.The addition of the
mariadb_server_argsoption is a valuable improvement to the documentation. It provides users with a solution for potential schema update errors on large databases. The new section is well-placed and includes helpful context and an example.The minor suggestions provided will further enhance clarity and adherence to documentation guidelines. Overall, this change will be beneficial for users experiencing issues with large databases during Home Assistant schema updates.
Fix startup logic
|
Is someone able to reopen the issue #3754 ? |
|
Is there anything blocking the approval of this PR? |
Fixes #3754
Summary by CodeRabbit
New Features
parameters.innodb_buffer_pool_sizeandmariadb_server_argsfor enhanced server control and performance optimization.Documentation
Configuration Changes
config.yamlto include the new parameterinnodb_buffer_pool_sizewith a default value of128Mand addedmariadb_server_argsfor additional configuration flexibility.