From eee72251d0ae5abcaacdd4471d02df22d45a89db Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Tue, 31 Mar 2015 12:21:19 -0400 Subject: [PATCH] add comparison and hash operators for dht::token Inspired in Gleb's previous patch, this patch adds a hash and comparison operator for dht::token. The previous patch, however, had a number of problems. Comparisons were failing in tokens that were verified (by me) to be equal to the ones Origin was generating. The main reasons for that, was that the byte-comparison loop must be unsigned, not signed. With the above change, the comparison function would always succeed *except* when the integer version of _data was that of a signed one. Looking at Origin, one verifies that the Murmur3Partitioner class overrides the comparison functions, and just does a Long comparison with the token. This patch implements a similar mechanism. With that, a list of tokens generated by origin in ascending order is verified by us to also be in ascending order. Signed-off-by: Glauber Costa --- dht/i_partitioner.cc | 74 ++++++++++++++++++++++++++++++++++++++ dht/i_partitioner.hh | 25 +++++++++++++ dht/murmur3_partitioner.cc | 27 ++++++++++++++ dht/murmur3_partitioner.hh | 2 ++ 4 files changed, 128 insertions(+) diff --git a/dht/i_partitioner.cc b/dht/i_partitioner.cc index d789d1a6a1..d5cdea81e3 100644 --- a/dht/i_partitioner.cc +++ b/dht/i_partitioner.cc @@ -70,6 +70,80 @@ midpoint(const token& t1, const token& t2) { return token{token::kind::key, std::move(avg)}; } +static inline unsigned char get_byte(const bytes& b, size_t off) { + if (off < b.size()) { + return b[off]; + } else { + return 0; + } +} + +bool i_partitioner::is_equal(const token& t1, const token& t2) { + + size_t sz = std::max(t1._data.size(), t2._data.size()); + + for (size_t i = 0; i < sz; i++) { + auto b1 = get_byte(t1._data, i); + auto b2 = get_byte(t2._data, i); + if (b1 != b2) { + return false; + } + } + return true; + +} + +bool i_partitioner::is_less(const token& t1, const token& t2) { + + size_t sz = std::max(t1._data.size(), t2._data.size()); + + for (size_t i = 0; i < sz; i++) { + auto b1 = get_byte(t1._data, i); + auto b2 = get_byte(t2._data, i); + if (b1 < b2) { + return true; + } else if (b1 > b2) { + return false; + } + } + return false; +} + +bool operator==(const token& t1, const token& t2) +{ + if (t1._kind != t2._kind) { + return false; + } else if (t1._kind == token::kind::key) { + return global_partitioner().is_equal(t1, t2); + } + return true; +} + +bool operator<(const token& t1, const token& t2) +{ + if (t1._kind < t2._kind) { + return true; + } else if (t1._kind == token::kind::key && t2._kind == token::kind::key) { + return global_partitioner().is_less(t1, t2); + } + return false; +} + +bool operator<(const decorated_key& lht, const decorated_key& rht) { + if (lht._token == rht._token) { + return static_cast(lht._key) < rht._key; + } else { + return lht._token < rht._token; + } +} + +bool operator==(const decorated_key& lht, const decorated_key& rht) { + if (lht._token == rht._token) { + return static_cast(lht._key) == rht._key; + } + return false; +} + // FIXME: get from global config // FIXME: make it per-keyspace murmur3_partitioner default_partitioner; diff --git a/dht/i_partitioner.hh b/dht/i_partitioner.hh index d3be589c9b..03c9502b89 100644 --- a/dht/i_partitioner.hh +++ b/dht/i_partitioner.hh @@ -63,6 +63,9 @@ public: token midpoint(const token& t1, const token& t2); token minimum_token(); +bool operator==(const token& t1, const token& t2); +bool operator<(const token& t1, const token& t2); + class decorated_key { public: @@ -143,8 +146,30 @@ public: virtual std::map describe_ownership(const std::vector& sorted_tokens) = 0; virtual data_type get_token_validator() = 0; + +protected: + /** + * @return true if t1's _data array is equal t2's. _kind comparison should be done separately. + */ + virtual bool is_equal(const token& t1, const token& t2); + /** + * @return true if t1's _data array is less then t2's. _kind comparison should be done separately. + */ + virtual bool is_less(const token& t1, const token& t2); + + friend bool operator==(const token& t1, const token& t2); + friend bool operator<(const token& t1, const token& t2); }; i_partitioner& global_partitioner(); } // dht + +namespace std { +template<> +struct hash { + size_t operator()(const dht::token& t) const { + return (t._kind == dht::token::kind::key) ? std::hash()(t._data) : 0; + } +}; +} diff --git a/dht/murmur3_partitioner.cc b/dht/murmur3_partitioner.cc index ee4b84cdc2..09d0cf1ce5 100644 --- a/dht/murmur3_partitioner.cc +++ b/dht/murmur3_partitioner.cc @@ -32,6 +32,33 @@ murmur3_partitioner::get_token(const partition_key& key_) { return token{token::kind::key, std::move(b)}; } +inline long long_token(const token& t) { + + if (t._data.size() != sizeof(long)) { + throw runtime_exception(sprint("Invalid token. Should have size %ld, has size %ld\n", sizeof(long), t._data.size())); + } + + auto ptr = const_cast(t._data.c_str()); + auto lp = reinterpret_cast(ptr); + return net::ntoh(*lp); +} + +bool murmur3_partitioner::is_equal(const token& t1, const token& t2) { + + auto l1 = long_token(t1); + auto l2 = long_token(t2); + + return l1 == l2; +} + +bool murmur3_partitioner::is_less(const token& t1, const token& t2) { + + auto l1 = long_token(t1); + auto l2 = long_token(t2); + + return l1 < l2; +} + std::map murmur3_partitioner::describe_ownership(const std::vector& sorted_tokens) { abort(); diff --git a/dht/murmur3_partitioner.hh b/dht/murmur3_partitioner.hh index bcdaa5d2e5..19c64c6ed7 100644 --- a/dht/murmur3_partitioner.hh +++ b/dht/murmur3_partitioner.hh @@ -14,6 +14,8 @@ public: virtual bool preserves_order() override { return false; } virtual std::map describe_ownership(const std::vector& sorted_tokens); virtual data_type get_token_validator(); + virtual bool is_equal(const token& t1, const token& t2); + virtual bool is_less(const token& t1, const token& t2); private: static int64_t normalize(int64_t in); };