linux-system-roles / storage

Ansible role for linux storage management
https://linux-system-roles.github.io/storage/
MIT License
101 stars 59 forks source link

feat: Add support for setting stripe size for LVM RAID #355

Closed vojtechtrefny closed 1 year ago

vojtechtrefny commented 1 year ago

Custom stripe size is needed for SAP HANA. We are using here the existing 'raid_chunk_size' property, the "chunk size" term comes from MD RAID where it has the same meaning as "stripe size" for LVM RAID.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: -0.02 :warning:

Comparison is base (1b4b4c5) 13.90% compared to head (b34c7f7) 13.88%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #355 +/- ## ========================================== - Coverage 13.90% 13.88% -0.02% ========================================== Files 8 8 Lines 1705 1707 +2 Branches 71 71 ========================================== Hits 237 237 - Misses 1468 1470 +2 ``` | Flag | Coverage Δ | | |---|---|---| | sanity | `16.54% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://app.codecov.io/gh/linux-system-roles/storage/pull/355?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles) | Coverage Δ | | |---|---|---| | [library/blivet.py](https://app.codecov.io/gh/linux-system-roles/storage/pull/355?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles#diff-bGlicmFyeS9ibGl2ZXQucHk=) | `0.00% <0.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

vojtechtrefny commented 1 year ago

I decided to reuse the raid_chunk_size argument for the LVM RAID stripe size here. I am not 100 % sure this is the best approach -- I wanted to avoid adding another LVM RAID specific argument when chunk and stripe size is basically the same thing in MD and LVM RAID, but it might be confusing. @dwlehman @richm what do you think?

richm commented 1 year ago

I decided to reuse the raid_chunk_size argument for the LVM RAID stripe size here. I am not 100 % sure this is the best approach -- I wanted to avoid adding another LVM RAID specific argument when chunk and stripe size is basically the same thing in MD and LVM RAID, but it might be confusing. @dwlehman @richm what do you think?

I would prefer to have some parameter with stripe_size in the name, but if 1) it will be difficult to add a new parameter and 2) storage admins will know that chunk_size and stripe_size are the same concept, then works for me. @briansmith0 wdyt?

briansmith0 commented 1 year ago

+1 to the comment from @richm

vojtechtrefny commented 1 year ago

Thank you for the feedback, I didn't realize that chunk size is also used by LVM for thin pools so using it for RAID could be confusing. #357 is replacement for this adding a new raid_stripe_size argument.