Lots of work on the btree_damage_visitor

persistent-data/data-structures/btree_damage_visitor.h

@@ -31,6 +31,13 @@ namespace persistent_data {
 			std::string desc_;
 		};
 
+		inline std::ostream &operator <<(std::ostream &out, damage const &d) {
+			out << "btree damage[level = " << d.level_
+			    << ", effected_keys = " << d.lost_keys_
+			    << ", \"" << d.desc_ << "\"]";
+			return out;
+		}
+
 		class damage_tracker {
 		public:
 			damage_tracker()
@@ -123,6 +130,8 @@ namespace persistent_data {
 	class btree_damage_visitor : public btree<Levels, ValueTraits>::visitor {
 	public:
 		typedef btree_detail::node_location node_location;
+		typedef range<block_address> range64;
+		typedef boost::optional<range64> maybe_range64;
 
 		btree_damage_visitor(block_counter &counter,
 				     ValueVisitor &value_visitor,
@@ -130,8 +139,7 @@ namespace persistent_data {
 			: counter_(counter),
 			  avoid_repeated_visits_(true),
 			  value_visitor_(value_visitor),
-			  damage_visitor_(damage_visitor),
+			  damage_visitor_(damage_visitor) {
-			  key_end_(0) {
 		}
 
 		bool visit_internal(node_location const &loc,
@@ -158,6 +166,10 @@ namespace persistent_data {
 			return true;
 		}
 
+		void visit_complete() {
+			end_walk();
+		}
+
 		typedef typename btree<Levels, ValueTraits>::visitor::error_outcome error_outcome;
 
 		error_outcome error_accessing_node(node_location const &l, block_address b,
@@ -184,8 +196,7 @@ namespace persistent_data {
 				if (loc.sub_root)
 					new_root(loc.level);
 
-				update_key_end(n.key_at(n.get_nr_entries() - 1) + 1ull);
+				good_internal(n.key_at(0));
-
 				return true;
 			}
 
@@ -205,8 +216,8 @@ namespace persistent_data {
 					new_root(loc.level);
 
 				bool r = check_leaf_key(loc.level, n);
-				if (r)
+				if (r && n.get_nr_entries() > 0)
-					update_key_end(n.key_at(n.get_nr_entries() - 1) + 1ull);
+					good_leaf(n.key_at(0), n.key_at(n.get_nr_entries() - 1) + 1);
 
 				return r;
 			}
@@ -239,7 +250,7 @@ namespace persistent_data {
 				    << n.get_location()
 				    << ", claims " << n.get_block_nr();
 
-				report_damage(n, out.str());
+				report_damage(out.str());
 				return false;
 			}
 
@@ -252,14 +263,14 @@ namespace persistent_data {
 			if (elt_size * n.get_max_entries() + sizeof(node_header) > MD_BLOCK_SIZE) {
 				std::ostringstream out;
 				out << "max entries too large: " << n.get_max_entries();
-				report_damage(n, out.str());
+				report_damage(out.str());
 				return false;
 			}
 
 			if (n.get_max_entries() % 3) {
 				std::ostringstream out;
 				out << "max entries is not divisible by 3: " << n.get_max_entries();
-				report_damage(n, out.str());
+				report_damage(out.str());
 				return false;
 			}
 
@@ -273,7 +284,7 @@ namespace persistent_data {
 				out << "bad nr_entries: "
 				    << n.get_nr_entries() << " < "
 				    << n.get_max_entries();
-				report_damage(n, out.str());
+				report_damage(out.str());
 				return false;
 			}
 
@@ -285,7 +296,7 @@ namespace persistent_data {
 				    << ", expected at least "
 				    << min
 				    << "(max_entries = " << n.get_max_entries() << ")";
-				report_damage(n, out.str());
+				report_damage(out.str());
 				return false;
 			}
 
@@ -306,7 +317,7 @@ namespace persistent_data {
 				if (k <= last_key) {
 					ostringstream out;
 					out << "keys are out of order, " << k << " <= " << last_key;
-					report_damage(n, out.str());
+					report_damage(out.str());
 					return false;
 				}
 				last_key = k;
@@ -324,7 +335,7 @@ namespace persistent_data {
 				ostringstream out;
 				out << "parent key mismatch: parent was " << *key
 				    << ", but lowest in node was " << n.key_at(0);
-				report_damage(n, out.str());
+				report_damage(out.str());
 				return false;
 			}
 
@@ -340,7 +351,7 @@ namespace persistent_data {
 				ostringstream out;
 				out << "the last key of the previous leaf was " << *last_leaf_key_[level]
 				    << " and the first key of this leaf is " << n.key_at(0);
-				report_damage(n, out.str());
+				report_damage(out.str());
 				return false;
 			}
 
@@ -354,31 +365,56 @@ namespace persistent_data {
 			last_leaf_key_[level] = boost::optional<uint64_t>();
 		}
 
-		void update_key_end(uint64_t end) {
-			key_end_ = max(end, key_end_);
-		}
-
-		template <typename node>
-		void report_damage(node const &n, std::string const &desc) {
-			range<uint64_t> lost_keys(key_end_);
-			damage d(0, lost_keys, desc);
-
-			cerr << "damage: keys = " << lost_keys << " " << desc << endl;
-			damage_visitor_.visit(d);
-		}
-
 		//--------------------------------
 
 		// damage tracking
 
 		void report_damage(std::string const &desc) {
-			range<uint64_t> lost_keys(key_end_);
+			damage_reasons_.push_back(desc);
-			damage d(0, lost_keys, desc);
+			dt_.bad_node();
+		}
 
-			cerr << "damage: keys = " << lost_keys << " " << desc << endl;
+		void good_internal(block_address b) {
+			maybe_range64 mr = dt_.good_internal(b);
+			if (mr)
+				issue_damage(*mr);
+		}
+
+		void good_leaf(block_address b, block_address e) {
+			maybe_range64 mr = dt_.good_leaf(b, e);
+
+			if (mr)
+				issue_damage(*mr);
+		}
+
+		void end_walk() {
+			maybe_range64 mr = dt_.end();
+			if (mr)
+				issue_damage(*mr);
+		}
+
+		void issue_damage(range64 const &r) {
+			// FIXME: we don't really know what level
+			// the damage is coming from
+			damage d(0, r, build_damage_desc());
+			clear_damage_desc();
 			damage_visitor_.visit(d);
 		}
 
+		std::string build_damage_desc() const {
+			std::string r;
+
+			std::list<std::string>::const_iterator it, end = damage_reasons_.end();
+			for (it = damage_reasons_.begin(); it != end; ++it)
+				r += *it;
+
+			return r;
+		}
+
+		void clear_damage_desc() {
+			damage_reasons_.clear();
+		}
+
 		//--------------------------------
 
 		block_counter &counter_;
@@ -390,7 +426,8 @@ namespace persistent_data {
 		std::set<block_address> seen_;
 		boost::optional<uint64_t> last_leaf_key_[Levels];
 
-		uint64_t key_end_;
+		damage_tracker dt_;
+		std::list<std::string> damage_reasons_;
 	};
 }
 

unit-tests/Makefile.in

@@ -50,6 +50,7 @@ TEST_SOURCE=\
 	unit-tests/btree_damage_visitor_t.cc \
 	unit-tests/buffer_t.cc \
 	unit-tests/cache_t.cc \
+	unit-tests/damage_tracker_t.cc \
 	unit-tests/endian_t.cc \
 	unit-tests/metadata_checker_t.cc \
 	unit-tests/space_map_t.cc \

unit-tests/btree_damage_visitor_t.cc

@@ -20,21 +20,21 @@ namespace {
 	block_address const SUPERBLOCK = 0;
 
 	struct thing {
-		thing(/* uint32_t x_ = 0, */ uint64_t y_ = 0)
+		thing(uint32_t x_ = 0, uint64_t y_ = 0)
-			: /* x(x_), */
+			: x(x_),
 			  y(y_) {
 		}
 
 		bool operator ==(thing const &rhs) const {
-			return /* (x == rhs.x) && */ (y == rhs.y);
+			return (x == rhs.x) && (y == rhs.y);
 		}
 
-		//uint32_t x;
+		uint32_t x;
 		uint64_t y;
 	};
 
 	struct thing_disk {
-		//le32 x;
+		le32 x;
 		le64 y;
 	};
 
@@ -44,16 +44,101 @@ namespace {
 		typedef persistent_data::no_op_ref_counter<value_type> ref_counter;
 
 		static void unpack(thing_disk const &disk, thing &value) {
-			//	value.x = to_cpu<uint32_t>(disk.x);
+			value.x = to_cpu<uint32_t>(disk.x);
 			value.y = to_cpu<uint64_t>(disk.y);
 		}
 
 		static void pack(thing const &value, thing_disk &disk) {
-			//	disk.x = to_disk<le32>(value.x);
+			disk.x = to_disk<le32>(value.x);
 			disk.y = to_disk<le64>(value.y);
 		}
 	};
 
+	template <uint32_t Levels, typename ValueTraits>
+	class btree_layout : public btree<Levels, ValueTraits>::visitor {
+	public:
+		struct node_info {
+			typedef boost::shared_ptr<node_info> ptr;
+
+			bool leaf;
+			unsigned depth;
+			unsigned level;
+			block_address b;
+			range<uint64_t> keys;
+		};
+
+		typedef btree_detail::node_location node_location;
+		typedef btree<Levels, ValueTraits> tree;
+		typedef boost::shared_ptr<btree_layout> ptr;
+
+		virtual bool visit_internal(node_location const &loc,
+					    typename tree::internal_node const &n) {
+			record_node(false, loc, n);
+			return true;
+		}
+
+		virtual bool visit_internal_leaf(node_location const &loc,
+						 typename tree::internal_node const &n) {
+			record_node(true, loc, n);
+			return true;
+		}
+
+
+		virtual bool visit_leaf(node_location const &loc,
+					typename tree::leaf_node const &n) {
+			record_node(true, loc, n);
+			return true;
+		}
+
+		virtual void visit_complete() {
+		}
+
+		vector<typename node_info::ptr> const &get_nodes() const {
+			return nodes_;
+		}
+
+		node_info const &random_node() const {
+			if (nodes_.empty())
+				throw runtime_error("no nodes in btree");
+
+			return *nodes_[::random() % nodes_.size()];
+		}
+
+	private:
+		// We rely on the visit order being depth first, lowest to highest.
+		template <typename N>
+		void record_node(bool leaf, node_location const &loc, N const &n) {
+			typename node_info::ptr ni(new node_info);
+
+			ni->leaf = leaf;
+			ni->depth = loc.depth;
+			ni->level = loc.level;
+			ni->b = n.get_location();
+
+			if (n.get_nr_entries())
+				ni->keys = range<uint64_t>(n.key_at(0));
+			else {
+				if (loc.key)
+					ni->keys = range<uint64_t>(*loc.key);
+				else
+					ni->keys = range<uint64_t>();
+			}
+
+			if (last_node_at_depth_.size() > loc.depth) {
+				typename node_info::ptr &last = last_node_at_depth_[loc.depth];
+
+				last->keys.end_ = ni->keys.begin_;
+				last_node_at_depth_[loc.depth] = ni;
+			} else
+				last_node_at_depth_.push_back(ni);
+
+			nodes_.push_back(ni);
+		}
+
+		vector<typename node_info::ptr> nodes_;
+		vector<typename node_info::ptr> last_node_at_depth_;
+	};
+
 	class value_visitor_mock {
 	public:
 		MOCK_METHOD1(visit, void(thing const &));
@@ -90,7 +175,7 @@ namespace {
 		void insert_values(unsigned nr) {
 			for (unsigned i = 0; i < nr; i++) {
 				uint64_t key[1] = {i};
-				thing value(i /*, i + 1234 */);
+				thing value(i, i + 1234);
 
 				tree_->insert(key, value);
 			}
@@ -100,13 +185,19 @@ namespace {
 			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);
+				begin++;
+			}
+		}
+
 		void expect_nr_values(unsigned nr) {
-			for (unsigned i = 0; i < nr; i++)
+			expect_value_range(0, nr);
-				EXPECT_CALL(value_visitor_, visit(Eq(thing(i /*, i + 1234 */)))).Times(1);
 		}
 
 		void expect_value(unsigned n) {
-			EXPECT_CALL(value_visitor_, visit(Eq(thing(n /*, n + 1234 */)))).Times(1);
+			EXPECT_CALL(value_visitor_, visit(Eq(thing(n, n + 1234)))).Times(1);
 		}
 
 		void expect_no_damage() {
@@ -152,24 +243,62 @@ TEST_F(BTreeDamageVisitorTests, visiting_an_empty_tree)
 	run();
 }
 
-TEST_F(BTreeDamageVisitorTests, visiting_an_tree_with_a_trashed_root)
+TEST_F(BTreeDamageVisitorTests, visiting_a_tree_with_a_trashed_root)
 {
 	trash_block(tree_->get_root());
 
 	expect_no_values();
-	expect_damage(0, range<uint64_t>());
+	expect_damage(0, range<uint64_t>(0ull));
 
 	run();
 }
 
 TEST_F(BTreeDamageVisitorTests, visiting_a_populated_tree_with_no_damage)
 {
-	insert_values(1000);
+	insert_values(10000);
 
-	expect_nr_values(1000);
+	expect_nr_values(10000);
 	expect_no_damage();
 
 	run();
 }
 
+TEST_F(BTreeDamageVisitorTests, visiting_a_populated_tree_with_a_damaged_leaf_node)
+{
+	insert_values(10000);
+	commit();
+
+	btree_layout<1, thing_traits> layout;
+	tree_->visit_depth_first(layout);
+
+	typedef typename btree_layout<1, thing_traits>::node_info node_info;
+	vector<typename node_info::ptr> const &nodes = layout.get_nodes();
+
+	unsigned nr_leaf = 0;
+	for (unsigned i = 0; i < nodes.size(); i++)
+		if (nodes[i]->leaf)
+			nr_leaf++;
+
+	unsigned target = random() % nr_leaf;
+	unsigned i;
+	for (i = 0; i < nodes.size(); i++)
+		if (nodes[i]->leaf) {
+			if (!target)
+				break;
+			else
+				target--;
+		}
+
+	typename node_info::ptr n = nodes[i];
+
+	trash_block(n->b);
+	cerr << "trashed leaf node with keys " << n->keys << endl;
+
+	expect_value_range(0, *n->keys.begin_);
+	expect_value_range(*n->keys.end_, 10000);
+	expect_damage(0, n->keys);
+
+	run();
+}
+
 //----------------------------------------------------------------