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: 'osd tree in|out|up|down' to filter tree results #15294

Merged
merged 2 commits into from May 30, 2017

Conversation

Projects
None yet
3 participants
@liewegas
Member

liewegas commented May 25, 2017

This lets you easily see down OSDs and where they are with
'ceph osd tree down'.

gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree
ID WEIGHT  TYPE NAME     UP/DOWN REWEIGHT PRIMARY-AFFINITY 
-1 5.00000 root default                                    
-2 3.00000     host gnit                                   
 0 1.00000         osd.0      up  1.00000          1.00000 
 1 1.00000         osd.1      up  1.00000          1.00000 
 2 1.00000         osd.2      up        0          1.00000 
-3 2.00000     host foo                                    
 3 1.00000         osd.3    down        0          1.00000 
 4 1.00000         osd.4      up  1.00000          1.00000 
gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree up
ID WEIGHT  TYPE NAME     UP/DOWN REWEIGHT PRIMARY-AFFINITY 
-1 5.00000 root default                                    
-2 3.00000     host gnit                                   
 0 1.00000         osd.0      up  1.00000          1.00000 
 1 1.00000         osd.1      up  1.00000          1.00000 
 2 1.00000         osd.2      up        0          1.00000 
-3 2.00000     host foo                                    
 4 1.00000         osd.4      up  1.00000          1.00000 
gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree down
ID WEIGHT  TYPE NAME     UP/DOWN REWEIGHT PRIMARY-AFFINITY 
-1 5.00000 root default                                    
-3 2.00000     host foo                                    
 3 1.00000         osd.3    down        0          1.00000 
gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree in
ID WEIGHT  TYPE NAME     UP/DOWN REWEIGHT PRIMARY-AFFINITY 
-1 5.00000 root default                                    
-2 3.00000     host gnit                                   
 0 1.00000         osd.0      up  1.00000          1.00000 
 1 1.00000         osd.1      up  1.00000          1.00000 
-3 2.00000     host foo                                    
 4 1.00000         osd.4      up  1.00000          1.00000 
gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree out
ID WEIGHT  TYPE NAME     UP/DOWN REWEIGHT PRIMARY-AFFINITY 
-1 5.00000 root default                                    
-2 3.00000     host gnit                                   
 2 1.00000         osd.2      up        0          1.00000 
-3 2.00000     host foo                                    
 3 1.00000         osd.3    down        0          1.00000 
gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree out down
ID WEIGHT  TYPE NAME     UP/DOWN REWEIGHT PRIMARY-AFFINITY 
-1 5.00000 root default                                    
-3 2.00000     host foo                                    
 3 1.00000         osd.3    down        0          1.00000 
gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree out up
ID WEIGHT  TYPE NAME     UP/DOWN REWEIGHT PRIMARY-AFFINITY 
-1 5.00000 root default                                    
-2 3.00000     host gnit                                   
 2 1.00000         osd.2      up        0          1.00000 
gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree in out
Error EINVAL: cannot specify both up and down or both in and out
gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree up down
Error EINVAL: cannot specify both up and down or both in and out
gnit:build (wip-osd-tree) 11:37 AM $ bin/ceph osd tree foo
foo not valid:  foo not in up|down|in|out
Invalid command:  unused arguments: [u'foo']
osd tree {} {up|down|in|out [up|down|in|out...]} :  print OSD tree
Error EINVAL: invalid command
gnit:build (wip-osd-tree) 11:38 AM $ bin/ceph osd tree foo up
foo not valid:  foo not in up|down|in|out
Invalid command:  unused arguments: [u'foo', u'up']
osd tree {} {up|down|in|out [up|down|in|out...]} :  print OSD tree
Error EINVAL: invalid command
crush/CrushTreeDumper: allow children to filter results
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas requested a review from branch-predictor May 25, 2017

@branch-predictor

That's something we could use for sure, especially if it would let us filter by a 2+ flags at once. Besides that, few nits.

bool down = false;
bool up = false;
bool in = false;
bool out = false;

This comment has been minimized.

@branch-predictor

branch-predictor May 26, 2017

Member

save 3 bytes by putting these bools in a bitfields.

This comment has been minimized.

@liewegas

liewegas May 26, 2017

Member

this was less code that defining bitfields; not worth optimizing memory usage for a one-off thing like this.

: Parent(crush), osdmap(osdmap_) {
if (state.find("up") != std::string::npos)
up = true;
else if (state.find("down") != std::string::npos)

This comment has been minimized.

@branch-predictor

branch-predictor May 26, 2017

Member

It would be even more useful if it would be possible to combine flags (like "in+down"). Right now it accepts only one flag.

This comment has been minimized.

@liewegas

liewegas May 26, 2017

Member

you actually can do that.. that's why it's find() and not operator==. 'ceph osd tree out,down' or 'out+down' or even 'downandout' works (but sadly not 'downandoutinbeverlyhills' as 'in' is in there too)

This comment has been minimized.

@liewegas

liewegas May 26, 2017

Member

oh nevermind, the MOnCommands doesn't allow that. i'll fix that up.

OSDTreeFormattingDumper(const CrushWrapper *crush, const OSDMap *osdmap_,
const string& state)
: Parent(crush), osdmap(osdmap_) {
if (state.find("up") != std::string::npos)

This comment has been minimized.

@branch-predictor

branch-predictor May 26, 2017

Member

It's the same code as above, how about moving it to single, separate func that returns int containing flags, then passing that around?

@@ -619,12 +619,12 @@ int main(int argc, const char **argv)
if (tree) {
if (tree_formatter) {
tree_formatter->open_object_section("tree");
osdmap.print_tree(tree_formatter.get(), NULL);
osdmap.print_tree(tree_formatter.get(), NULL, string());

This comment has been minimized.

@branch-predictor

branch-predictor May 26, 2017

Member

That string construction wouldn't be necessary if it would accept int.

@liewegas

This comment has been minimized.

Member

liewegas commented May 26, 2017

ok, fixed everything and added tests, except for the common parent for the dumper class. That's not possible without some pretty multi-inheritance because the (immediate) parent classes are different and it would probably end up being more code than the two trivialish overrides.

@liewegas liewegas requested a review from tchaikov May 30, 2017

@branch-predictor

This comment has been minimized.

Member

branch-predictor commented May 30, 2017

LGTM

Reviewed-By: Piotr Dałek <piotr.dalek@corp.ovh.com>
@tchaikov

aside from the nits, lgtm.

@@ -2900,22 +2917,38 @@ class OSDTreePlainDumper : public CrushTreeDumper::Dumper<TextTable> {
private:
const OSDMap *osdmap;
unsigned filter;

This comment has been minimized.

@tchaikov

tchaikov May 30, 2017

Contributor

could make this a const, as we don't change it after initializing it.

@@ -76,8 +76,31 @@ namespace CrushTreeDumper {
clear();
}
virtual bool should_dump_leaf(int i) {

This comment has been minimized.

@tchaikov

tchaikov May 30, 2017

Contributor

could be a const.

virtual bool should_dump_leaf(int i) {
return true;
}
virtual bool should_dump_empty_bucket() {

This comment has been minimized.

@tchaikov

tchaikov May 30, 2017

Contributor

ditto.

mon: add up|down|in|out filters to 'osd tree'
For example, 'ceph osd tree down' will show *just* down OSDs and their
ancestors.  \o/

Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas merged commit 99d96c3 into ceph:master May 30, 2017

2 of 3 checks passed

default Build started sha1 is merged.
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details

@liewegas liewegas deleted the liewegas:wip-osd-tree branch May 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment