From 8107d99e3d47a1069ccbd169feda036695488f38 Mon Sep 17 00:00:00 2001 From: Piotr Jastrzebski Date: Wed, 29 Jan 2020 09:17:19 +0100 Subject: [PATCH] partitioner: move from_string to token Signed-off-by: Piotr Jastrzebski --- db/size_estimates_virtual_reader.cc | 2 +- db/system_distributed_keyspace.cc | 27 +-------------------------- db/system_keyspace.cc | 8 ++++---- dht/boot_strapper.cc | 2 +- dht/i_partitioner.hh | 5 ----- dht/murmur3_partitioner.cc | 9 --------- dht/murmur3_partitioner.hh | 1 - dht/token.cc | 9 +++++++++ dht/token.hh | 5 +++++ repair/repair.cc | 8 ++++---- service/storage_service.cc | 6 +++--- service/storage_service.hh | 2 +- test/boost/mutation_reader_test.cc | 1 - test/boost/nonwrapping_range_test.cc | 6 +++--- test/boost/range_test.cc | 6 +++--- thrift/handler.cc | 10 +++++----- 16 files changed, 40 insertions(+), 67 deletions(-) diff --git a/db/size_estimates_virtual_reader.cc b/db/size_estimates_virtual_reader.cc index 242288f889..f0eb05fd65 100644 --- a/db/size_estimates_virtual_reader.cc +++ b/db/size_estimates_virtual_reader.cc @@ -169,7 +169,7 @@ static system_keyspace::range_estimates estimate(const column_family& cf, const int64_t count{0}; utils::estimated_histogram hist{0}; auto from_bytes = [] (auto& b) { - return dht::global_partitioner().from_sstring(utf8_type->to_string(b)); + return dht::token::from_sstring(utf8_type->to_string(b)); }; dht::token_range_vector ranges; ::compat::unwrap_into( diff --git a/db/system_distributed_keyspace.cc b/db/system_distributed_keyspace.cc index a9d9fe09bd..0adc551944 100644 --- a/db/system_distributed_keyspace.cc +++ b/db/system_distributed_keyspace.cc @@ -230,31 +230,6 @@ static db::consistency_level quorum_if_many(size_t num_token_owners) { return num_token_owners > 1 ? db::consistency_level::QUORUM : db::consistency_level::ONE; } -// --------------- TODO: copy-pasted from murmur3_partitioner; remove this after haaawk's change is merged -static int64_t normalize(int64_t in) { - return in == std::numeric_limits::lowest() - ? std::numeric_limits::max() - : in; -} - -static dht::token get_token(uint64_t value) { - auto t = net::hton(normalize(value)); - bytes b(bytes::initialized_later(), 8); - std::copy_n(reinterpret_cast(&t), 8, b.begin()); - return dht::token{dht::token::kind::key, std::move(b)}; -} - -static dht::token token_from_string(const sstring& t) { - auto lp = boost::lexical_cast(t); - if (lp == std::numeric_limits::min()) { - return dht::minimum_token(); - } else { - return get_token(uint64_t(lp)); - } -} - -// --------------- - static list_type_impl::native_type prepare_cdc_generation_description(const cdc::topology_description& description) { list_type_impl::native_type ret; for (auto& e: description.entries()) { @@ -295,7 +270,7 @@ static cdc::token_range_description get_token_range_description_from_value(const on_internal_error(cdc_log, "get_token_range_description_from_value: stream tuple type size != 3"); } - auto token = token_from_string(value_cast(tup[0])); + auto token = dht::token::from_sstring(value_cast(tup[0])); auto streams = get_streams_from_list_value(tup[1]); auto sharding_ignore_msb = uint8_t(value_cast(tup[2])); diff --git a/db/system_keyspace.cc b/db/system_keyspace.cc index 08d5a67307..a549e18a1c 100644 --- a/db/system_keyspace.cc +++ b/db/system_keyspace.cc @@ -1519,8 +1519,8 @@ std::unordered_set decode_tokens(set_type_impl::native_type& tokens) std::unordered_set tset; for (auto& t: tokens) { auto str = value_cast(t); - assert(str == dht::global_partitioner().from_sstring(str).to_sstring()); - tset.insert(dht::global_partitioner().from_sstring(str)); + assert(str == dht::token::from_sstring(str).to_sstring()); + tset.insert(dht::token::from_sstring(str)); } return tset; } @@ -2134,11 +2134,11 @@ future> load_view_build_progress() { for (auto& row : *cql_result) { auto ks_name = row.get_as("keyspace_name"); auto cf_name = row.get_as("view_name"); - auto first_token = dht::global_partitioner().from_sstring(row.get_as("first_token")); + auto first_token = dht::token::from_sstring(row.get_as("first_token")); auto next_token_sstring = row.get_opt("next_token"); std::optional next_token; if (next_token_sstring) { - next_token = dht::global_partitioner().from_sstring(std::move(next_token_sstring).value()); + next_token = dht::token::from_sstring(std::move(next_token_sstring).value()); } auto cpu_id = row.get_as("cpu_id"); progress.emplace_back(view_build_progress{ diff --git a/dht/boot_strapper.cc b/dht/boot_strapper.cc index c0b724e1dc..a568a1493a 100644 --- a/dht/boot_strapper.cc +++ b/dht/boot_strapper.cc @@ -79,7 +79,7 @@ std::unordered_set boot_strapper::get_bootstrap_tokens(token_metadata met blogger.debug("tokens manually specified as {}", initial_tokens); std::unordered_set tokens; for (auto& token_string : initial_tokens) { - auto token = dht::global_partitioner().from_sstring(token_string); + auto token = dht::token::from_sstring(token_string); if (metadata.get_endpoint(token)) { throw std::runtime_error(format("Bootstrapping to existing token {} is not allowed (decommission/removenode the old node first).", token_string)); } diff --git a/dht/i_partitioner.hh b/dht/i_partitioner.hh index 943a90361c..d7c3df4ff4 100644 --- a/dht/i_partitioner.hh +++ b/dht/i_partitioner.hh @@ -183,11 +183,6 @@ public: virtual token get_token(const schema& s, partition_key_view key) const = 0; virtual token get_token(const sstables::key_view& key) const = 0; - /** - * @return a token from its partitioner-specific string representation - */ - virtual dht::token from_sstring(const sstring& t) const = 0; - /** * @return a token from its partitioner-specific byte representation */ diff --git a/dht/murmur3_partitioner.cc b/dht/murmur3_partitioner.cc index 78307cc9b5..15a6c2a511 100644 --- a/dht/murmur3_partitioner.cc +++ b/dht/murmur3_partitioner.cc @@ -104,15 +104,6 @@ murmur3_partitioner::bias(uint64_t n) const { return get_token(n - uint64_t(std::numeric_limits::min())); } -dht::token murmur3_partitioner::from_sstring(const sstring& t) const { - auto lp = boost::lexical_cast(t); - if (lp == std::numeric_limits::min()) { - return minimum_token(); - } else { - return get_token(uint64_t(lp)); - } -} - dht::token murmur3_partitioner::from_bytes(bytes_view bytes) const { if (bytes.size() != sizeof(int64_t)) { throw runtime_exception(format("Invalid token. Should have size {:d}, has size {:d}\n", sizeof(int64_t), bytes.size())); diff --git a/dht/murmur3_partitioner.hh b/dht/murmur3_partitioner.hh index dca746ce6d..dd5a99d123 100644 --- a/dht/murmur3_partitioner.hh +++ b/dht/murmur3_partitioner.hh @@ -44,7 +44,6 @@ public: virtual std::map describe_ownership(const std::vector& sorted_tokens) override; virtual data_type get_token_validator() override; virtual int tri_compare(const token& t1, const token& t2) const override; - virtual dht::token from_sstring(const sstring& t) const override; virtual dht::token from_bytes(bytes_view bytes) const override; virtual unsigned shard_of(const token& t) const override; diff --git a/dht/token.cc b/dht/token.cc index bd685e2430..bec2986e44 100644 --- a/dht/token.cc +++ b/dht/token.cc @@ -112,4 +112,13 @@ token token::get_random_token() { return {kind::key, dht::get_random_number()}; } +token token::from_sstring(const sstring& t) { + auto lp = boost::lexical_cast(t); + if (lp == std::numeric_limits::min()) { + return minimum_token(); + } else { + return token(kind::key, uint64_t(lp)); + } +} + } // namespace dht diff --git a/dht/token.hh b/dht/token.hh index 11e347fd28..90fd573a60 100644 --- a/dht/token.hh +++ b/dht/token.hh @@ -113,6 +113,11 @@ public: */ static token get_random_token(); + /** + * @return a token from string representation + */ + static dht::token from_sstring(const sstring& t); + }; const token& minimum_token(); diff --git a/repair/repair.cc b/repair/repair.cc index 3d914131b0..b56de6403b 100644 --- a/repair/repair.cc +++ b/repair/repair.cc @@ -1232,8 +1232,8 @@ private: throw(std::runtime_error("range must have two components " "separated by ':', got '" + range + "'")); } - auto tok_start = dht::global_partitioner().from_sstring(token_strings[0]); - auto tok_end = dht::global_partitioner().from_sstring(token_strings[1]); + auto tok_start = dht::token::from_sstring(token_strings[0]); + auto tok_end = dht::token::from_sstring(token_strings[1]); auto rng = wrapping_range( ::range::bound(tok_start, false), ::range::bound(tok_end, true)); @@ -1377,12 +1377,12 @@ static int do_repair_start(seastar::sharded& db, sstring keyspace, std::optional<::range::bound> tok_end; if (!options.start_token.empty()) { tok_start = ::range::bound( - dht::global_partitioner().from_sstring(options.start_token), + dht::token::from_sstring(options.start_token), true); } if (!options.end_token.empty()) { tok_end = ::range::bound( - dht::global_partitioner().from_sstring(options.end_token), + dht::token::from_sstring(options.end_token), false); } dht::token_range given_range_complement(tok_end, tok_start); diff --git a/service/storage_service.cc b/service/storage_service.cc index eb510e5067..a6a8e938b2 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -402,7 +402,7 @@ std::unordered_set get_replace_tokens() { } tokens.erase(""); for (auto token_string : tokens) { - auto token = dht::global_partitioner().from_sstring(token_string); + auto token = dht::token::from_sstring(token_string); ret.insert(token); } return ret; @@ -804,7 +804,7 @@ void storage_service::join_token_ring(int delay) { } } else { for (auto token_string : initial_tokens) { - auto token = dht::global_partitioner().from_sstring(token_string); + auto token = dht::token::from_sstring(token_string); _bootstrap_tokens.insert(token); } slogger.info("Saved tokens not found. Using configuration value: {}", _bootstrap_tokens); @@ -1645,7 +1645,7 @@ std::unordered_set storage_service::get_tokens_for(inet_address std::unordered_set ret; boost::split(tokens, tokens_string, boost::is_any_of(";")); for (auto str : tokens) { - auto t = dht::global_partitioner().from_sstring(str); + auto t = dht::token::from_sstring(str); slogger.trace("endpoint={}, token_str={} token={}", endpoint, str, t); ret.emplace(std::move(t)); } diff --git a/service/storage_service.hh b/service/storage_service.hh index 6d59507dc5..b737a36114 100644 --- a/service/storage_service.hh +++ b/service/storage_service.hh @@ -1894,7 +1894,7 @@ private: public: future<> move(sstring new_token) { // FIXME: getPartitioner().getTokenFactory().validate(newToken); - return move(dht::global_partitioner().from_sstring(new_token)); + return move(dht::token::from_sstring(new_token)); } private: diff --git a/test/boost/mutation_reader_test.cc b/test/boost/mutation_reader_test.cc index 944bf10a3b..fd4fd1e0a7 100644 --- a/test/boost/mutation_reader_test.cc +++ b/test/boost/mutation_reader_test.cc @@ -1968,7 +1968,6 @@ public: virtual dht::token get_token(const schema& s, partition_key_view key) const override { return _partitioner.get_token(s, key); } virtual dht::token get_token(const sstables::key_view& key) const override { return _partitioner.get_token(key); } - virtual dht::token from_sstring(const sstring& t) const override { return _partitioner.from_sstring(t); } virtual dht::token from_bytes(bytes_view bytes) const override { return _partitioner.from_bytes(bytes); } virtual bool preserves_order() override { return _partitioner.preserves_order(); } virtual std::map describe_ownership(const std::vector& sorted_tokens) override { return _partitioner.describe_ownership(sorted_tokens); } diff --git a/test/boost/nonwrapping_range_test.cc b/test/boost/nonwrapping_range_test.cc index c647795ded..54bc67a8ee 100644 --- a/test/boost/nonwrapping_range_test.cc +++ b/test/boost/nonwrapping_range_test.cc @@ -245,8 +245,8 @@ BOOST_AUTO_TEST_CASE(range_overlap_tests) { auto get_item(std::string left, std::string right, std::string val) { using value_type = std::unordered_set; - auto l = dht::global_partitioner().from_sstring(left); - auto r = dht::global_partitioner().from_sstring(right); + auto l = dht::token::from_sstring(left); + auto r = dht::token::from_sstring(right); auto rg = dht::token_range({{l, false}}, {r}); value_type v{val}; return std::make_pair(locator::token_metadata::range_to_interval(rg), v); @@ -272,7 +272,7 @@ BOOST_AUTO_TEST_CASE(test_range_interval_map) { } auto search_item = [&mymap] (std::string val) { - auto tok = dht::global_partitioner().from_sstring(val); + auto tok = dht::token::from_sstring(val); auto search = dht::token_range(tok); auto it = mymap.find(locator::token_metadata::range_to_interval(search)); if (it != mymap.end()) { diff --git a/test/boost/range_test.cc b/test/boost/range_test.cc index b753ba8794..66705890b6 100644 --- a/test/boost/range_test.cc +++ b/test/boost/range_test.cc @@ -282,8 +282,8 @@ BOOST_AUTO_TEST_CASE(range_overlap_tests) { auto get_item(std::string left, std::string right, std::string val) { using value_type = std::unordered_set; - auto l = dht::global_partitioner().from_sstring(left); - auto r = dht::global_partitioner().from_sstring(right); + auto l = dht::token::from_sstring(left); + auto r = dht::token::from_sstring(right); auto rg = range({{l, false}}, {r}); value_type v{val}; return std::make_pair(locator::token_metadata::range_to_interval(rg), v); @@ -309,7 +309,7 @@ BOOST_AUTO_TEST_CASE(test_range_interval_map) { } auto search_item = [&mymap] (std::string val) { - auto tok = dht::global_partitioner().from_sstring(val); + auto tok = dht::token::from_sstring(val); auto search = range(tok); auto it = mymap.find(locator::token_metadata::range_to_interval(search)); if (it != mymap.end()) { diff --git a/thrift/handler.cc b/thrift/handler.cc index 29ed94827c..c7ff95b7ab 100644 --- a/thrift/handler.cc +++ b/thrift/handler.cc @@ -803,8 +803,8 @@ public: void describe_splits_ex(thrift_fn::function const& _return)> cob, thrift_fn::function exn_cob, const std::string& cfName, const std::string& start_token, const std::string& end_token, const int32_t keys_per_split) { with_cob(std::move(cob), std::move(exn_cob), [&]{ dht::token_range_vector ranges; - auto tstart = start_token.empty() ? dht::minimum_token() : dht::global_partitioner().from_sstring(sstring(start_token)); - auto tend = end_token.empty() ? dht::maximum_token() : dht::global_partitioner().from_sstring(sstring(end_token)); + auto tstart = start_token.empty() ? dht::minimum_token() : dht::token::from_sstring(sstring(start_token)); + auto tend = end_token.empty() ? dht::maximum_token() : dht::token::from_sstring(sstring(end_token)); range r({{ std::move(tstart), false }}, {{ std::move(tend), true }}); auto cf = sstring(cfName); auto splits = service::get_local_storage_service().get_splits(current_keyspace(), cf, std::move(r), keys_per_split); @@ -1714,7 +1714,7 @@ private: auto start = range.start_key.empty() ? dht::ring_position::starting_at(dht::minimum_token()) : partitioner.decorate_key(s, key_from_thrift(s, to_bytes(range.start_key))); - auto end = dht::ring_position::ending_at(partitioner.from_sstring(sstring(range.end_token))); + auto end = dht::ring_position::ending_at(dht::token::from_sstring(sstring(range.end_token))); if (end.token().is_minimum()) { end = dht::ring_position::ending_at(dht::maximum_token()); } else if (end.less_compare(s, start)) { @@ -1725,8 +1725,8 @@ private: } // Token range can wrap; the start token is exclusive. - auto start = dht::ring_position::ending_at(partitioner.from_sstring(sstring(range.start_token))); - auto end = dht::ring_position::ending_at(partitioner.from_sstring(sstring(range.end_token))); + auto start = dht::ring_position::ending_at(dht::token::from_sstring(sstring(range.start_token))); + auto end = dht::ring_position::ending_at(dht::token::from_sstring(sstring(range.end_token))); if (end.token().is_minimum()) { end = dht::ring_position::ending_at(dht::maximum_token()); }