From 9cee0465940677754536dbf4037f377f0ea7b820 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 1 Nov 2011 11:31:03 +0000 Subject: [PATCH] fix a bug in btree lookup --- btree.h | 29 +++-------------- btree.tcc | 73 ++++++++++++++++++++++++++++++++++++++++--- unit-tests/btree_t.cc | 54 ++++++++++++++++++++++++++------ 3 files changed, 119 insertions(+), 37 deletions(-) diff --git a/btree.h b/btree.h index 40f1c60..4666aab 100644 --- a/btree.h +++ b/btree.h @@ -97,6 +97,7 @@ namespace persistent_data { void set_max_entries(); // calculates the max for you. size_t get_value_size() const; + void set_value_size(size_t); uint64_t key_at(unsigned i) const; void set_key(unsigned i, uint64_t k); @@ -258,30 +259,6 @@ namespace persistent_data { std::list::write_ref> spine_; block_address root_; }; - - // FIXME: make a member of btree - template - optional - lookup_raw(ro_spine &spine, block_address block, uint64_t key) { - - using namespace boost; - typedef typename ValueTraits::value_type leaf_type; - - for (;;) { - spine.step(block); - node_ref leaf = spine.template get_node(); - - optional mi = leaf.exact_search(key); - if (!mi) - return optional(); - - if (leaf.get_type() == btree_detail::LEAF) - return optional(leaf.value_at(*mi)); - - node_ref internal = spine.template get_node(); - block = internal.value_at(*mi); - } - } } template @@ -346,6 +323,10 @@ namespace persistent_data { void visit(typename visitor::ptr visitor) const; private: + template + optional + lookup_raw(btree_detail::ro_spine &spine, block_address block, uint64_t key) const; + template void split_node(btree_detail::shadow_spine &spine, block_address parent_index, diff --git a/btree.tcc b/btree.tcc index fed0d1a..93461a0 100644 --- a/btree.tcc +++ b/btree.tcc @@ -102,6 +102,13 @@ node_ref::get_value_size() const return to_cpu(raw_->header.value_size); } +template +void +node_ref::set_value_size(size_t s) +{ + raw_->header.value_size = to_disk<__le32>(static_cast(s)); +} + template uint64_t node_ref::key_at(unsigned i) const @@ -181,9 +188,9 @@ node_ref::copy_entries(node_ref const &rhs, if ((n + count) > get_max_entries()) throw runtime_error("too many entries"); - set_nr_entries(n + count); ::memcpy(key_ptr(n), rhs.key_ptr(begin), sizeof(uint64_t) * count); ::memcpy(value_ptr(n), rhs.value_ptr(begin), sizeof(typename ValueTraits::disk_type) * count); + set_nr_entries(n + count); } template @@ -216,6 +223,9 @@ node_ref::exact_search(uint64_t key) const if (i < 0 || static_cast(i) >= get_nr_entries()) return optional(); + if (key != key_at(i)) + return optional(); + return optional(i); } @@ -287,6 +297,7 @@ btree(typename transaction_manager::ptr tm, n.set_type(btree_detail::LEAF); n.set_nr_entries(0); n.set_max_entries(); + n.set_value_size(sizeof(typename ValueTraits::disk_type)); root_ = root.get_location(); } @@ -309,6 +320,22 @@ btree::~btree() } +namespace { + template + struct lower_bound_search { + static optional search(btree_detail::node_ref n, uint64_t key) { + return n.lower_bound(key); + } + }; + + template + struct exact_search { + static optional search(btree_detail::node_ref n, uint64_t key) { + return n.exact_search(key); + } + }; +} + template typename btree::maybe_value btree::lookup(key const &key) const @@ -320,14 +347,14 @@ btree::lookup(key const &key) const for (unsigned level = 0; level < Levels - 1; ++level) { optional mroot = - lookup_raw(spine, root, key[level]); + lookup_raw >(spine, root, key[level]); if (!mroot) return maybe_value(); root = *mroot; } - return lookup_raw(spine, root, key[Levels - 1]); + return lookup_raw >(spine, root, key[Levels - 1]); } template @@ -422,6 +449,38 @@ btree::destroy() } #endif +template +template +optional +btree:: +lookup_raw(ro_spine &spine, block_address block, uint64_t key) const +{ + using namespace boost; + typedef typename ValueTraits::value_type leaf_type; + + for (;;) { + spine.step(block); + node_ref leaf = spine.template get_node(); + + optional mi; + if (leaf.get_type() == btree_detail::LEAF) { + mi = Search::search(leaf, key); + if (!mi) + return optional(); + return optional(leaf.value_at(*mi)); + + } + + mi = leaf.lower_bound(key); + if (!mi || *mi < 0) + return optional(); + + node_ref internal = spine.template get_node(); + block = internal.value_at(*mi); + } +} + + template template void @@ -456,14 +515,20 @@ split_beneath(btree_detail::shadow_spine &spine, node_ref l = to_node(left); l.set_nr_entries(0); l.set_max_entries(); + l.set_value_size(sizeof(typename ValueTraits::disk_type)); write_ref right = tm_->new_block(); node_ref r = to_node(right); r.set_nr_entries(0); r.set_max_entries(); + r.set_value_size(sizeof(typename ValueTraits::disk_type)); { node_ref p = spine.template get_node(); + + if (p.get_value_size() != sizeof(typename ValueTraits::disk_type)) + throw std::runtime_error("bad value_size"); + nr_left = p.get_nr_entries() / 2; nr_right = p.get_nr_entries() - nr_left; type = p.get_type(); @@ -480,8 +545,8 @@ split_beneath(btree_detail::shadow_spine &spine, internal_node p = spine.template get_node(); p.set_type(btree_detail::INTERNAL); p.set_nr_entries(2); + p.set_value_size(sizeof(typename uint64_traits::disk_type)); - // FIXME: set the value_size p.overwrite_at(0, l.key_at(0), left.get_location()); p.overwrite_at(1, r.key_at(0), right.get_location()); } diff --git a/unit-tests/btree_t.cc b/unit-tests/btree_t.cc index 997ef68..d0cdba5 100644 --- a/unit-tests/btree_t.cc +++ b/unit-tests/btree_t.cc @@ -1,5 +1,5 @@ #include "transaction_manager.h" -#include "core_map.h" +#include "space_map_core.h" #include "btree.h" #define BOOST_TEST_MODULE BTreeTests @@ -16,7 +16,7 @@ namespace { transaction_manager::ptr create_tm() { - block_manager<>::ptr bm(new block_manager<>("./test.data", NR_BLOCKS)); + block_manager<>::ptr bm(new block_manager<>("./test.data", NR_BLOCKS, 4, true)); space_map::ptr sm(new core_map(NR_BLOCKS)); transaction_manager::ptr tm(new transaction_manager(bm, sm)); return tm; @@ -36,17 +36,26 @@ namespace { // class constraint_visitor : public btree<1, uint64_traits>::visitor { public: - bool visit_internal(unsigned level, bool is_root, btree_detail::node_ref const &n) { + typedef btree_detail::node_ref internal_node; + typedef btree_detail::node_ref leaf_node; + + bool visit_internal(unsigned level, bool sub_root, + boost::optional key, + internal_node const &n) { check_duplicate_block(n.get_location()); return true; } - bool visit_internal_leaf(unsigned level, bool is_root, btree_detail::node_ref const &n) { + bool visit_internal_leaf(unsigned level, bool sub_root, + boost::optional key, + internal_node const &n) { check_duplicate_block(n.get_location()); return true; } - bool visit_leaf(unsigned level, bool is_root, btree_detail::node_ref const &n) { + bool visit_leaf(unsigned level, bool sub_root, + boost::optional key, + leaf_node const &n) { check_duplicate_block(n.get_location()); return true; } @@ -77,7 +86,7 @@ namespace { BOOST_AUTO_TEST_CASE(empty_btree_contains_nothing) { - auto tree = create_btree(); + btree<1, uint64_traits>::ptr tree = create_btree(); check_constraints(tree); for (uint64_t i = 0; i < 1000; i++) { @@ -90,19 +99,46 @@ BOOST_AUTO_TEST_CASE(insert_works) { unsigned const COUNT = 100000; - auto tree = create_btree(); + btree<1, uint64_traits>::ptr tree = create_btree(); for (uint64_t i = 0; i < COUNT; i++) { uint64_t key[1] = {i * 7}; uint64_t value = i; tree->insert(key, value); - auto l = tree->lookup(key); - BOOST_CHECK(l); + btree<1, uint64_traits>::maybe_value l = tree->lookup(key); + BOOST_REQUIRE(l); BOOST_CHECK_EQUAL(*l, i); } check_constraints(tree); } +BOOST_AUTO_TEST_CASE(insert_does_not_insert_imaginary_values) +{ + btree<1, uint64_traits>::ptr tree = create_btree(); + uint64_t key[1] = {0}; + uint64_t value = 100; + + btree<1, uint64_traits>::maybe_value l = tree->lookup(key); + BOOST_CHECK(!l); + + key[0] = 1; + l = tree->lookup(key); + BOOST_CHECK(!l); + + key[0] = 0; + tree->insert(key, value); + + l = tree->lookup(key); + BOOST_REQUIRE(l); + BOOST_CHECK_EQUAL(*l, 100); + + key[0] = 1; + l = tree->lookup(key); + BOOST_CHECK(!l); + + check_constraints(tree); +} + //----------------------------------------------------------------