-
Notifications
You must be signed in to change notification settings - Fork 6k
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
osd: add asock command to dump the scrub queue #12728
Conversation
71c5a74
to
71f1016
Compare
FYI, result of this command:
and, xml result:
|
assert(f != nullptr); | ||
Mutex::Locker l(sched_scrub_lock); | ||
|
||
f->open_array_section("sched scrubs"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an XML tag cannot contain space in it. see https://www.w3.org/TR/2008/REC-xml-20081126/#NT-NameChar.
so probably we can simply put "scrubs" as the tag name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 ok
@@ -686,7 +686,7 @@ class OSDService { | |||
} | |||
|
|||
void dumps_scrub(Formatter *f) { | |||
assert(f!=nullptr); | |||
assert(f != nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please squash this commit into the previous one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i will squash commits after review
r = admin_socket->register_command("dump_scrubs", | ||
"dump_scrubs", | ||
asok_hook, | ||
"print schedlued scrubs"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/schedlued/scheduled/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. thanks for your review
f->open_object_section("scrub"); | ||
f->dump_stream("pgid") << i.pgid; | ||
f->dump_stream("sched_time") << i.sched_time; | ||
f->dump_stream("deadline") << i.deadline; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can have a boolean field named "forced"? see http://tracker.ceph.com/issues/17861#note-5. i know, technically, it's redundant. but it helps user to understand this scrub. @athanatos what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it has been added. by the way, it seems that there isn't not a way to do a force scrub via ceph tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both "ceph osd scrub ${osd_num}" and "ceph pg scrub ${pg_num}" force a scrub job.
91545c1
to
7d1a627
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
r = admin_socket->register_command("dump_scrubs", | ||
"dump_scrubs", | ||
asok_hook, | ||
"print schedlued scrubs"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. thanks for your review
f->open_object_section("scrub"); | ||
f->dump_stream("pgid") << i.pgid; | ||
f->dump_stream("sched_time") << i.sched_time; | ||
f->dump_stream("deadline") << i.deadline; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it has been added. by the way, it seems that there isn't not a way to do a force scrub via ceph tools.
This seems reasonable to me. I'm ok with it if @tchaikov is. |
nit, the change happens in "osd" but not in any of the cli tools, might want to change the prefix of the title to "osd:". |
@tchaikov OK, i will change title and test again. |
@liuchang0812 sorry for the confusion. and please note that the title of this pull request is different from the one of your commit. i recommend change the later. |
😄 @tchaikov I get it now |
* add a public method to dumps scrubs * register new command "dump_scrubs" to asock_admin Fixes: http://tracker.ceph.com/issues/17861 Signed-off-by: liuchang0812 <liuchang0812@gmail.com>
7d1a627
to
7542a35
Compare
@tchaikov Updated, Waiting for test result. |
any comments is appreciated.
@tchaikov @liewegas @athanatos
Fixes: http://tracker.ceph.com/issues/17861
Signed-off-by: liuchang0812 liuchang0812@gmail.com