crush/CrushTester: check for overlapped rules

list rules if there are more than one of them can be chosen by the CRUSH
for a given ruleset, pool type and a pool size. this is not entirely a
buggy crush map, but it might be worthy to get the attention of the user
that multiple rules overlap with each other, and only the first one will
be used by the CRUSH.

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/src/crush/CrushTester.cc b/src/crush/CrushTester.cc
index d10aab93eda896a026ff30b2457f25320ee23ac3..adf147bd09316d0f20727a57b4272a38efaddea2 100644 (file)
--- a/src/crush/CrushTester.cc
+++ b/src/crush/CrushTester.cc
@@ -8,6 +8,8 @@
 #include <algorithm>
 #include <stdlib.h>
 #include <boost/lexical_cast.hpp>
+#include <boost/icl/interval_map.hpp>
+#include <boost/algorithm/string/join.hpp>
 #include <common/SubProcess.h>
 
 void CrushTester::set_device_weight(int dev, float f)
@@ -450,6 +452,51 @@ bool CrushTester::check_name_maps(unsigned max_id) const
   return true;
 }
 
+static string get_rule_name(CrushWrapper& crush, int rule)
+{
+  if (crush.get_rule_name(rule))
+    return crush.get_rule_name(rule);
+  else
+    return string("rule") + std::to_string(rule);
+}
+
+void CrushTester::check_overlapped_rules() const
+{
+  namespace icl = boost::icl;
+  typedef std::set<string> RuleNames;
+  typedef icl::interval_map<int, RuleNames> Rules;
+  // <ruleset, type> => interval_map<size, {names}>
+  typedef std::map<std::pair<int, int>, Rules> RuleSets;
+  using interval = icl::interval<int>;
+
+  // mimic the logic of crush_find_rule(), but it only return the first matched
+  // one, but I am collecting all of them by the overlapped sizes.
+  RuleSets rulesets;
+  for (int rule = 0; rule < crush.get_max_rules(); rule++) {
+    if (!crush.rule_exists(rule)) {
+      continue;
+    }
+    Rules& rules = rulesets[{crush.get_rule_mask_ruleset(rule),
+                            crush.get_rule_mask_type(rule)}];
+    rules += make_pair(interval::closed(crush.get_rule_mask_min_size(rule),
+                                       crush.get_rule_mask_max_size(rule)),
+                      RuleNames{get_rule_name(crush, rule)});
+  }
+  for (auto i : rulesets) {
+    auto ruleset_type = i.first;
+    const Rules& rules = i.second;
+    for (auto r : rules) {
+      const RuleNames& names = r.second;
+      // if there are more than one rules covering the same size range,
+      // print them out.
+      if (names.size() > 1) {
+       err << "overlapped rules in ruleset " << ruleset_type.first << ": "
+           << boost::join(names, ", ") << "\n";
+      }
+    }
+  }
+}
+
 int CrushTester::test()
 {
   if (min_rule < 0 || max_rule < 0) {

diff --git a/src/crush/CrushTester.h b/src/crush/CrushTester.h
index 2f1a2c60e753737ece6ac139d95cde94da9c1f47..bd625f3dda8869879604405b6ac434267a95d75a 100644 (file)
--- a/src/crush/CrushTester.h
+++ b/src/crush/CrushTester.h
@@ -347,6 +347,10 @@ public:
    *         large, true otherwise
    */
   bool check_name_maps(unsigned max_id = 0) const;
+  /**
+   * print out overlapped crush rules belonging to the same ruleset
+   */
+  void check_overlapped_rules() const;
   int test();
   int test_with_crushtool(const char *crushtool_cmd = "crushtool",
                          int max_id = -1,

diff --git a/src/test/cli/crushtool/check-overlapped-rules.crushmap.txt b/src/test/cli/crushtool/check-overlapped-rules.crushmap.txt
new file mode 100644 (file)
index 0000000..cd88863
--- /dev/null
+++ b/src/test/cli/crushtool/check-overlapped-rules.crushmap.txt
@@ -0,0 +1,89 @@
+device 0 device0
+device 1 device1
+device 2 device2
+device 3 device3
+device 4 device4
+
+type 0 osd
+type 1 host
+
+host host0 {
+       id -1
+       alg straw
+       hash 0
+       item device0 weight 1.000
+       item device1 weight 1.000
+       item device2 weight 1.000
+       item device3 weight 1.000
+       item device4 weight 1.000
+}
+
+rule rule-r0 {
+       ruleset 0
+       type replicated
+       min_size 1
+       max_size 3
+       step take host0
+       step choose firstn 0 type osd
+       step emit
+}
+
+rule rule-r1 {
+       ruleset 0
+       type replicated
+       min_size 1
+       max_size 1
+       step take host0
+       step choose firstn 0 type osd
+       step emit
+}
+
+rule rule-r2 {
+       ruleset 0
+       type replicated
+       min_size 1
+       max_size 2
+       step take host0
+       step choose firstn 0 type osd
+       step emit
+}
+
+rule rule-r3 {
+       ruleset 0
+       type replicated
+       min_size 2
+       max_size 3
+       step take host0
+       step choose indep 0 type osd
+       step emit
+}
+
+rule rule-r4 {
+       ruleset 0
+       type replicated
+       min_size 4
+       max_size 5
+       step take host0
+       step choose indep 0 type osd
+       step emit
+}
+
+rule rule-e0 {
+       ruleset 0
+       type erasure
+       min_size 1
+       max_size 10
+       step take host0
+       step choose indep 0 type osd
+       step emit
+}
+
+rule rule-e1 {
+       ruleset 1
+       type erasure
+       min_size 1
+       max_size 10
+       step take host0
+       step choose indep 0 type osd
+       step emit
+}

diff --git a/src/test/cli/crushtool/check-overlapped-rules.t b/src/test/cli/crushtool/check-overlapped-rules.t
new file mode 100644 (file)
index 0000000..acad57d
--- /dev/null
+++ b/src/test/cli/crushtool/check-overlapped-rules.t
@@ -0,0 +1,6 @@
+  $ crushtool -c "$TESTDIR/check-overlapped-rules.crushmap.txt" -o "$TESTDIR/check-overlapped-rules.crushmap"
+  $ crushtool -i "$TESTDIR/check-overlapped-rules.crushmap" --check
+  overlapped rules in ruleset 0: rule-r0, rule-r1, rule-r2
+  overlapped rules in ruleset 0: rule-r0, rule-r2, rule-r3
+  overlapped rules in ruleset 0: rule-r0, rule-r3
+  $ rm -f "$TESTDIR/check-overlapped-rules.crushmap"

diff --git a/src/tools/crushtool.cc b/src/tools/crushtool.cc
index 510d28de323cea77478be4d66622ab4325188213..d228d6fa46dc09968d313cd5e9ae8c918a3eca10 100644 (file)
--- a/src/tools/crushtool.cc
+++ b/src/tools/crushtool.cc
@@ -846,8 +846,11 @@ int main(int argc, const char **argv)
   }
 
   if (check) {
-    if (!tester.check_name_maps(max_id)) {
-      exit(1);
+    tester.check_overlapped_rules();
+    if (max_id >= 0) {
+      if (!tester.check_name_maps(max_id)) {
+       exit(1);
+      }
     }
   }