diff --git a/persistent-data/data-structures/btree.h b/persistent-data/data-structures/btree.h index be64f24..652aee6 100644 --- a/persistent-data/data-structures/btree.h +++ b/persistent-data/data-structures/btree.h @@ -26,6 +26,7 @@ #include #include #include +#include //---------------------------------------------------------------- @@ -266,9 +267,36 @@ namespace persistent_data { // Used when visiting the nodes that make up a btree. struct node_location { - unsigned level; + node_location() + : depth(0) { + } + + void inc_depth() { + depth++; + } + + void push_key(uint64_t k) { + path.push_back(k); + depth = 0; + } + + bool is_sub_root() const { + return depth == 0; // && path.size(); + } + + unsigned level() const { + return path.size(); + } + + // Keys used to access this sub tree + std::deque path; + + // in this sub tree unsigned depth; - bool sub_root; + + // This is the key from the parent node to this + // node. If this node is a root then there will be + // no parent, and hence no key. boost::optional key; }; } diff --git a/persistent-data/data-structures/btree.tcc b/persistent-data/data-structures/btree.tcc index e91670f..e5b4e9f 100644 --- a/persistent-data/data-structures/btree.tcc +++ b/persistent-data/data-structures/btree.tcc @@ -746,11 +746,6 @@ btree::visit_depth_first(visitor &v) const { node_location loc; - loc.level = 0; - loc.depth = 0; - loc.sub_root = true; - loc.key = boost::optional(); - walk_tree(v, loc, root_); v.visit_complete(); } @@ -785,29 +780,28 @@ btree::walk_tree_internal(visitor &v, read_ref blk = tm_->read_lock(b, validator_); internal_node o = to_node(blk); + + // FIXME: use a switch statement if (o.get_type() == INTERNAL) { if (v.visit_internal(loc, o)) for (unsigned i = 0; i < o.get_nr_entries(); i++) { node_location loc2(loc); - loc2.depth = loc.depth + 1; - loc2.sub_root = false; - loc2.key = boost::optional(o.key_at(i)); + loc2.inc_depth(); + loc2.key = o.key_at(i); walk_tree(v, loc2, o.value_at(i)); } - } else if (loc.level < Levels - 1) { + } else if (loc.path.size() < Levels - 1) { if (v.visit_internal_leaf(loc, o)) for (unsigned i = 0; i < o.get_nr_entries(); i++) { node_location loc2(loc); - loc2.level = loc.level + 1; - loc2.depth = loc.depth + 1; - loc2.sub_root = true; - loc2.key = boost::optional(o.key_at(i)); + loc2.push_key(o.key_at(i)); + loc2.key = optional(); - walk_tree(v, loc, o.value_at(i)); + walk_tree(v, loc2, o.value_at(i)); } } else { diff --git a/persistent-data/data-structures/btree_checker.h b/persistent-data/data-structures/btree_checker.h index 9055c1e..da2cff4 100644 --- a/persistent-data/data-structures/btree_checker.h +++ b/persistent-data/data-structures/btree_checker.h @@ -95,11 +95,11 @@ namespace persistent_data { if (!already_visited(n) && check_block_nr(n) && check_max_entries(n) && - check_nr_entries(n, loc.sub_root) && + check_nr_entries(n, loc.is_sub_root()) && check_ordered_keys(n) && - check_parent_key(loc.sub_root ? optional() : loc.key, n)) { - if (loc.sub_root) - new_root(loc.level); + check_parent_key(loc.is_sub_root() ? optional() : loc.key, n)) { + if (loc.is_sub_root()) + new_root(loc.level()); return true; } @@ -113,13 +113,13 @@ namespace persistent_data { if (!already_visited(n) && check_block_nr(n) && check_max_entries(n) && - check_nr_entries(n, loc.sub_root) && + check_nr_entries(n, loc.is_sub_root()) && check_ordered_keys(n) && - check_parent_key(loc.sub_root ? optional() : loc.key, n)) { - if (loc.sub_root) - new_root(loc.level); + check_parent_key(loc.is_sub_root() ? optional() : loc.key, n)) { + if (loc.is_sub_root()) + new_root(loc.level()); - return check_leaf_key(loc.level, n); + return check_leaf_key(loc.level(), n); } return false; diff --git a/persistent-data/data-structures/btree_damage_visitor.h b/persistent-data/data-structures/btree_damage_visitor.h index 3ca43da..c4c1367 100644 --- a/persistent-data/data-structures/btree_damage_visitor.h +++ b/persistent-data/data-structures/btree_damage_visitor.h @@ -38,6 +38,8 @@ namespace persistent_data { return out; } + // Tracks damage in a single level btree. Use multiple + // trackers if you have a multilayer tree. class damage_tracker { public: damage_tracker() @@ -190,11 +192,11 @@ namespace persistent_data { if (!already_visited(n) && check_block_nr(n) && check_max_entries(n) && - check_nr_entries(n, loc.sub_root) && + check_nr_entries(n, loc.is_sub_root()) && check_ordered_keys(n) && - check_parent_key(loc.sub_root ? optional() : loc.key, n)) { - if (loc.sub_root) - new_root(loc.level); + check_parent_key(loc.is_sub_root() ? optional() : loc.key, n)) { + if (loc.is_sub_root()) + new_root(loc.level()); good_internal(n.key_at(0)); return true; @@ -209,13 +211,13 @@ namespace persistent_data { if (!already_visited(n) && check_block_nr(n) && check_max_entries(n) && - check_nr_entries(n, loc.sub_root) && + check_nr_entries(n, loc.is_sub_root()) && check_ordered_keys(n) && - check_parent_key(loc.sub_root ? optional() : loc.key, n)) { - if (loc.sub_root) - new_root(loc.level); + check_parent_key(loc.is_sub_root() ? optional() : loc.key, n)) { + if (loc.is_sub_root()) + new_root(loc.level()); - bool r = check_leaf_key(loc.level, n); + bool r = check_leaf_key(loc.level(), n); if (r && n.get_nr_entries() > 0) good_leaf(n.key_at(0), n.key_at(n.get_nr_entries() - 1) + 1); @@ -291,7 +293,7 @@ namespace persistent_data { block_address min = n.get_max_entries() / 3; if (!is_root && (n.get_nr_entries() < min)) { ostringstream out; - out << "too few entries in btree: " + out << "too few entries in btree_node: " << n.get_nr_entries() << ", expected at least " << min diff --git a/thin-provisioning/metadata_dumper.cc b/thin-provisioning/metadata_dumper.cc index c713eec..1848ab0 100644 --- a/thin-provisioning/metadata_dumper.cc +++ b/thin-provisioning/metadata_dumper.cc @@ -50,7 +50,7 @@ namespace { return false; } - return (loc.sub_root && loc.key) ? (*loc.key == dev_id_) : true; + return (loc.is_sub_root() && loc.key) ? (*loc.key == dev_id_) : true; } bool visit_internal_leaf(node_location const &loc, diff --git a/unit-tests/btree_damage_visitor_t.cc b/unit-tests/btree_damage_visitor_t.cc index 710a359..45fb43f 100644 --- a/unit-tests/btree_damage_visitor_t.cc +++ b/unit-tests/btree_damage_visitor_t.cc @@ -196,7 +196,7 @@ namespace { ni->leaf = leaf; ni->depth = loc.depth; - ni->level = loc.level; + ni->level = loc.level(); ni->b = n.get_location(); if (n.get_nr_entries()) @@ -235,30 +235,80 @@ namespace { MOCK_METHOD1(visit, void(btree_detail::damage const &)); }; - class BTreeDamageVisitorTests : public Test { + class DamageTests : public Test { public: - BTreeDamageVisitorTests() + DamageTests() : bm_(create_bm(NR_BLOCKS)), sm_(setup_core_map()), - tm_(new transaction_manager(bm_, sm_)), - tree_(new btree<1, thing_traits>(tm_, rc_)) { + tm_(new transaction_manager(bm_, sm_)) { } - space_map::ptr setup_core_map() { - space_map::ptr sm(new core_map(NR_BLOCKS)); - sm->inc(SUPERBLOCK); - return sm; - } + virtual ~DamageTests() {} void tree_complete() { commit(); discover_layout(); } + void run() { + commit(); + run_(); + } + void trash_block(block_address b) { ::test::zero_block(bm_, b); } + //-------------------------------- + + void expect_no_values() { + EXPECT_CALL(value_visitor_, visit(_)).Times(0); + } + + void expect_no_damage() { + EXPECT_CALL(damage_visitor_, visit(_)).Times(0); + } + + void expect_damage(unsigned level, range keys) { + EXPECT_CALL(damage_visitor_, visit(Eq(damage(level, keys, "foo")))).Times(1); + } + + //-------------------------------- + + with_temp_directory dir_; + block_manager<>::ptr bm_; + space_map::ptr sm_; + transaction_manager::ptr tm_; + thing_traits::ref_counter rc_; + + optional layout_; + + value_visitor_mock value_visitor_; + damage_visitor_mock damage_visitor_; + + private: + space_map::ptr setup_core_map() { + space_map::ptr sm(new core_map(NR_BLOCKS)); + sm->inc(SUPERBLOCK); + return sm; + } + + void commit() { + block_manager<>::write_ref superblock(bm_->superblock(SUPERBLOCK)); + } + + virtual void discover_layout() = 0; + virtual void run_() = 0; + }; + + //-------------------------------- + + class BTreeDamageVisitorTests : public DamageTests { + public: + BTreeDamageVisitorTests() + : tree_(new btree<1, thing_traits>(tm_, rc_)) { + } + void insert_values(unsigned nr) { for (unsigned i = 0; i < nr; i++) { uint64_t key[1] = {i}; @@ -268,10 +318,6 @@ namespace { } } - void expect_no_values() { - EXPECT_CALL(value_visitor_, visit(_)).Times(0); - } - void expect_value_range(uint64_t begin, uint64_t end) { while (begin < end) { EXPECT_CALL(value_visitor_, visit(Eq(thing(begin, begin + 1234)))).Times(1); @@ -287,49 +333,76 @@ namespace { EXPECT_CALL(value_visitor_, visit(Eq(thing(n, n + 1234)))).Times(1); } - void expect_no_damage() { - EXPECT_CALL(damage_visitor_, visit(_)).Times(0); + btree<1, thing_traits>::ptr tree_; + + private: + virtual void discover_layout() { + btree_layout_visitor<1, thing_traits> visitor; + tree_->visit_depth_first(visitor); + layout_ = visitor.get_layout(); } - void expect_damage(unsigned level, range keys) { - EXPECT_CALL(damage_visitor_, visit(Eq(damage(level, keys, "foo")))).Times(1); - } - - void run() { - // We must commit before we do the test to ensure - // all the block numbers and checksums are written - // to the btree nodes. - commit(); - + virtual void run_() { block_counter counter; btree_damage_visitor visitor(counter, value_visitor_, damage_visitor_); tree_->visit_depth_first(visitor); } + }; - with_temp_directory dir_; - block_manager<>::ptr bm_; - space_map::ptr sm_; - transaction_manager::ptr tm_; + //-------------------------------- - thing_traits::ref_counter rc_; - btree<1, thing_traits>::ptr tree_; - - optional layout_; - - value_visitor_mock value_visitor_; - damage_visitor_mock damage_visitor_; - - private: - void commit() { - block_manager<>::write_ref superblock(bm_->superblock(SUPERBLOCK)); + // 2 level btree + class BTreeDamageVisitor2Tests : public DamageTests { + public: + BTreeDamageVisitor2Tests() + : tree_(new btree<2, thing_traits>(tm_, rc_)) { } - void discover_layout() { - btree_layout_visitor<1, thing_traits> visitor; + void insert_values(unsigned nr_sub_trees, unsigned nr_values) { + for (unsigned i = 0; i < nr_sub_trees; i++) + insert_sub_tree_values(i, nr_values); + } + + void insert_sub_tree_values(unsigned sub_tree, unsigned nr_values) { + for (unsigned i = 0; i < nr_values; i++) { + uint64_t key[2] = {sub_tree, i}; + thing value(key_to_value(key)); + tree_->insert(key, value); + } + } + + void expect_values(unsigned nr_sub_trees, unsigned nr_values) { + for (unsigned i = 0; i < nr_sub_trees; i++) + expect_sub_tree_values(i, nr_values); + } + + void expect_sub_tree_values(unsigned sub_tree, unsigned nr_values) { + for (unsigned i = 0; i < nr_values; i++) { + uint64_t key[2] = {sub_tree, i}; + EXPECT_CALL(value_visitor_, visit(Eq(key_to_value(key)))); + } + } + + btree<2, thing_traits>::ptr tree_; + + private: + thing key_to_value(uint64_t key[2]) { + return thing(key[0] * 1000000 + key[1], key[1] + 1234); + } + + virtual void discover_layout() { + btree_layout_visitor<2, thing_traits> visitor; tree_->visit_depth_first(visitor); layout_ = visitor.get_layout(); } + + virtual void run_() { + block_counter counter; + btree_damage_visitor + visitor(counter, value_visitor_, damage_visitor_); + tree_->visit_depth_first(visitor); + } }; } @@ -454,3 +527,33 @@ TEST_F(BTreeDamageVisitorTests, damaged_internal) } //---------------------------------------------------------------- + +TEST_F(BTreeDamageVisitor2Tests, empty_tree) +{ + expect_no_damage(); + expect_no_values(); + + run(); +} + +TEST_F(BTreeDamageVisitor2Tests, tree_with_a_trashed_root) +{ + trash_block(tree_->get_root()); + + expect_no_values(); + expect_damage(0, range(0ull)); + + run(); +} + +TEST_F(BTreeDamageVisitor2Tests, populated_tree_with_no_damage) +{ + insert_values(10, 10); + + expect_values(10, 10); + expect_no_damage(); + + run(); +} + +//---------------------------------------------------------------- diff --git a/unit-tests/metadata_checker_t.cc b/unit-tests/metadata_checker_t.cc index e9fa9e6..988ec1a 100644 --- a/unit-tests/metadata_checker_t.cc +++ b/unit-tests/metadata_checker_t.cc @@ -115,7 +115,7 @@ namespace { ni->leaf = leaf; ni->depth = loc.depth; - ni->level = loc.level; + ni->level = loc.level(); ni->b = n.get_location(); if (n.get_nr_entries())