Merge branch 'cleanup-sst' of github.com:glommer/urchin into db

Sstable cleanup, from Glauber (privatize some functions).

sstables/partition.cc

@@ -5,6 +5,7 @@
 #include "sstables.hh"
 #include "types.hh"
 #include "core/future-util.hh"
+#include "key.hh"
 
 #include "dht/i_partitioner.hh"
 
@@ -24,7 +25,7 @@ namespace sstables {
 // This code should work in all kinds of vectors in whose's elements is possible to aquire
 // a key view.
 template <typename T>
-int binary_search(const T& entries, const key& sk) {
+int sstable::binary_search(const T& entries, const key& sk) {
     int low = 0, mid = entries.size(), high = mid - 1, result = -1;
 
     auto& partitioner = dht::global_partitioner();
@@ -58,7 +59,7 @@ int binary_search(const T& entries, const key& sk) {
     return -mid - (result < 0 ? 1 : 2);
 }
 
-int summary::binary_search(const key& sk) {
-    return sstables::binary_search(entries, sk);
-}
+// Force generation, so we make it available outside this compilation unit without moving that
+// much code to .hh
+template int sstable::binary_search<>(const std::vector<summary_entry>& entries, const key& sk);
 }

sstables/sstables.hh

@@ -19,6 +19,7 @@
 #include "row.hh"
 
 namespace sstables {
+class key;
 
 class malformed_sstable_exception : public std::exception {
     sstring _msg;
@@ -105,6 +106,10 @@ private:
 
     future<index_list> read_indexes(uint64_t position, uint64_t quantity);
 
+    future<index_list> read_indexes(uint64_t position) {
+        return read_indexes(position, _summary.header.sampling_level);
+    }
+
     input_stream<char> data_stream_at(uint64_t pos);
     // Read exactly the specific byte range from the data file (after
     // uncompression, if the file is compressed). This can be used to read
@@ -114,7 +119,10 @@ private:
     // for iteration through all the rows.
     future<temporary_buffer<char>> data_read(uint64_t pos, size_t len);
 
+    template <typename T>
+    int binary_search(const T& entries, const key& sk);
 
+    future<summary_entry&> read_summary_entry(size_t i);
 public:
     // Read one or few rows at the given byte range from the data file,
     // feeding them into the consumer. This function reads the entire given
@@ -144,15 +152,9 @@ public:
     static version_types version_from_sstring(sstring& s);
     static format_types format_from_sstring(sstring& s);
 
-    future<index_list> read_indexes(uint64_t position) {
-        return read_indexes(position, _summary.header.sampling_level);
-    }
-
     future<> load();
     future<> store();
 
-    future<summary_entry&> read_summary_entry(size_t i);
-
     void set_generation(unsigned long generation) { _generation = generation; }
 
     // Allow the test cases from sstable_test.cc to test private methods. We use

sstables/types.hh

@@ -2,7 +2,6 @@
 
 #include "core/enum.hh"
 #include "bytes.hh"
-#include "sstables/key.hh"
 
 namespace sstables {
 
@@ -115,7 +114,6 @@ struct summary_la {
     // filled with the same data. It's too early to judge that the data is useless.
     // However, it was tested that Cassandra loads successfully a Summary file with
     // this structure removed from it. Anyway, let's pay attention to it.
-    int binary_search(const key& sk);
 };
 using summary = summary_la;
 

tests/urchin/sstable_test.cc

@@ -11,7 +11,9 @@
 #include "core/align.hh"
 #include "core/do_with.hh"
 #include "sstables/sstables.hh"
+#include "sstables/key.hh"
 #include "tests/test-utils.hh"
+#include "schema.hh"
 #include <memory>
 
 using namespace sstables;
@@ -56,6 +58,10 @@ public:
         return _sst->read_summary();
     }
 
+    future<summary_entry&> read_summary_entry(size_t i) {
+        return _sst->read_summary_entry(i);
+    }
+
     summary& get_summary() {
         return _sst->_summary;
     }
@@ -68,6 +74,10 @@ public:
         return _sst->_components;
     }
 
+    template <typename T>
+    int binary_search(const T& entries, const key& sk) {
+        return _sst->binary_search(entries, sk);
+    }
 };
 }
 
@@ -201,7 +211,7 @@ SEASTAR_TEST_CASE(composite_index_read_0_21_20) {
 template<uint64_t Position, uint64_t EntryPosition, uint64_t EntryKeySize>
 future<> summary_query(sstring path, int generation) {
     return reusable_sst(path, generation).then([] (sstable_ptr ptr) {
-        return ptr->read_summary_entry(Position).then([ptr] (auto entry) {
+        return sstables::test(ptr).read_summary_entry(Position).then([ptr] (auto entry) {
             BOOST_REQUIRE(entry.position == EntryPosition);
             BOOST_REQUIRE(entry.key.size() == EntryKeySize);
             return make_ready_future<>();
@@ -773,7 +783,7 @@ SEASTAR_TEST_CASE(find_key_map) {
         kk.push_back(map);
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
-        BOOST_REQUIRE(summary.binary_search(key) == 0);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == 0);
     });
 }
 
@@ -793,7 +803,7 @@ SEASTAR_TEST_CASE(find_key_set) {
         kk.push_back(set);
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
-        BOOST_REQUIRE(summary.binary_search(key) == 0);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == 0);
     });
 }
 
@@ -813,7 +823,7 @@ SEASTAR_TEST_CASE(find_key_list) {
         kk.push_back(list);
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
-        BOOST_REQUIRE(summary.binary_search(key) == 0);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == 0);
     });
 }
 
@@ -831,7 +841,7 @@ SEASTAR_TEST_CASE(find_key_composite) {
         kk.push_back(boost::any(b2));
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
-        BOOST_REQUIRE(summary.binary_search(key) == 0);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == 0);
     });
 }
 
@@ -842,7 +852,7 @@ SEASTAR_TEST_CASE(all_in_place) {
         int idx = 0;
         for (auto& e: summary.entries) {
             auto key = sstables::key::from_bytes(e.key);
-            BOOST_REQUIRE(summary.binary_search(key) == idx++);
+            BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == idx++);
         }
     });
 }
@@ -861,6 +871,6 @@ SEASTAR_TEST_CASE(not_find_key_composite_bucket0) {
 
         auto key = sstables::key::from_deeply_exploded(*s, kk);
         // (result + 1) * -1 -1 = 0
-        BOOST_REQUIRE(summary.binary_search(key) == -2);
+        BOOST_REQUIRE(sstables::test(sstp).binary_search(summary.entries, key) == -2);
     });
 }