crush: fix distortion of straw scalers by 0-weight items

The presence of a 0-weight item in a straw bucket should have no effect
on the placement of other items.  Add a test validating that and fix
crush_calc_straw() to fix the distortion.

Note that this effects the *precalculation* of the straw bucket inputs and
does not effect the actually mapping process given a compiled or encoded
CRUSH map, and only when straw_calc_version == 1 (i.e., the admin opted in
to the new behavior).

Backport: giant, firefly
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 85498bc8f62ca56506b33f3c5ec4fc4b111ed73d)

diff --git a/src/crush/builder.c b/src/crush/builder.c
index 64137dab4f18e03359f82097868484d01309281a..93bd6c417b126cd6b19758e9b8a4dd01efe328db 100644 (file)
--- a/src/crush/builder.c
+++ b/src/crush/builder.c
@@ -488,6 +488,7 @@ int crush_calc_straw(struct crush_map *map, struct crush_bucket_straw *bucket)
                        if (weights[reverse[i]] == 0) {
                                bucket->straws[reverse[i]] = 0;
                                i++;
+                               numleft--;
                                continue;
                        }
 

diff --git a/src/test/crush/TestCrushWrapper.cc b/src/test/crush/TestCrushWrapper.cc
index 5eefaf15ef955d2440c7572482b7ba1bdb688b80..f4ae2a0569d2fd365a07e792f5f77423406588c9 100644 (file)
--- a/src/test/crush/TestCrushWrapper.cc
+++ b/src/test/crush/TestCrushWrapper.cc
@@ -67,6 +67,58 @@ TEST(CrushWrapper, get_immediate_parent) {
   delete c;
 }
 
+TEST(CrushWrapper, straw_zero) {
+  // zero weight items should have no effect on placement.
+
+  CrushWrapper *c = new CrushWrapper;
+  const int ROOT_TYPE = 1;
+  c->set_type_name(ROOT_TYPE, "root");
+  const int OSD_TYPE = 0;
+  c->set_type_name(OSD_TYPE, "osd");
+
+  int n = 5;
+  int items[n], weights[n];
+  for (int i=0; i <n; ++i) {
+    items[i] = i;
+    weights[i] = 0x10000 * (n-i-1);
+  }
+
+  c->set_max_devices(n);
+
+  string root_name0("root0");
+  int root0;
+  EXPECT_EQ(0, c->add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_RJENKINS1,
+                            ROOT_TYPE, n, items, weights, &root0));
+  EXPECT_EQ(0, c->set_item_name(root0, root_name0));
+
+  string name0("rule0");
+  int ruleset0 = c->add_simple_ruleset(name0, root_name0, "osd",
+                                      "firstn", pg_pool_t::TYPE_REPLICATED);
+  EXPECT_EQ(0, ruleset0);
+
+  string root_name1("root1");
+  int root1;
+  EXPECT_EQ(0, c->add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_RJENKINS1,
+                            ROOT_TYPE, n-1, items, weights, &root1));
+  EXPECT_EQ(0, c->set_item_name(root1, root_name1));
+
+  string name1("rule1");
+  int ruleset1 = c->add_simple_ruleset(name1, root_name1, "osd",
+                                      "firstn", pg_pool_t::TYPE_REPLICATED);
+  EXPECT_EQ(1, ruleset1);
+
+  vector<unsigned> reweight(n, 0x10000);
+  for (int i=0; i<10000; ++i) {
+    vector<int> out0, out1;
+    c->do_rule(ruleset0, i, out0, 1, reweight);
+    ASSERT_EQ(1u, out0.size());
+    c->do_rule(ruleset1, i, out1, 1, reweight);
+    ASSERT_EQ(1u, out1.size());
+    ASSERT_EQ(out0[0], out1[0]);
+    //cout << i << "\t" << out0 << "\t" << out1 << std::endl;
+  }
+}
+
 TEST(CrushWrapper, move_bucket) {
   CrushWrapper *c = new CrushWrapper;