From 90675d3a95518f189bea70121b894edbcfc05d96 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 26 Aug 2011 11:13:13 +0100 Subject: [PATCH] include sm_disk bitmaps in metadata counts --- btree.h | 8 +++++--- btree.tcc | 22 +++++++++------------ btree_validator.h | 49 ++++++++++++++++++++++++++++------------------- metadata.cc | 34 ++++++++++++++++++++++---------- space_map_disk.h | 12 +++++++++++- 5 files changed, 78 insertions(+), 47 deletions(-) diff --git a/btree.h b/btree.h index 0235471..13dbc7f 100644 --- a/btree.h +++ b/btree.h @@ -325,9 +325,11 @@ namespace persistent_data { virtual ~visitor() {} typedef boost::shared_ptr ptr; - virtual void visit_internal(unsigned level, bool is_root, internal_node const &n) = 0; - virtual void visit_internal_leaf(unsigned level, bool is_root, internal_node const &n) = 0; - virtual void visit_leaf(unsigned level, bool is_root, leaf_node const &n) = 0; + // The bool return values indicate whether the walk + // should be continued into sub trees of the node (true == continue). + virtual bool visit_internal(unsigned level, bool is_root, internal_node const &n) = 0; + virtual bool visit_internal_leaf(unsigned level, bool is_root, internal_node const &n) = 0; + virtual bool visit_leaf(unsigned level, bool is_root, leaf_node const &n) = 0; }; // Walks the tree in depth first order diff --git a/btree.tcc b/btree.tcc index f821153..5d9dca3 100644 --- a/btree.tcc +++ b/btree.tcc @@ -627,25 +627,21 @@ walk_tree(typename visitor::ptr visitor, { using namespace btree_detail; - try { - read_ref blk = tm_->read_lock(b); - internal_node o = to_node(blk); - if (o.get_type() == INTERNAL) { - visitor->visit_internal(level, is_root, o); + read_ref blk = tm_->read_lock(b); + internal_node o = to_node(blk); + if (o.get_type() == INTERNAL) { + if (visitor->visit_internal(level, is_root, o)) for (unsigned i = 0; i < o.get_nr_entries(); i++) walk_tree(visitor, level, false, o.value_at(i)); - } else if (level < Levels - 1) { - visitor->visit_internal_leaf(level, is_root, o); + } else if (level < Levels - 1) { + if (visitor->visit_internal_leaf(level, is_root, o)) for (unsigned i = 0; i < o.get_nr_entries(); i++) walk_tree(visitor, level + 1, true, o.value_at(i)); - } else { - leaf_node ov = to_node(blk); - visitor->visit_leaf(level, is_root, ov); - } - } catch (...) { // FIXME: we should only catch terminate_walk type exceptions - + } else { + leaf_node ov = to_node(blk); + visitor->visit_leaf(level, is_root, ov); } } diff --git a/btree_validator.h b/btree_validator.h index 4e6fde6..7fa4843 100644 --- a/btree_validator.h +++ b/btree_validator.h @@ -74,51 +74,60 @@ namespace persistent_data { errs_(new error_set("btree errors")) { } - void visit_internal(unsigned level, bool is_root, + bool visit_internal(unsigned level, bool is_root, btree_detail::node_ref const &n) { - check_duplicate_block(n.get_location()); + if (already_visited(n)) + return false; + check_block_nr(n); check_max_entries(n); check_nr_entries(n, is_root); - - for (unsigned i = 0; i < n.get_nr_entries(); i++) - counter_.inc(n.value_at(i)); + return true; } - void visit_internal_leaf(unsigned level, bool is_root, + bool visit_internal_leaf(unsigned level, bool is_root, btree_detail::node_ref const &n) { - check_duplicate_block(n.get_location()); + if (already_visited(n)) + return false; + check_block_nr(n); check_max_entries(n); check_nr_entries(n, is_root); - - for (unsigned i = 0; i < n.get_nr_entries(); i++) - counter_.inc(n.value_at(i)); + return true; } - void visit_leaf(unsigned level, bool is_root, + bool visit_leaf(unsigned level, bool is_root, btree_detail::node_ref const &n) { - counter_.inc(n.get_location()); - check_duplicate_block(n.get_location()); + if (already_visited(n)) + return false; + check_block_nr(n); check_max_entries(n); check_nr_entries(n, is_root); + return true; } boost::optional get_errors() const { return errs_; } + protected: + block_counter &get_counter() { + return counter_; + } + private: - void check_duplicate_block(block_address b) { - if (seen_.count(b)) { - std::ostringstream out; - out << "duplicate block in btree: " << b; - errs_->add_child(out.str()); - throw runtime_error(out.str()); - } + template + bool already_visited(node const &n) { + block_address b = n.get_location(); + + counter_.inc(b); + + if (seen_.count(b) > 0) + return true; seen_.insert(b); + return false; } template diff --git a/metadata.cc b/metadata.cc index ea9e253..3077d51 100644 --- a/metadata.cc +++ b/metadata.cc @@ -54,20 +54,31 @@ namespace { data_counter_(data_counter) { } - void visit_internal_leaf(unsigned level, bool is_root, + // Sharing can only occur in level 1 nodes. + // FIXME: not true once we start having held roots. + bool visit_internal_leaf(unsigned level, bool is_root, btree_detail::node_ref const &n) { - btree_validator<2, block_traits, MD_BLOCK_SIZE>::visit_internal_leaf(level, is_root, n); + + bool r = btree_validator<2, block_traits, MD_BLOCK_SIZE>::visit_internal_leaf(level, is_root, n); + if (!r && level == 0) { + throw runtime_error("unexpected sharing in level 0 of mapping tree."); + } for (unsigned i = 0; i < n.get_nr_entries(); i++) devices_.insert(n.key_at(i)); + + return r; } - void visit_leaf(unsigned level, bool is_root, + bool visit_leaf(unsigned level, bool is_root, btree_detail::node_ref const &n) { - btree_validator<2, block_traits, MD_BLOCK_SIZE>::visit_leaf(level, is_root, n); + bool r = btree_validator<2, block_traits, MD_BLOCK_SIZE>::visit_leaf(level, is_root, n); - for (unsigned i = 0; i < n.get_nr_entries(); i++) - data_counter_.inc(n.value_at(i).block_); + if (r) + for (unsigned i = 0; i < n.get_nr_entries(); i++) + data_counter_.inc(n.value_at(i).block_); + + return r; } set const &get_devices() const { @@ -87,12 +98,15 @@ namespace { : btree_validator<1, device_details_traits, MD_BLOCK_SIZE>(counter) { } - void visit_leaf(unsigned level, bool is_root, + bool visit_leaf(unsigned level, bool is_root, btree_detail::node_ref const &n) { - btree_validator<1, device_details_traits, MD_BLOCK_SIZE>::visit_leaf(level, is_root, n); + bool r = btree_validator<1, device_details_traits, MD_BLOCK_SIZE>::visit_leaf(level, is_root, n); - for (unsigned i = 0; i < n.get_nr_entries(); i++) - devices_.insert(n.key_at(i)); + if (r) + for (unsigned i = 0; i < n.get_nr_entries(); i++) + devices_.insert(n.key_at(i)); + + return r; } set const &get_devices() const { diff --git a/space_map_disk.h b/space_map_disk.h index ff78020..22c61c9 100644 --- a/space_map_disk.h +++ b/space_map_disk.h @@ -321,6 +321,17 @@ namespace persistent_data { bitmap_tree_validator(block_counter &counter) : btree_validator<1, index_entry_traits, BlockSize>(counter) { } + + bool visit_leaf(unsigned level, bool is_root, + btree_detail::node_ref const &n) { + bool r = btree_validator<1, index_entry_traits, BlockSize>::visit_leaf(level, is_root, n); + + if (r) + for (unsigned i = 0; i < n.get_nr_entries(); i++) + btree_validator<1, index_entry_traits, BlockSize>::get_counter().inc(n.value_at(i).blocknr_); + + return r; + } }; template @@ -362,7 +373,6 @@ namespace persistent_data { sm_disk_base::check(counter); typename bitmap_tree_validator::ptr v(new bitmap_tree_validator(counter)); - counter.inc(bitmaps_.get_root()); bitmaps_.visit(v); }