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

crush,mon: add weight-set introspection and manipulation commands #16326

Merged
merged 40 commits into from Jul 24, 2017

Conversation

Projects
None yet
3 participants
@liewegas
Member

liewegas commented Jul 13, 2017

  • 'osd crush tree' shows a column per weight-set (which are mapped to pools)
  • json version of above shows full weight-set, including positions (plain view only shows first weight)
  • mon commands to create, list, dump, remove, weight-sets
  • mon command to adjust a weight in a weight-set
  • many doc updates
  • crush updates so adjust weight-sets sanely when items are removed from the map (by maintaining the summation property for the tree within each weight-set).
$ ceph osd crush tree
ID WEIGHT  foo     TYPE NAME     
-1 4.00000         root default  
-2 4.00000 3.00000     host gnit 
 0 1.00000 0               osd.0 
 1 1.00000 1.00000         osd.1 
 2 1.00000 1.00000         osd.2 
 3 1.00000 1.00000         osd.3 
$ ceph osd crush tree -f json-pretty
[
    [
        {
            "id": -1,
            "name": "default",
            "type": "root",
            "type_id": 10,
            "children": [
                -2
            ]
        },
        {
            "id": -2,
            "name": "gnit",
            "type": "host",
            "type_id": 1,
            "pool_weights": {
                "foo": [
                    3.000000
                ]
            },
            "children": [
                3,
                2,
                1,
                0
            ]
        },
        {
            "id": 0,
            "name": "osd.0",
            "type": "osd",
            "type_id": 0,
            "crush_weight": 1.000000,
            "depth": 2,
            "pool_weights": {
                "foo": [
                    0.000000
                ]
            }
        },
        {
            "id": 1,
            "name": "osd.1",
            "type": "osd",
            "type_id": 0,
            "crush_weight": 1.000000,
            "depth": 2,
            "pool_weights": {
                "foo": [
                    1.000000
                ]
            }
        },
        {
            "id": 2,
            "name": "osd.2",
            "type": "osd",
            "type_id": 0,
            "crush_weight": 1.000000,
            "depth": 2,
            "pool_weights": {
                "foo": [
                    1.000000
                ]
            }
        },
        {
            "id": 3,
            "name": "osd.3",
            "type": "osd",
            "type_id": 0,
            "crush_weight": 1.000000,
            "depth": 2,
            "pool_weights": {
                "foo": [
                    1.000000
                ]
            }
        }
    ],
    []
]

@liewegas liewegas added this to the luminous milestone Jul 13, 2017

@liewegas liewegas changed the title from WIP crush,mon: add weight-set introspection and manipulation commands to crush,mon: add weight-set introspection and manipulation commands Jul 13, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 14, 2017

@idryomov another semantic change here, this time with choose_args. there is now a fallback to a choose_arg index of -1 if there isn't a pool-specific choose_arg set.

This still RFC/WIP. @jdurgin does this seem ok?

  • 'ceph osd crush weight-set create-compat' creates a compat weight-set. This acts like a fallback. If you create a per-pool weight-set, that works for that pool. Otherwise we try the compat/default one. If that doesnt' exist either, then we use the normal crush weights.
@liewegas

This comment has been minimized.

Member

liewegas commented Jul 14, 2017

Not really sure how to do naming, though. How shoudl teh compat weight-set show up in 'ls' output? I used "(compat)" for the osd crush tree output, but that's weird to type for, say, the pool argument in 'ceph osd crush weight-set reweight (compat) osd.1 .99'. I can do a separate reweight command (reweight-compat), but we still need to show something in 'ls' and 'dump'. '(compat)' is a legal pool name, by the way.

@liewegas liewegas changed the title from crush,mon: add weight-set introspection and manipulation commands to WIP crush,mon: add weight-set introspection and manipulation commands Jul 14, 2017

@liewegas liewegas changed the title from WIP crush,mon: add weight-set introspection and manipulation commands to crush,mon: add weight-set introspection and manipulation commands Jul 18, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 18, 2017

retest this please

@xiexingguo

This comment has been minimized.

Member

xiexingguo commented Jul 19, 2017

retest this please

@xiexingguo

This comment has been minimized.

Member

xiexingguo commented Jul 19, 2017

This is really a huge patch set. I haven't get all of these through.
But I think it is okay to merge this as long as the QA tests are happy and I'll do some follow-up validations later.

