From 3099684cf6ebea6c1e3fdad3988c90c8f37341db Mon Sep 17 00:00:00 2001 From: Loic Dachary Date: Tue, 9 May 2017 21:59:23 +0200 Subject: [PATCH 1/2] crush,osd: s/chooseargs/choose_args/ Signed-off-by: Loic Dachary --- src/crush/CrushWrapper.cc | 4 ++-- src/crush/CrushWrapper.h | 4 ++-- src/include/ceph_features.h | 4 ++-- src/osd/OSDMap.cc | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc index ee6db9bc629..04246a84225 100644 --- a/src/crush/CrushWrapper.cc +++ b/src/crush/CrushWrapper.cc @@ -105,12 +105,12 @@ bool CrushWrapper::is_v5_rule(unsigned ruleid) const return false; } -bool CrushWrapper::has_chooseargs() const +bool CrushWrapper::has_choose_args() const { return !choose_args.empty(); } -bool CrushWrapper::has_incompat_chooseargs() const +bool CrushWrapper::has_incompat_choose_args() const { // FIXME: if the chooseargs all have 1 position *and* do not remap IDs then // we can fabricate a compatible crush map for legacy clients by swapping the diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h index c47a5badc41..03cbc4ec7c9 100644 --- a/src/crush/CrushWrapper.h +++ b/src/crush/CrushWrapper.h @@ -316,8 +316,8 @@ public: bool has_v3_rules() const; bool has_v4_buckets() const; bool has_v5_rules() const; - bool has_chooseargs() const; // any chooseargs - bool has_incompat_chooseargs() const; // chooseargs that can't be made compat + bool has_choose_args() const; // any choose_args + bool has_incompat_choose_args() const; // choose_args that can't be made compat bool is_v2_rule(unsigned ruleid) const; bool is_v3_rule(unsigned ruleid) const; diff --git a/src/include/ceph_features.h b/src/include/ceph_features.h index 8b8933b7e01..9decdaff3e7 100755 --- a/src/include/ceph_features.h +++ b/src/include/ceph_features.h @@ -101,7 +101,7 @@ DEFINE_CEPH_FEATURE(21, 2, SERVER_LUMINOUS) DEFINE_CEPH_FEATURE(21, 2, RESEND_ON_SPLIT) // overlap DEFINE_CEPH_FEATURE(21, 2, RADOS_BACKOFF) // overlap DEFINE_CEPH_FEATURE(21, 2, OSDMAP_PG_UPMAP) // overlap -DEFINE_CEPH_FEATURE(21, 2, CRUSH_CHOOSEARGS) // overlap +DEFINE_CEPH_FEATURE(21, 2, CRUSH_CHOOSE_ARGS) // overlap DEFINE_CEPH_FEATURE_RETIRED(22, 1, BACKFILL_RESERVATION, JEWEL, LUMINOUS) DEFINE_CEPH_FEATURE(23, 1, MSG_AUTH) @@ -253,7 +253,7 @@ DEFINE_CEPH_FEATURE_DEPRECATED(63, 1, RESERVED_BROKEN, LUMINOUS) // client-facin CEPH_FEATURE_CRUSH_TUNABLES5 | \ CEPH_FEATURE_CRUSH_V2 | \ CEPH_FEATURE_CRUSH_V4 | \ - CEPH_FEATURE_CRUSH_CHOOSEARGS) + CEPH_FEATURE_CRUSH_CHOOSE_ARGS) /* * make sure we don't try to use the reserved features diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc index 446e2ca8c3f..d7bbff6b7de 100644 --- a/src/osd/OSDMap.cc +++ b/src/osd/OSDMap.cc @@ -1167,8 +1167,8 @@ uint64_t OSDMap::get_features(int entity_type, uint64_t *pmask) const features |= CEPH_FEATURE_CRUSH_V4; if (crush->has_nondefault_tunables5()) features |= CEPH_FEATURE_CRUSH_TUNABLES5; - if (crush->has_incompat_chooseargs()) - features |= CEPH_FEATURE_CRUSH_CHOOSEARGS; + if (crush->has_incompat_choose_args()) + features |= CEPH_FEATURE_CRUSH_CHOOSE_ARGS; mask |= CEPH_FEATURES_CRUSH; if (!pg_upmap.empty() || !pg_upmap_items.empty()) @@ -1250,7 +1250,7 @@ pair OSDMap::get_min_compat_client() const uint64_t f = get_features(CEPH_ENTITY_TYPE_CLIENT, nullptr); if (HAVE_FEATURE(f, OSDMAP_PG_UPMAP) || // v12.0.0-1733-g27d6f43 - HAVE_FEATURE(f, CRUSH_CHOOSEARGS)) { // v12.0.1-2172-gef1ef28 + HAVE_FEATURE(f, CRUSH_CHOOSE_ARGS)) { // v12.0.1-2172-gef1ef28 return make_pair("luminous", "12.2.0"); } if (HAVE_FEATURE(f, CRUSH_TUNABLES5)) { // v10.0.0-612-g043a737 From 7114581aaa057b1ccba21d96f30166f536275222 Mon Sep 17 00:00:00 2001 From: Loic Dachary Date: Mon, 8 May 2017 18:57:23 +0200 Subject: [PATCH 2/2] crush: encode can override weights with weight set Encode a "legacy" crushmap if (1) we're using weight sets, (2) the client is lacking features for the weight set, but (3) the weight set has a single position and no id remapping. Since these maps are only used by clients (not humans), then there is no need to preserve the original crush weights. We can just swap them for the real weights and the legacy clients will behave as expected. Fixes: http://tracker.ceph.com/issues/19836 Signed-off-by: Loic Dachary --- src/crush/CrushWrapper.cc | 41 +++++++++++++++++--- src/test/crush/CrushWrapper.cc | 69 ++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 6 deletions(-) diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc index 04246a84225..a0a54066907 100644 --- a/src/crush/CrushWrapper.cc +++ b/src/crush/CrushWrapper.cc @@ -112,10 +112,20 @@ bool CrushWrapper::has_choose_args() const bool CrushWrapper::has_incompat_choose_args() const { - // FIXME: if the chooseargs all have 1 position *and* do not remap IDs then - // we can fabricate a compatible crush map for legacy clients by swapping the - // choose_args weights in for the real weights. until then, - return has_chooseargs(); + if (choose_args.size() != 1) + return true; + crush_choose_arg_map arg_map = choose_args.begin()->second; + for (__u32 i = 0; i < arg_map.size; i++) { + crush_choose_arg *arg = &arg_map.args[i]; + if (arg->weight_set_size == 0 && + arg->ids_size == 0) + continue; + if (arg->weight_set_size != 1) + return true; + if (arg->ids_size != 0) + return true; + } + return false; } int CrushWrapper::split_id_class(int i, int *idout, int *classout) const @@ -1394,6 +1404,16 @@ void CrushWrapper::encode(bufferlist& bl, uint64_t features) const ::encode(crush->max_rules, bl); ::encode(crush->max_devices, bl); + bool encode_compat_choose_args = false; + crush_choose_arg_map arg_map; + memset(&arg_map, '\0', sizeof(arg_map)); + if (has_choose_args() && + !HAVE_FEATURE(features, CRUSH_CHOOSE_ARGS)) { + assert(!has_incompat_choose_args()); + encode_compat_choose_args = true; + arg_map = choose_args.begin()->second; + } + // buckets for (int i=0; imax_buckets; i++) { __u32 alg = 0; @@ -1437,8 +1457,17 @@ void CrushWrapper::encode(bufferlist& bl, uint64_t features) const break; case CRUSH_BUCKET_STRAW2: - for (unsigned j=0; jbuckets[i]->size; j++) { - ::encode((reinterpret_cast(crush->buckets[i]))->item_weights[j], bl); + { + __u32 *weights; + if (encode_compat_choose_args && + arg_map.args[i].weight_set_size > 0) { + weights = arg_map.args[i].weight_set[0].weights; + } else { + weights = (reinterpret_cast(crush->buckets[i]))->item_weights; + } + for (unsigned j=0; jbuckets[i]->size; j++) { + ::encode(weights[j], bl); + } } break; diff --git a/src/test/crush/CrushWrapper.cc b/src/test/crush/CrushWrapper.cc index 6e6fb05e5e7..42e67f006dd 100644 --- a/src/test/crush/CrushWrapper.cc +++ b/src/test/crush/CrushWrapper.cc @@ -961,6 +961,75 @@ TEST(CrushWrapper, distance) { ASSERT_EQ(1, c.get_common_ancestor_distance(g_ceph_context, 3, p)); } +TEST(CrushWrapper, choose_args_compat) { + CrushWrapper c; + c.create(); + c.set_type_name(1, "host"); + c.set_type_name(2, "rack"); + c.set_type_name(3, "root"); + + int weight = 12; + + map loc; + loc["host"] = "b1"; + loc["rack"] = "r11"; + loc["root"] = "default"; + int item = 1; + c.insert_item(g_ceph_context, item, weight, "osd.1", loc); + + loc["host"] = "b2"; + loc["rack"] = "r12"; + loc["root"] = "default"; + item = 2; + c.insert_item(g_ceph_context, item, weight, "osd.2", loc); + + assert(c.add_simple_ruleset("rule1", "r11", "host", "firstn", pg_pool_t::TYPE_ERASURE) >= 0); + + int id = c.get_item_id("b1"); + + __u32 weights = 666 * 0x10000; + crush_weight_set weight_set; + weight_set.size = 1; + weight_set.weights = &weights; + crush_choose_arg choose_args[c.get_max_buckets()]; + memset(choose_args, '\0', sizeof(crush_choose_arg) * c.get_max_buckets()); + choose_args[-1-id].ids_size = 0; + choose_args[-1-id].weight_set_size = 1; + choose_args[-1-id].weight_set = &weight_set; + crush_choose_arg_map arg_map; + arg_map.size = c.get_max_buckets(); + arg_map.args = choose_args; + + uint64_t features = CEPH_FEATURE_CRUSH_TUNABLES5|CEPH_FEATURE_INCARNATION_2; + + // if the client is capable, encode choose_args + { + c.choose_args[0] = arg_map; + bufferlist bl; + c.encode(bl, features|CEPH_FEATURE_CRUSH_CHOOSE_ARGS); + bufferlist::iterator i(bl.begin()); + CrushWrapper c_new; + c_new.decode(i); + ASSERT_EQ(1u, c_new.choose_args.size()); + ASSERT_EQ(1u, c_new.choose_args[0].args[-1-id].weight_set_size); + ASSERT_EQ(weights, c_new.choose_args[0].args[-1-id].weight_set[0].weights[0]); + ASSERT_EQ(weight, c_new.get_bucket_item_weightf(id, 0)); + } + + // if the client is not compatible, copy choose_arg in the weights + { + c.choose_args[0] = arg_map; + bufferlist bl; + c.encode(bl, features); + c.choose_args.clear(); + bufferlist::iterator i(bl.begin()); + CrushWrapper c_new; + c_new.decode(i); + ASSERT_EQ(0u, c_new.choose_args.size()); + ASSERT_EQ((int)weights, c_new.get_bucket_item_weight(id, 0)); + } +} + TEST(CrushWrapper, remove_unused_root) { CrushWrapper c; c.create();