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

Jump to bottom

osd: support pool level recovery_priority and recovery_op_priority #5953

Merged
merged 4 commits into from Dec 4, 2015
Merged

osd: support pool level recovery_priority and recovery_op_priority #5953

merged 4 commits into from Dec 4, 2015

Conversation

guangyy
Copy link
Contributor

@guangyy guangyy commented Sep 17, 2015

Fixes: 13121
Signed-off-by: Guang Yang yguang@yahoo-inc.com

@guangyy
Copy link
Contributor Author

guangyy commented Sep 17, 2015

@liewegas , per the discussion on the mailing list, we would like to think through the way to manage pool settings, could we track that separately with a code refactor?

@liewegas
Copy link
Member

Sure

@guangyy
Copy link
Contributor Author

guangyy commented Sep 17, 2015

@dachary , the failure on ubuntu looks like a false positive?

@ghost
Copy link

ghost commented Sep 17, 2015

@guangyy you are correct.

Failed building wheel for coverage

Please ignore the false negative from the bot on Ubuntu 14.04.

@guangyy
Copy link
Contributor Author

guangyy commented Sep 17, 2015

Thanks @dachary .

@guangyy
Copy link
Contributor Author

guangyy commented Sep 22, 2015

Rebased to resolve the conflicts. @liewegas , could you help to review this one? Thanks!

@loic-bot
Copy link

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/7953/

@guangyy
Copy link
Contributor Author

guangyy commented Sep 24, 2015

Hi @dzafman , could you take a look at this one? I referred 0985ae7 and 00e9031

dzafman
dzafman reviewed Sep 28, 2015
View reviewed changes
src/mon/OSDMonitor.cc
@@ -3539,6 +3543,11 @@ bool OSDMonitor::preprocess_command(MonOpRequestRef op)
break;
case DEEP_SCRUB_INTERVAL:
f->dump_int("deep_scrub_interval", p->deep_scrub_interval);
case RECOVERY_PRIORITY:
Copy link
Contributor

Choose a reason for hiding this comment

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

break missing before this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Accepted.

@dzafman
Copy link
Contributor

dzafman commented Sep 28, 2015

Only some minor issues. Needs a rebase too.

@dzafman dzafman self-assigned this Sep 28, 2015
@guangyy
Copy link
Contributor Author

guangyy commented Sep 29, 2015

Thanks @dzafman for the review. Looks like we should wait for #6081 and rebase on top of that one to proceed. I will do once that PR pass the review.

@ghost ghost added feature core labels Oct 16, 2015
@ghost
Copy link

ghost commented Oct 16, 2015

@guangyy this needs rebasing

@guangyy
Copy link
Contributor Author

guangyy commented Oct 16, 2015

Hi @dachary , yeah I am waiting for #6081, this patch depends on that one, once that one get reviewed, I will rebase on that one. Thanks!

Guang Yang added 2 commits December 2, 2015 01:51
osd: add recovery_priority and recovery_op_priority to pool_opts_t
61f61a2 
Fixes: 13121
Signed-off-by: Guang Yang <yguang@yahoo-inc.com>
osd: prioritize recovery based on pool's customized priority
7455897 
Fixes: 13121
Signed-off-by: Guang Yang <yguang@yahoo-inc.com>
@guangyy
Copy link
Contributor Author

guangyy commented Dec 3, 2015

Hi @dzafman ,
Rebased and do some changes according tot he review comments, please help to review again.

dzafman
dzafman reviewed Dec 3, 2015
View reviewed changes
src/mon/MonCommands.h
@@ -674,11 +674,11 @@ COMMAND("osd pool rename " \
"rename <srcpool> to <destpool>", "osd", "rw", "cli,rest")
COMMAND("osd pool get " \
"name=pool,type=CephPoolname " \
"name=var,type=CephChoices,strings=size|min_size|crash_replay_interval|pg_num|pgp_num|crush_ruleset|hashpspool|nodelete|nopgchange|nosizechange|write_fadvise_dontneed|noscrub|nodeep-scrub|hit_set_type|hit_set_period|hit_set_count|hit_set_fpp|auid|target_max_objects|target_max_bytes|cache_target_dirty_ratio|cache_target_dirty_high_ratio|cache_target_full_ratio|cache_min_flush_age|cache_min_evict_age|erasure_code_profile|min_read_recency_for_promote|all|min_write_recency_for_promote|fast_read|hit_set_grade_decay_rate|hit_set_search_last_n|scrub_min_interval|scrub_max_interval|deep_scrub_interval", \
"name=var,type=CephChoices,strings=size|min_size|crash_replay_interval|pg_num|pgp_num|crush_ruleset|hashpspool|nodelete|nopgchange|nosizechange|write_fadvise_dontneed|noscrub|nodeep-scrub|hit_set_type|hit_set_period|hit_set_count|hit_set_fpp|auid|target_max_objects|target_max_bytes|cache_target_dirty_ratio|cache_target_dirty_high_ratio|cache_target_full_ratio|cache_min_flush_age|cache_min_evict_age|erasure_code_profile|min_read_recency_for_promote|all|min_write_recency_for_promote|fast_read|hit_set_grade_decay_rate|hit_set_search_last_n|scrub_min_interval|scrub_max_interval|deep_scrub_interval|recovery_priority|recovery_op_priorit", \
Copy link
Contributor

Choose a reason for hiding this comment

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

recovery_op_priorit missing "y"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, sorry missing it.

@loic-bot
Copy link

loic-bot commented Dec 3, 2015

FAILURE: http://jenkins.ceph.dachary.org/job/ceph/9784/

@dzafman
Copy link
Contributor

dzafman commented Dec 3, 2015

One of my review comments caused the Jenkins failure. We should have a functional test case.

Guang Yang added 2 commits December 3, 2015 02:10
mon: support recovery_priority and recovery_op_priority with monitor …
f0cc722 
…commands

Fixes: 13121
Signed-off-by: Guang Yang yguang@yahoo-inc.com
qa: add a test for the recovery_priority/recovery_op_priority pool se…
ba346bf 
…ttings.

Signed-off-by: Guang Yang <yguang@yahoo-inc.com>
@loic-bot
Copy link

loic-bot commented Dec 3, 2015

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/9790/

liewegas added a commit that referenced this pull request Dec 4, 2015
@liewegas
Merge pull request #5953 from guangyy/wip-13121
c3e3ee0 
osd: support pool level recovery_priority and recovery_op_priority

Reviewed-by: David Zafman <dzafman@redhat.com>
@liewegas liewegas merged commit c3e3ee0 into ceph:master Dec 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dzafman dzafman

Labels
core feature needs-qa wip-sage-testing
Projects
None yet
Milestone
No milestone
4 participants
@guangyy @liewegas @loic-bot @dzafman