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

mon/OSDMonitor: Warnings for expected_num_objects #23072

Merged
merged 2 commits into from
Jul 26, 2018

Conversation

fullerdj
Copy link
Contributor

Warn if the user tries to create a pool with wrong or missing expected_num_objects

if (expected_num_objects == 0 &&
cct->_conf->osd_objectstore == "filestore" &&
cct->_conf->filestore_merge_threshold < 0) {
if (pg_num >= 1024 || pg_num / osdmap.get_num_osds() >= 50) {
Copy link
Member

Choose a reason for hiding this comment

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

Did we change the num of pgs per osd threshold from 100 to 50?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't we? I thought we did in the last conversation we had. If you think we should keep at 100, I'm ok with that.

if (expected_num_objects == 0 &&
cct->_conf->osd_objectstore == "filestore" &&
cct->_conf->filestore_merge_threshold < 0) {
if (pg_num >= 1024 || pg_num / osdmap.get_num_osds() >= 50) {
Copy link
Member

Choose a reason for hiding this comment

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

should check for divide by zero here - you could have no osds

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch.

@fullerdj
Copy link
Contributor Author

@jdurgin, @neha-ojha pointed out that I mentioned 100 PGs/OSD as the threshold before (and once since) writing this code. Did we settle on 50 or was it 100 the whole time?

@jdurgin
Copy link
Member

jdurgin commented Jul 17, 2018 via email

@fullerdj
Copy link
Contributor Author

fullerdj commented Jul 17, 2018 via email

@yuriw
Copy link
Contributor

yuriw commented Jul 17, 2018

@neha-ojha this needs to be fixed/rebased https://pastebin.com/wu5aaXZz

The expected_num_objects argument to ceph osd pool create is
only effective on filestore pools when merging is disabled
(filestore_merge_threshold < 0). Warn and disallow pool creation
in this situation.

Signed-off-by: Douglas Fuller <dfuller@redhat.com>
@yuriw
Copy link
Contributor

yuriw commented Jul 18, 2018

still broken https://pastebin.com/6kdiUsgU

@smithfarm smithfarm changed the title Warnings for expected_num_objects mon/OSDMonitor: Warnings for expected_num_objects Jul 19, 2018
@yuriw
Copy link
Contributor

yuriw commented Jul 20, 2018

@fullerdj fullerdj added the DNM label Jul 23, 2018
@fullerdj
Copy link
Contributor Author

[dnm] added while pgcalc discussion is ongoing.

When creating a pool on filestore, warn if the user appears to be
creating a pool to store a large number of objects but omitted the
expected_num_objects parameter. Create the pool anyway.

Fixes: http://tracker.ceph.com/issues/24687
Signed-off-by: Douglas Fuller <dfuller@redhat.com>
@fullerdj
Copy link
Contributor Author

Message updated and DNM tag removed.

@jdurgin jdurgin merged commit ce9c45c into ceph:master Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants