Skip to content

Commit

Permalink
crush: reset bucket->h.items[i] when removing tree item
Browse files Browse the repository at this point in the history
* crush: so we don't see the reference after the removing, this keeps
  check_item_loc() happy, and move_bucket() use check_item_loc() to see if
  the removed bucket disappears after the removal.
* test: also add unittest_crush_wrapper::CrushWrapper.insert_item

Fixes: http://tracker.ceph.com/issues/16525
Signed-off-by: Kefu Chai <kchai@redhat.com>
  • Loading branch information
tchaikov committed Jul 1, 2016
1 parent 85bb43e commit a7069c7
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/crush/builder.c
Expand Up @@ -1015,7 +1015,8 @@ int crush_remove_tree_bucket_item(struct crush_bucket_tree *bucket, int item)

if (bucket->h.items[i] != item)
continue;


bucket->h.items[i] = 0;
node = crush_calc_tree_node(i);
weight = bucket->node_weights[node];
bucket->node_weights[node] = 0;
Expand Down
42 changes: 42 additions & 0 deletions src/test/crush/CrushWrapper.cc
Expand Up @@ -708,6 +708,48 @@ TEST(CrushWrapper, insert_item) {
delete c;
}

TEST(CrushWrapper, remove_item) {
auto *c = new CrushWrapper;

const int ROOT_TYPE = 2;
c->set_type_name(ROOT_TYPE, "root");
const int HOST_TYPE = 1;
c->set_type_name(HOST_TYPE, "host");
const int OSD_TYPE = 0;
c->set_type_name(OSD_TYPE, "osd");

{
int root;
ASSERT_EQ(0, c->add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_RJENKINS1,
ROOT_TYPE, 0, NULL, NULL, &root));
c->set_item_name(root, "root0");
}

{
int host;
c->add_bucket(0, CRUSH_BUCKET_TREE, CRUSH_HASH_RJENKINS1,
HOST_TYPE, 0, NULL, NULL, &host);
c->set_item_name(host, "host0");
}

const int num_osd = 12;
{
map<string, string> loc = {{"root", "root0"},
{"host", "host0"}};
string name{"osd."};
for (int item = 0; item < num_osd; item++) {
ASSERT_EQ(0, c->insert_item(g_ceph_context, item, 1.0,
name + to_string(item), loc));
}
}
const int item_to_remove = num_osd / 2;
map<string, string> loc;
loc.insert(c->get_immediate_parent(item_to_remove));
ASSERT_EQ(0, c->remove_item(g_ceph_context, item_to_remove, true));
float weight;
EXPECT_FALSE(c->check_item_loc(g_ceph_context, item_to_remove, loc, &weight));
}

TEST(CrushWrapper, item_bucket_names) {
CrushWrapper *c = new CrushWrapper;
int index = 123;
Expand Down

0 comments on commit a7069c7

Please sign in to comment.