changed++;
}
}
if (touched) {

This comment has been minimized.

@xiexingguo

xiexingguo Jul 19, 2017

Member

if (changed) will be enough(hence we can drop touched here)

CrushWrapper newcrush;
_get_pending_crush(newcrush);
int64_t pool;
if (prefix == "osd crush weight-set create") {

This comment has been minimized.

@xiexingguo

xiexingguo Jul 19, 2017

Member

should be "osd crush weight-set rm"

@xiexingguo

This comment has been minimized.

Member

xiexingguo commented Jul 19, 2017

BTW, the documentation really helps!

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 19, 2017

fixed those 2 items and updated the test so that it should catch the 'rm' bug.

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 21, 2017

/a/sage-2017-07-21_03:24:52-rados-wip-sage-testing-distro-basic-smithi/1426654
/a/sage-2017-07-21_03:24:52-rados-wip-sage-testing-distro-basic-smithi/1426454

@@ -1990,9 +1990,12 @@ void CrushWrapper::decode(bufferlist::iterator& blp)
::decode(weight_set->weights[l], blp);
}
::decode(arg->ids_size, blp);
arg->ids = (__s32 *)calloc(arg->ids_size, sizeof(__s32));

This comment has been minimized.

@idryomov

idryomov Jul 21, 2017

Contributor

Should we do the same for weight_set calloc (slightly above) and get rid of arg->weight_set_size == 0 check in get_choose_arg_weights()? I wondered why it's there...

The kernel client checks both: https://github.com/ceph/ceph-client/blob/33e9c8dbfbcef8e4cda8e43a445e692ab7e0d8c0/net/ceph/osdmap.c#L222

This comment has been minimized.

@liewegas

liewegas Jul 21, 2017

Member

yeah, fixing!

liewegas added some commits Jul 13, 2017

crush/CrushWrapper: fix decoding of choose_args weight-set
Assert that ids_size matches the bucket while we are at it.

Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: has_non_straw2_buckets()
Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: whitespace cleanup in decode
Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: have_choose_args(index)
Signed-off-by: Sage Weil <sage@redhat.com>

liewegas added some commits Jul 14, 2017

crush/CrushWrapper: remove_item: weight down weight-sets
Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: detach_bucket uninline
Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: detach_bucket: weight down weight_sets
Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: link_bucket whitespace
Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: remove_item_under whitespace
Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: remove_item_under: weight down weight-sets
Signed-off-by: Sage Weil <sage@redhat.com>
mon/OSDMonitor: 'osd crush rule ls' plaintext output
More suitable for scripting.

Signed-off-by: Sage Weil <sage@redhat.com>
doc/rados/operations/crush-map: prune intro
Signed-off-by: Sage Weil <sage@redhat.com>
doc/rados/operations/crush-map: update crush location section
Fix various inaccuracies, simplify.

Signed-off-by: Sage Weil <sage@redhat.com>
doc/rados/operations/crush-map: warn off manual crush map edits
Signed-off-by: Sage Weil <sage@redhat.com>
doc/rados/operations/crush-map*: restructure and clean up
Signed-off-by: Sage Weil <sage@redhat.com>
doc/rados/operations/crush-map: document weight set commands
Signed-off-by: Sage Weil <sage@redhat.com>
doc/rados/operations/crush-map: creating rules
Signed-off-by: Sage Weil <sage@redhat.com>
qa/workunits/mon/crush_ops: require luminous clients for test
Signed-off-by: Sage Weil <sage@redhat.com>
qa/workunits/mon/crush_ops.sh: fix in-use rule removal test
Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: only allocate weight_set if non-empty
Signed-off-by: Sage Weil <sage@redhat.com>
weight_set->size, sizeof(__u32));
for (__u32 l = 0; l < weight_set->size; l++)
::decode(weight_set->weights[l], blp);
if (arg->weight_set_size) {

This comment has been minimized.

@idryomov

idryomov Jul 21, 2017

Contributor

What about arg->weight_set_size == 0 check in get_choose_arg_weights()? get_choose_arg_ids() doesn't have a corresponding arg->ids_size == 0 check, so we shouldn't need a weight_set one now either.

This comment has been minimized.

@liewegas

liewegas Jul 21, 2017

Member

Ah, yep.. that's now true. fixed!

crush: assume weight_set != null imples weight_set_size > 0
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas

This comment has been minimized.

Member

liewegas commented Jul 23, 2017

2017-07-23T02:37:55.096 INFO:tasks.ceph.osd.2.smithi065.stderr:/build/ceph-12.1.1-380-g5e8fa3e/src/osd/PrimaryLogPG.cc: 1845: FAILED assert(!cct->_conf->osd_debug_misdirected_ops)
2017-07-23T02:37:55.097 INFO:tasks.ceph.osd.2.smithi065.stderr:2017-07-23 02:37:55.090590 7f76325b2700 -1 osd.2 pg_epoch: 83 pg[3.7( v 83'1 (0'0,83'1] local-lis/les=80/83 n=1 ec=73/73 lis/c 80/80 les/c/f 83/83/0 73/80/73) [2,1] r=0 lpr=82 luod=0'0 lua=0'0 crt=83'1 lcod 0'0 mlcod 0'0 active+clean] do_op 3.7 does not contain 3:ee71ac71:::benchmark_data_smithi065_17586_object26:head pg_num 26 hash 8e358e77

/a/sage-2017-07-23_02:11:52-rados-wip-weight-set-distro-basic-smithi/1433151

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 24, 2017

ok that failure was unrelated, http://tracker.ceph.com/issues/20754

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 24, 2017

@idryomov updated patch look ok? If so, I think this is ready to merge!

@xiexingguo

This comment has been minimized.

Member

xiexingguo commented Jul 24, 2017

👍

@idryomov

This comment has been minimized.

Contributor

idryomov commented Jul 24, 2017

The parts I looked at look good.

@xiexingguo

This comment has been minimized.

Member

xiexingguo commented Jul 24, 2017

The parts I looked at look good.

+1 @liewegas Hope this can merge ASAP so I can continue with #16388

@liewegas liewegas merged commit fc8374b into ceph:master Jul 24, 2017

2 of 4 checks passed

make check make check failed
Details
make check (arm64) make check failed
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details

@liewegas liewegas deleted the liewegas:wip-weight-set branch Jul 24, 2017

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