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.

Sign up for GitHub

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

Jump to bottom

ceph-volume,python-common: Data allocate fraction #40659

Merged
merged 7 commits into from
Jun 7, 2021
Merged

ceph-volume,python-common: Data allocate fraction #40659

merged 7 commits into from
Jun 7, 2021

Conversation

PepperJo
Copy link
Contributor

@PepperJo PepperJo commented Apr 8, 2021

This PR allows to partially allocate a data device when preparing/deploying an OSD with ceph-volume or through the device group specification. The fraction can be between (0,1.0] where a fraction of e.g. 0.8 will allocate 80% of the data device, i.e. the lvm size will only account for 80% of the device's capacity. The rest of the device will not be allocated. This can be used to alleviate write performance degradation of Flash SSDs.
This PR adds --data-allocate-fraciton to ceph-volume lvm batch and the data_allocate_fraction feature to the device group specification.

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@PepperJo PepperJo requested review from a team as code owners April 8, 2021 08:47
@sebastian-philipp sebastian-philipp changed the title Data allocate fraction ceph-volume,pyhton-common: Data allocate fraction Apr 23, 2021
@sebastian-philipp
Copy link
Contributor

Added [needs-review]

andrewschoen
andrewschoen requested changes Jun 2, 2021
View reviewed changes
Copy link
Contributor

@andrewschoen andrewschoen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good. I think we need to add tests for data_allocate_fraction though. Tests for other argument validators can be found in tests.util.test_arg_validators as an example.

src/ceph-volume/ceph_volume/devices/lvm/batch.py Outdated Show resolved Hide resolved
@PepperJo
ceph-volume: data_allocate_pct: allocate percentage
9e0aaa4 
Add data-allocate-pct to ceph-volume lvm batch to allow to only
allocate a certain percentage of a drive, e.g. to reduce wear leveling
impact on SSDs.

Signed-off-by: Jonas Pfefferle <pepperjo@japf.ch>
@PepperJo
python-common: translate to_ceph_volume: add allocation fraction
4d518ca 
Add --data-allocate-fraction to translate from json to ceph-volume
command.

Signed-off-by: Jonas Pfefferle <pepperjo@japf.ch>
@PepperJo
python-common: add data_allocate_fraction to DriveGroupSpec
8f249d1 
data_allocate_fraction is used to partially allocate a drive.

Signed-off-by: Jonas Pfefferle <pepperjo@japf.ch>
@PepperJo
ceph-volume: data_allocate_fraction: new validation class
287ab0e 
Move validator for data_allocate_fraction to utils.arg_validators

Signed-off-by: Jonas Pfefferle <pepperjo@japf.ch>
@PepperJo
ceph-volume: data_allocate_fraction: arg_validators tests
adf8c17 
Add tests for argument validator. Check for Nan, <=0.0 and > 1.0

Signed-off-by: Jonas Pfefferle <pepperjo@japf.ch>
@andrewschoen
Copy link
Contributor

jenkins test ceph-volume all

@andrewschoen
Copy link
Contributor

jenkins test ceph-volume tox

@PepperJo PepperJo force-pushed the data_allocate_fraction_v2 branch 3 times, most recently from 99a0045 to 492e6dd Compare June 4, 2021 11:59
@PepperJo
ceph-volume: data_allocate_fraction: batch tests
43b06cd 
lvm batch tests with data allocate fraction

Signed-off-by: Jonas Pfefferle <pepperjo@japf.ch>
@PepperJo
python-common: data_allocate_fraction translate test
6a41314 
Add DriveGroupSpec translate test with data_allocate_fraction

Signed-off-by: Jonas Pfefferle <pepperjo@japf.ch>
@andrewschoen
Copy link
Contributor

jenkins test ceph-volume all

@andrewschoen
Copy link
Contributor

jenkins test ceph-volume tox

@tchaikov tchaikov changed the title ceph-volume,pyhton-common: Data allocate fraction ceph-volume,python-common: Data allocate fraction Jun 4, 2021
@andrewschoen
Copy link
Contributor

jenkins test ceph-volume tox

@PepperJo
Copy link
Contributor Author

PepperJo commented Jun 4, 2021

@andrewschoen thanks for your review. I moved the function as suggested and added a few tests.

@PepperJo PepperJo requested review from andrewschoen and removed request for a team June 4, 2021 16:57
@andrewschoen
Copy link
Contributor

jenkins test ceph-volume all

@andrewschoen
Copy link
Contributor

@andrewschoen thanks for your review. I moved the function as suggested and added a few tests.

@PepperJo thank you for the work! These changes look great. If the CI finishes clean, I'll approve this.

@PepperJo
Copy link
Contributor Author

PepperJo commented Jun 5, 2021

@andrewschoen I checked the failed tests and if I interpret them correctly there seems to be a problem with fetch ceph red hat development repository? That seems to be unrelated to my changes.

@andrewschoen
Copy link
Contributor

jenkins test ceph-volume all

@andrewschoen
Copy link
Contributor

@andrewschoen I checked the failed tests and if I interpret them correctly there seems to be a problem with fetch ceph red hat development repository? That seems to be unrelated to my changes.

@PepperJo I agree, they do look unrelated. It looks like there was some trouble accessing some dev repos to test with, which I suspect is transient. I'll try again as these tests did all pass on previous runs.

@andrewschoen
Copy link
Contributor

jenkins test ceph-volume lvm centos8-filestore-dmcrypt

@andrewschoen
Copy link
Contributor

jenkins test ceph-volume lvm centos8-bluestore-dmcrypt

1 similar comment
@andrewschoen
Copy link
Contributor

jenkins test ceph-volume lvm centos8-bluestore-dmcrypt

@andrewschoen andrewschoen merged commit acaad27 into ceph:master Jun 7, 2021
@PepperJo
Copy link
Contributor Author

PepperJo commented Jun 8, 2021

@andrewschoen Thank you!

@adk3798 adk3798 mentioned this pull request Sep 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewschoen andrewschoen andrewschoen approved these changes

Assignees
No one assigned
Labels
ceph-volume cephadm needs-review pybind
Projects
None yet
Milestone
No milestone
3 participants
@PepperJo @sebastian-philipp @andrewschoen