From 78f40cac8db496c00d398f97d7bde7ed674bd892 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Thu, 13 Aug 2020 13:28:54 +0300 Subject: [PATCH 01/19] token_metadata: add get_datacenter_racks() const variant Needed for passing a const token_metadata& to calculate_natural_endpoints methods. Signed-off-by: Benny Halevy --- locator/token_metadata.hh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/locator/token_metadata.hh b/locator/token_metadata.hh index 6a4822859d..64bd392ffb 100644 --- a/locator/token_metadata.hh +++ b/locator/token_metadata.hh @@ -113,6 +113,13 @@ public: return _dc_racks; } + const std::unordered_map>>& + get_datacenter_racks() const { + return _dc_racks; + } + const endpoint_dc_rack& get_location(const inet_address& ep) const; private: /** multi-map: DC -> endpoints in that DC */ From b4f76cbb8a95a7e212aa677a044698d063d38dba Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Thu, 13 Aug 2020 12:41:48 +0300 Subject: [PATCH 02/19] replication_strategy: calculate_natural_endpoints: make token_metadata& param const No replication strategy needs to change token_metadata when calculating natural endpoints. Signed-off-by: Benny Halevy --- locator/abstract_replication_strategy.hh | 2 +- locator/everywhere_replication_strategy.hh | 2 +- locator/local_strategy.cc | 2 +- locator/local_strategy.hh | 2 +- locator/network_topology_strategy.cc | 8 ++++---- locator/network_topology_strategy.hh | 2 +- locator/simple_strategy.cc | 2 +- locator/simple_strategy.hh | 2 +- 8 files changed, 11 insertions(+), 11 deletions(-) diff --git a/locator/abstract_replication_strategy.hh b/locator/abstract_replication_strategy.hh index 49812c43b8..e5bca829f3 100644 --- a/locator/abstract_replication_strategy.hh +++ b/locator/abstract_replication_strategy.hh @@ -87,7 +87,7 @@ public: snitch_ptr& snitch, const std::map& config_options, replication_strategy_type my_type); - virtual std::vector calculate_natural_endpoints(const token& search_token, token_metadata& tm) const = 0; + virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const = 0; virtual ~abstract_replication_strategy() {} static std::unique_ptr create_replication_strategy(const sstring& ks_name, const sstring& strategy_name, token_metadata& token_metadata, const std::map& config_options); static void validate_replication_strategy(const sstring& ks_name, diff --git a/locator/everywhere_replication_strategy.hh b/locator/everywhere_replication_strategy.hh index 8ca508a156..bfa9862b71 100644 --- a/locator/everywhere_replication_strategy.hh +++ b/locator/everywhere_replication_strategy.hh @@ -46,7 +46,7 @@ class everywhere_replication_strategy : public abstract_replication_strategy { public: everywhere_replication_strategy(const sstring& keyspace_name, token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); - virtual std::vector calculate_natural_endpoints(const token& search_token, token_metadata& tm) const override { + virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const override { return tm.get_all_endpoints(); } std::vector get_natural_endpoints(const token& search_token) override; diff --git a/locator/local_strategy.cc b/locator/local_strategy.cc index 3e6a0bfae6..8105ea0961 100644 --- a/locator/local_strategy.cc +++ b/locator/local_strategy.cc @@ -34,7 +34,7 @@ std::vector local_strategy::get_natural_endpoints(const token& t) return calculate_natural_endpoints(t, _token_metadata); } -std::vector local_strategy::calculate_natural_endpoints(const token& t, token_metadata& tm) const { +std::vector local_strategy::calculate_natural_endpoints(const token& t, const token_metadata& tm) const { return std::vector({utils::fb_utilities::get_broadcast_address()}); } diff --git a/locator/local_strategy.hh b/locator/local_strategy.hh index f39ef213f0..4bbf6c624f 100644 --- a/locator/local_strategy.hh +++ b/locator/local_strategy.hh @@ -36,7 +36,7 @@ using token = dht::token; class local_strategy : public abstract_replication_strategy { protected: - virtual std::vector calculate_natural_endpoints(const token& search_token, token_metadata& tm) const override; + virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const override; public: local_strategy(const sstring& keyspace_name, token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); virtual ~local_strategy() {}; diff --git a/locator/network_topology_strategy.cc b/locator/network_topology_strategy.cc index b188a0ddcd..a58893da8d 100644 --- a/locator/network_topology_strategy.cc +++ b/locator/network_topology_strategy.cc @@ -104,7 +104,7 @@ network_topology_strategy::network_topology_strategy( std::vector network_topology_strategy::calculate_natural_endpoints( - const token& search_token, token_metadata& tm) const { + const token& search_token, const token_metadata& tm) const { using endpoint_set = utils::sequenced_set; using endpoint_dc_rack_set = std::unordered_set; @@ -200,20 +200,20 @@ network_topology_strategy::calculate_natural_endpoints( // tracks the racks we have already placed replicas in endpoint_dc_rack_set seen_racks; - topology& tp = tm.get_topology(); + const topology& tp = tm.get_topology(); // // all endpoints in each DC, so we can check when we have exhausted all // the members of a DC // - std::unordered_map>& all_endpoints = tp.get_datacenter_endpoints(); // // all racks in a DC so we can check when we have exhausted all racks in a // DC // - std::unordered_map>>& racks = tp.get_datacenter_racks(); diff --git a/locator/network_topology_strategy.hh b/locator/network_topology_strategy.hh index 4dd34757b4..d04bfa57e1 100644 --- a/locator/network_topology_strategy.hh +++ b/locator/network_topology_strategy.hh @@ -76,7 +76,7 @@ protected: * progress in each DC, rack etc. */ virtual std::vector calculate_natural_endpoints( - const token& search_token, token_metadata& tm) const override; + const token& search_token, const token_metadata& tm) const override; virtual void validate_options() const override; diff --git a/locator/simple_strategy.cc b/locator/simple_strategy.cc index ebf06b6095..2cdc9480f0 100644 --- a/locator/simple_strategy.cc +++ b/locator/simple_strategy.cc @@ -42,7 +42,7 @@ simple_strategy::simple_strategy(const sstring& keyspace_name, token_metadata& t } } -std::vector simple_strategy::calculate_natural_endpoints(const token& t, token_metadata& tm) const { +std::vector simple_strategy::calculate_natural_endpoints(const token& t, const token_metadata& tm) const { const std::vector& tokens = tm.sorted_tokens(); if (tokens.empty()) { diff --git a/locator/simple_strategy.hh b/locator/simple_strategy.hh index af17187150..af7e9badde 100644 --- a/locator/simple_strategy.hh +++ b/locator/simple_strategy.hh @@ -30,7 +30,7 @@ namespace locator { class simple_strategy : public abstract_replication_strategy { protected: - virtual std::vector calculate_natural_endpoints(const token& search_token, token_metadata& tm) const override; + virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const override; public: simple_strategy(const sstring& keyspace_name, token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); virtual ~simple_strategy() {}; From 23a06259984bc1cd8690bf64c25d4704087bf450 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Thu, 20 Aug 2020 14:45:57 +0300 Subject: [PATCH 03/19] token_ranges: get rid of unused get_pending_ranges variant Signed-off-by: Benny Halevy --- locator/token_metadata.cc | 13 ------------- locator/token_metadata.hh | 3 --- 2 files changed, 16 deletions(-) diff --git a/locator/token_metadata.cc b/locator/token_metadata.cc index feee1f209e..43e93e0ed6 100644 --- a/locator/token_metadata.cc +++ b/locator/token_metadata.cc @@ -459,9 +459,6 @@ private: void set_pending_ranges(const sstring& keyspace_name, std::unordered_multimap, inet_address> new_pending_ranges); public: - /** a mutable map may be returned but caller should not modify it */ - const std::unordered_map, std::unordered_set>& get_pending_ranges(sstring keyspace_name); - std::vector> get_pending_ranges(sstring keyspace_name, inet_address endpoint); /** * Calculate pending ranges according to bootsrapping and leaving nodes. Reasoning is: @@ -1366,11 +1363,6 @@ token_metadata_impl::get_pending_ranges_mm(sstring keyspace_name) { return _pending_ranges[keyspace_name]; } -const std::unordered_map, std::unordered_set>& -token_metadata_impl::get_pending_ranges(sstring keyspace_name) { - return _pending_ranges_map[keyspace_name]; -} - std::vector> token_metadata_impl::get_pending_ranges(sstring keyspace_name, inet_address endpoint) { std::vector> ret; @@ -1875,11 +1867,6 @@ token_metadata::interval_to_range(boost::icl::interval::interval_type i) return token_metadata_impl::interval_to_range(std::move(i)); } -const std::unordered_map, std::unordered_set>& -token_metadata::get_pending_ranges(sstring keyspace_name) { - return _impl->get_pending_ranges(std::move(keyspace_name)); -} - std::vector> token_metadata::get_pending_ranges(sstring keyspace_name, inet_address endpoint) { return _impl->get_pending_ranges(std::move(keyspace_name), endpoint); diff --git a/locator/token_metadata.hh b/locator/token_metadata.hh index 64bd392ffb..5ac2d31de5 100644 --- a/locator/token_metadata.hh +++ b/locator/token_metadata.hh @@ -269,9 +269,6 @@ public: static boost::icl::interval::interval_type range_to_interval(range r); static range interval_to_range(boost::icl::interval::interval_type i); - /** a mutable map may be returned but caller should not modify it */ - const std::unordered_map, std::unordered_set>& get_pending_ranges(sstring keyspace_name); - std::vector> get_pending_ranges(sstring keyspace_name, inet_address endpoint); /** * Calculate pending ranges according to bootsrapping and leaving nodes. Reasoning is: From ca61c2797a63b540109ccdbc294c6025b33f31c3 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Thu, 20 Aug 2020 15:06:59 +0300 Subject: [PATCH 04/19] token_ranges: get_pending_ranges: return empty vector if keyspace not found Rather than creating a bogus empty entry. With that, it can be marked as const. Signed-off-by: Benny Halevy --- locator/token_metadata.cc | 18 ++++++++---------- locator/token_metadata.hh | 3 ++- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/locator/token_metadata.cc b/locator/token_metadata.cc index 43e93e0ed6..a4c195e61e 100644 --- a/locator/token_metadata.cc +++ b/locator/token_metadata.cc @@ -455,11 +455,11 @@ public: static range interval_to_range(boost::icl::interval::interval_type i); private: - std::unordered_multimap, inet_address>& get_pending_ranges_mm(sstring keyspace_name); void set_pending_ranges(const sstring& keyspace_name, std::unordered_multimap, inet_address> new_pending_ranges); public: - std::vector> get_pending_ranges(sstring keyspace_name, inet_address endpoint); + // returns an empty vector if keyspace_name not found + std::vector> get_pending_ranges(sstring keyspace_name, inet_address endpoint) const; /** * Calculate pending ranges according to bootsrapping and leaving nodes. Reasoning is: * @@ -1358,20 +1358,18 @@ void token_metadata_impl::set_pending_ranges(const sstring& keyspace_name, _pending_ranges_map[keyspace_name] = std::move(map); } -std::unordered_multimap, inet_address>& -token_metadata_impl::get_pending_ranges_mm(sstring keyspace_name) { - return _pending_ranges[keyspace_name]; -} - std::vector> -token_metadata_impl::get_pending_ranges(sstring keyspace_name, inet_address endpoint) { +token_metadata_impl::get_pending_ranges(sstring keyspace_name, inet_address endpoint) const { std::vector> ret; - for (auto x : get_pending_ranges_mm(keyspace_name)) { + const auto it = _pending_ranges.find(keyspace_name); + if (it != _pending_ranges.end()) { + for (auto x : it->second) { auto& range_token = x.first; auto& ep = x.second; if (ep == endpoint) { ret.push_back(range_token); } + } } return ret; } @@ -1868,7 +1866,7 @@ token_metadata::interval_to_range(boost::icl::interval::interval_type i) } std::vector> -token_metadata::get_pending_ranges(sstring keyspace_name, inet_address endpoint) { +token_metadata::get_pending_ranges(sstring keyspace_name, inet_address endpoint) const { return _impl->get_pending_ranges(std::move(keyspace_name), endpoint); } diff --git a/locator/token_metadata.hh b/locator/token_metadata.hh index 5ac2d31de5..e011c88fa3 100644 --- a/locator/token_metadata.hh +++ b/locator/token_metadata.hh @@ -269,7 +269,8 @@ public: static boost::icl::interval::interval_type range_to_interval(range r); static range interval_to_range(boost::icl::interval::interval_type i); - std::vector> get_pending_ranges(sstring keyspace_name, inet_address endpoint); + // returns an empty vector if keyspace_name not found + std::vector> get_pending_ranges(sstring keyspace_name, inet_address endpoint) const; /** * Calculate pending ranges according to bootsrapping and leaving nodes. Reasoning is: * From 65d89512d0372402aca9d3fd2f16ca0e9d150822 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Thu, 20 Aug 2020 15:06:59 +0300 Subject: [PATCH 05/19] token_ranges: pending_endpoints_for: return empty vector if keyspace not found Rather than creating a bogus empty entry. With that, it can be marked as const. Signed-off-by: Benny Halevy --- locator/token_metadata.cc | 21 ++++++++++++--------- locator/token_metadata.hh | 3 ++- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/locator/token_metadata.cc b/locator/token_metadata.cc index a4c195e61e..74bc11b0b3 100644 --- a/locator/token_metadata.cc +++ b/locator/token_metadata.cc @@ -729,7 +729,8 @@ public: #endif sstring print_pending_ranges(); public: - std::vector pending_endpoints_for(const token& token, const sstring& keyspace_name); + // returns empty vector if keyspace_name not found. + std::vector pending_endpoints_for(const token& token, const sstring& keyspace_name) const; #if 0 /** * @deprecated retained for benefit of old tests @@ -1553,22 +1554,24 @@ token_metadata_impl token_metadata_impl::clone_after_all_settled() { return metadata; } -std::vector token_metadata_impl::pending_endpoints_for(const token& token, const sstring& keyspace_name) { - // Fast path 0: no pending ranges at all - if (_pending_ranges_interval_map.empty()) { +std::vector token_metadata_impl::pending_endpoints_for(const token& token, const sstring& keyspace_name) const { + // Fast path 0: pending ranges not found for this keyspace_name + const auto pr_it = _pending_ranges_interval_map.find(keyspace_name); + if (pr_it == _pending_ranges_interval_map.end()) { return {}; } - // Fast path 1: no pending ranges for this keyspace_name - if (_pending_ranges_interval_map[keyspace_name].empty()) { + // Fast path 1: empty pending ranges for this keyspace_name + const auto& ks_map = pr_it->second; + if (ks_map.empty()) { return {}; } // Slow path: lookup pending ranges std::vector endpoints; auto interval = range_to_interval(range(token)); - auto it = _pending_ranges_interval_map[keyspace_name].find(interval); - if (it != _pending_ranges_interval_map[keyspace_name].end()) { + const auto it = ks_map.find(interval); + if (it != ks_map.end()) { // interval_map does not work with std::vector, convert to std::vector of ips endpoints = std::vector(it->second.begin(), it->second.end()); } @@ -1917,7 +1920,7 @@ token_metadata::print_pending_ranges() { } std::vector -token_metadata::pending_endpoints_for(const token& token, const sstring& keyspace_name) { +token_metadata::pending_endpoints_for(const token& token, const sstring& keyspace_name) const { return _impl->pending_endpoints_for(token, keyspace_name); } diff --git a/locator/token_metadata.hh b/locator/token_metadata.hh index e011c88fa3..92ed755795 100644 --- a/locator/token_metadata.hh +++ b/locator/token_metadata.hh @@ -318,7 +318,8 @@ public: sstring print_pending_ranges(); - std::vector pending_endpoints_for(const token& token, const sstring& keyspace_name); + // returns empty vector if keyspace_name not found. + std::vector pending_endpoints_for(const token& token, const sstring& keyspace_name) const; /** @return an endpoint to token multimap representation of tokenToEndpointMap (a copy) */ std::multimap get_endpoint_to_token_map_for_reading(); From 22275e579ec2bdf6c40d077d8e0c5ec1a0a727b1 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 08:47:44 +0300 Subject: [PATCH 06/19] token_metadata: mark const methods Many token_metadata methods do not modify the object and can be marked as const. The motivation is to better control who may modify token_metadata. Signed-off-by: Benny Halevy --- locator/abstract_replication_strategy.cc | 8 +- locator/abstract_replication_strategy.hh | 8 +- locator/token_metadata.cc | 130 +++++++++++------------ locator/token_metadata.hh | 40 +++---- 4 files changed, 93 insertions(+), 93 deletions(-) diff --git a/locator/abstract_replication_strategy.cc b/locator/abstract_replication_strategy.cc index 9816b076a2..d0102d3cbd 100644 --- a/locator/abstract_replication_strategy.cc +++ b/locator/abstract_replication_strategy.cc @@ -244,7 +244,7 @@ abstract_replication_strategy::get_primary_ranges_within_dc(inet_address ep) { } std::unordered_multimap -abstract_replication_strategy::get_address_ranges(token_metadata& tm) const { +abstract_replication_strategy::get_address_ranges(const token_metadata& tm) const { std::unordered_multimap ret; for (auto& t : tm.sorted_tokens()) { dht::token_range_vector r = tm.get_primary_ranges_for(t); @@ -260,7 +260,7 @@ abstract_replication_strategy::get_address_ranges(token_metadata& tm) const { } std::unordered_map> -abstract_replication_strategy::get_range_addresses(token_metadata& tm) const { +abstract_replication_strategy::get_range_addresses(const token_metadata& tm) const { std::unordered_map> ret; for (auto& t : tm.sorted_tokens()) { dht::token_range_vector ranges = tm.get_primary_ranges_for(t); @@ -273,12 +273,12 @@ abstract_replication_strategy::get_range_addresses(token_metadata& tm) const { } dht::token_range_vector -abstract_replication_strategy::get_pending_address_ranges(token_metadata& tm, token pending_token, inet_address pending_address) { +abstract_replication_strategy::get_pending_address_ranges(const token_metadata& tm, token pending_token, inet_address pending_address) const { return get_pending_address_ranges(tm, std::unordered_set{pending_token}, pending_address); } dht::token_range_vector -abstract_replication_strategy::get_pending_address_ranges(token_metadata& tm, std::unordered_set pending_tokens, inet_address pending_address) { +abstract_replication_strategy::get_pending_address_ranges(const token_metadata& tm, std::unordered_set pending_tokens, inet_address pending_address) const { dht::token_range_vector ret; auto temp = tm.clone_only_token_map(); temp.update_normal_tokens(pending_tokens, pending_address); diff --git a/locator/abstract_replication_strategy.hh b/locator/abstract_replication_strategy.hh index e5bca829f3..3e49adc6a0 100644 --- a/locator/abstract_replication_strategy.hh +++ b/locator/abstract_replication_strategy.hh @@ -134,13 +134,13 @@ public: // instead of one node globally. dht::token_range_vector get_primary_ranges_within_dc(inet_address ep); - std::unordered_multimap get_address_ranges(token_metadata& tm) const; + std::unordered_multimap get_address_ranges(const token_metadata& tm) const; - std::unordered_map> get_range_addresses(token_metadata& tm) const; + std::unordered_map> get_range_addresses(const token_metadata& tm) const; - dht::token_range_vector get_pending_address_ranges(token_metadata& tm, token pending_token, inet_address pending_address); + dht::token_range_vector get_pending_address_ranges(const token_metadata& tm, token pending_token, inet_address pending_address) const; - dht::token_range_vector get_pending_address_ranges(token_metadata& tm, std::unordered_set pending_tokens, inet_address pending_address); + dht::token_range_vector get_pending_address_ranges(const token_metadata& tm, std::unordered_set pending_tokens, inet_address pending_address) const; }; } diff --git a/locator/token_metadata.cc b/locator/token_metadata.cc index 74bc11b0b3..5b30338d16 100644 --- a/locator/token_metadata.cc +++ b/locator/token_metadata.cc @@ -131,7 +131,7 @@ public: return _topology; } - void debug_show(); + void debug_show() const; #if 0 private static final Logger logger = LoggerFactory.getLogger(TokenMetadata.class); @@ -357,15 +357,15 @@ public: #endif - bool is_member(inet_address endpoint); + bool is_member(inet_address endpoint) const; - bool is_leaving(inet_address endpoint); + bool is_leaving(inet_address endpoint) const; // Is this node being replaced by another node - bool is_being_replaced(inet_address endpoint); + bool is_being_replaced(inet_address endpoint) const; // Is any node being replaced by another node - bool is_any_node_being_replaced(); + bool is_any_node_being_replaced() const; void add_replacing_endpoint(inet_address existing_node, inet_address replacing_node); @@ -379,7 +379,7 @@ public: * Create a copy of TokenMetadata with only tokenToEndpointMap. That is, pending ranges, * bootstrap tokens and leaving endpoints are not included in the copy. */ - token_metadata_impl clone_only_token_map() { + token_metadata_impl clone_only_token_map() const { return token_metadata_impl(this->_token_to_endpoint_map, this->_endpoint_to_host_id_map, this->_topology); } #if 0 @@ -415,7 +415,7 @@ public: * * @return new token metadata */ - token_metadata_impl clone_after_all_left() { + token_metadata_impl clone_after_all_left() const { auto all_left_metadata = clone_only_token_map(); for (auto endpoint : _leaving_endpoints) { @@ -432,7 +432,7 @@ public: * * @return new token metadata */ - token_metadata_impl clone_after_all_settled(); + token_metadata_impl clone_after_all_settled() const; #if 0 public InetAddress getEndpoint(Token token) { @@ -448,9 +448,9 @@ public: } #endif public: - dht::token_range_vector get_primary_ranges_for(std::unordered_set tokens); + dht::token_range_vector get_primary_ranges_for(std::unordered_set tokens) const; - dht::token_range_vector get_primary_ranges_for(token right); + dht::token_range_vector get_primary_ranges_for(token right) const; static boost::icl::interval::interval_type range_to_interval(range r); static range interval_to_range(boost::icl::interval::interval_type i); @@ -484,24 +484,24 @@ public: * changes state in the cluster, so it should be manageable. */ future<> calculate_pending_ranges( - token_metadata& unpimplified_this, - abstract_replication_strategy& strategy, const sstring& keyspace_name); + const token_metadata& unpimplified_this, + const abstract_replication_strategy& strategy, const sstring& keyspace_name); future<> calculate_pending_ranges_for_leaving( - token_metadata& unpimplified_this, - abstract_replication_strategy& strategy, + const token_metadata& unpimplified_this, + const abstract_replication_strategy& strategy, lw_shared_ptr, inet_address>> new_pending_ranges, - lw_shared_ptr all_left_metadata); + lw_shared_ptr all_left_metadata) const; void calculate_pending_ranges_for_bootstrap( - abstract_replication_strategy& strategy, + const abstract_replication_strategy& strategy, lw_shared_ptr, inet_address>> new_pending_ranges, - lw_shared_ptr all_left_metadata); + lw_shared_ptr all_left_metadata) const; future<> calculate_pending_ranges_for_replacing( - token_metadata& unpimplified_this, - abstract_replication_strategy& strategy, - lw_shared_ptr, inet_address>> new_pending_ranges); + const token_metadata& unpimplified_this, + const abstract_replication_strategy& strategy, + lw_shared_ptr, inet_address>> new_pending_ranges) const; public: - token get_predecessor(token t); + token get_predecessor(token t) const; #if 0 public Token getSuccessor(Token token) @@ -727,7 +727,7 @@ public: return sb.toString(); } #endif - sstring print_pending_ranges(); + sstring print_pending_ranges() const; public: // returns empty vector if keyspace_name not found. std::vector pending_endpoints_for(const token& token, const sstring& keyspace_name) const; @@ -743,12 +743,12 @@ public: public: /** @return an endpoint to token multimap representation of tokenToEndpointMap (a copy) */ - std::multimap get_endpoint_to_token_map_for_reading(); + std::multimap get_endpoint_to_token_map_for_reading() const; /** * @return a (stable copy, won't be modified) Token to Endpoint map for all the normal and bootstrapping nodes * in the cluster. */ - std::map get_normal_and_bootstrapping_token_to_endpoint_map(); + std::map get_normal_and_bootstrapping_token_to_endpoint_map() const; #if 0 /** @@ -1087,7 +1087,7 @@ std::optional token_metadata_impl::get_endpoint(const token& token } } -void token_metadata_impl::debug_show() { +void token_metadata_impl::debug_show() const { auto reporter = std::make_shared>(); reporter->set_callback ([reporter, this] { fmt::print("Endpoint -> Token\n"); @@ -1160,7 +1160,7 @@ const std::unordered_map& token_metadata_impl::get_en return _endpoint_to_host_id_map; } -bool token_metadata_impl::is_member(inet_address endpoint) { +bool token_metadata_impl::is_member(inet_address endpoint) const { return _topology.has_endpoint(endpoint); } @@ -1229,15 +1229,15 @@ void token_metadata_impl::remove_bootstrap_tokens(std::unordered_set toke } } -bool token_metadata_impl::is_leaving(inet_address endpoint) { +bool token_metadata_impl::is_leaving(inet_address endpoint) const { return _leaving_endpoints.contains(endpoint); } -bool token_metadata_impl::is_being_replaced(inet_address endpoint) { +bool token_metadata_impl::is_being_replaced(inet_address endpoint) const { return _replacing_endpoints.contains(endpoint); } -bool token_metadata_impl::is_any_node_being_replaced() { +bool token_metadata_impl::is_any_node_being_replaced() const { return !_replacing_endpoints.empty(); } @@ -1252,7 +1252,7 @@ void token_metadata_impl::remove_endpoint(inet_address endpoint) { invalidate_cached_rings(); } -token token_metadata_impl::get_predecessor(token t) { +token token_metadata_impl::get_predecessor(token t) const { auto& tokens = sorted_tokens(); auto it = std::lower_bound(tokens.begin(), tokens.end(), t); if (it == tokens.end() || *it != t) { @@ -1268,7 +1268,7 @@ token token_metadata_impl::get_predecessor(token t) { } } -dht::token_range_vector token_metadata_impl::get_primary_ranges_for(std::unordered_set tokens) { +dht::token_range_vector token_metadata_impl::get_primary_ranges_for(std::unordered_set tokens) const { dht::token_range_vector ranges; ranges.reserve(tokens.size() + 1); // one of the ranges will wrap for (auto right : tokens) { @@ -1281,7 +1281,7 @@ dht::token_range_vector token_metadata_impl::get_primary_ranges_for(std::unorder return ranges; } -dht::token_range_vector token_metadata_impl::get_primary_ranges_for(token right) { +dht::token_range_vector token_metadata_impl::get_primary_ranges_for(token right) const { return get_primary_ranges_for(std::unordered_set{right}); } @@ -1376,10 +1376,10 @@ token_metadata_impl::get_pending_ranges(sstring keyspace_name, inet_address endp } future<> token_metadata_impl::calculate_pending_ranges_for_leaving( - token_metadata& unpimplified_this, - abstract_replication_strategy& strategy, + const token_metadata& unpimplified_this, + const abstract_replication_strategy& strategy, lw_shared_ptr, inet_address>> new_pending_ranges, - lw_shared_ptr all_left_metadata) { + lw_shared_ptr all_left_metadata) const { std::unordered_multimap address_ranges = strategy.get_address_ranges(unpimplified_this); // get all ranges that will be affected by leaving nodes std::unordered_set> affected_ranges; @@ -1414,9 +1414,9 @@ future<> token_metadata_impl::calculate_pending_ranges_for_leaving( } future<> token_metadata_impl::calculate_pending_ranges_for_replacing( - token_metadata& unpimplified_this, - abstract_replication_strategy& strategy, - lw_shared_ptr, inet_address>> new_pending_ranges) { + const token_metadata& unpimplified_this, + const abstract_replication_strategy& strategy, + lw_shared_ptr, inet_address>> new_pending_ranges) const { if (_replacing_endpoints.empty()) { return make_ready_future<>(); } @@ -1439,9 +1439,9 @@ future<> token_metadata_impl::calculate_pending_ranges_for_replacing( } void token_metadata_impl::calculate_pending_ranges_for_bootstrap( - abstract_replication_strategy& strategy, + const abstract_replication_strategy& strategy, lw_shared_ptr, inet_address>> new_pending_ranges, - lw_shared_ptr all_left_metadata) { + lw_shared_ptr all_left_metadata) const { // For each of the bootstrapping nodes, simply add and remove them one by one to // allLeftMetadata and check in between what their ranges would be. std::unordered_multimap bootstrap_addresses; @@ -1470,8 +1470,8 @@ void token_metadata_impl::calculate_pending_ranges_for_bootstrap( } future<> token_metadata_impl::calculate_pending_ranges( - token_metadata& unpimplified_this, - abstract_replication_strategy& strategy, const sstring& keyspace_name) { + const token_metadata& unpimplified_this, + const abstract_replication_strategy& strategy, const sstring& keyspace_name) { auto new_pending_ranges = make_lw_shared, inet_address>>(); tlogger.debug("calculate_pending_ranges: keyspace_name={}, bootstrap_tokens={}, leaving nodes={}, replacing_endpoints={}", @@ -1511,7 +1511,7 @@ size_t token_metadata_impl::count_normal_token_owners() const { return eps.size(); } -sstring token_metadata_impl::print_pending_ranges() { +sstring token_metadata_impl::print_pending_ranges() const { std::stringstream ss; for (auto& x : _pending_ranges) { @@ -1544,7 +1544,7 @@ void token_metadata_impl::del_replacing_endpoint(inet_address existing_node) { _replacing_endpoints.erase(existing_node); } -token_metadata_impl token_metadata_impl::clone_after_all_settled() { +token_metadata_impl token_metadata_impl::clone_after_all_settled() const { token_metadata_impl metadata = clone_only_token_map(); for (auto endpoint : _leaving_endpoints) { @@ -1578,13 +1578,13 @@ std::vector token_metadata_impl::pending_endpoints_for(const return endpoints; } -std::map token_metadata_impl::get_normal_and_bootstrapping_token_to_endpoint_map() { +std::map token_metadata_impl::get_normal_and_bootstrapping_token_to_endpoint_map() const { std::map ret(_token_to_endpoint_map.begin(), _token_to_endpoint_map.end()); ret.insert(_bootstrap_tokens.begin(), _bootstrap_tokens.end()); return ret; } -std::multimap token_metadata_impl::get_endpoint_to_token_map_for_reading() { +std::multimap token_metadata_impl::get_endpoint_to_token_map_for_reading() const { std::multimap cloned; for (const auto& x : _token_to_endpoint_map) { cloned.emplace(x.second, x.first); @@ -1751,7 +1751,7 @@ token_metadata::get_topology() const { } void -token_metadata::debug_show() { +token_metadata::debug_show() const { _impl->debug_show(); } @@ -1806,22 +1806,22 @@ token_metadata::remove_endpoint(inet_address endpoint) { } bool -token_metadata::is_member(inet_address endpoint) { +token_metadata::is_member(inet_address endpoint) const { return _impl->is_member(endpoint); } bool -token_metadata::is_leaving(inet_address endpoint) { +token_metadata::is_leaving(inet_address endpoint) const { return _impl->is_leaving(endpoint); } bool -token_metadata::is_being_replaced(inet_address endpoint) { +token_metadata::is_being_replaced(inet_address endpoint) const { return _impl->is_being_replaced(endpoint); } bool -token_metadata::is_any_node_being_replaced() { +token_metadata::is_any_node_being_replaced() const { return _impl->is_any_node_being_replaced(); } @@ -1834,27 +1834,27 @@ void token_metadata::del_replacing_endpoint(inet_address existing_node) { } token_metadata -token_metadata::clone_only_token_map() { +token_metadata::clone_only_token_map() const { return token_metadata(std::make_unique(_impl->clone_only_token_map())); } token_metadata -token_metadata::clone_after_all_left() { +token_metadata::clone_after_all_left() const { return token_metadata(std::make_unique(_impl->clone_after_all_left())); } token_metadata -token_metadata::clone_after_all_settled() { +token_metadata::clone_after_all_settled() const { return token_metadata(std::make_unique(_impl->clone_after_all_settled())); } dht::token_range_vector -token_metadata::get_primary_ranges_for(std::unordered_set tokens) { +token_metadata::get_primary_ranges_for(std::unordered_set tokens) const { return _impl->get_primary_ranges_for(std::move(tokens)); } dht::token_range_vector -token_metadata::get_primary_ranges_for(token right) { +token_metadata::get_primary_ranges_for(token right) const { return _impl->get_primary_ranges_for(right); } @@ -1874,28 +1874,28 @@ token_metadata::get_pending_ranges(sstring keyspace_name, inet_address endpoint) } future<> -token_metadata::calculate_pending_ranges(abstract_replication_strategy& strategy, const sstring& keyspace_name) { +token_metadata::calculate_pending_ranges(const abstract_replication_strategy& strategy, const sstring& keyspace_name) { return _impl->calculate_pending_ranges(*this, strategy, keyspace_name); } future<> token_metadata::calculate_pending_ranges_for_leaving( - abstract_replication_strategy& strategy, + const abstract_replication_strategy& strategy, lw_shared_ptr, inet_address>> new_pending_ranges, - lw_shared_ptr all_left_metadata) { + lw_shared_ptr all_left_metadata) const { return _impl->calculate_pending_ranges_for_leaving(*this, strategy, std::move(new_pending_ranges), std::move(all_left_metadata)); } void token_metadata::calculate_pending_ranges_for_bootstrap( - abstract_replication_strategy& strategy, + const abstract_replication_strategy& strategy, lw_shared_ptr, inet_address>> new_pending_ranges, - lw_shared_ptr all_left_metadata) { + lw_shared_ptr all_left_metadata) const { _impl->calculate_pending_ranges_for_bootstrap(strategy, std::move(new_pending_ranges), std::move(all_left_metadata)); } token -token_metadata::get_predecessor(token t) { +token_metadata::get_predecessor(token t) const { return _impl->get_predecessor(t); } @@ -1915,7 +1915,7 @@ token_metadata::count_normal_token_owners() const { } sstring -token_metadata::print_pending_ranges() { +token_metadata::print_pending_ranges() const { return _impl->print_pending_ranges(); } @@ -1925,12 +1925,12 @@ token_metadata::pending_endpoints_for(const token& token, const sstring& keyspac } std::multimap -token_metadata::get_endpoint_to_token_map_for_reading() { +token_metadata::get_endpoint_to_token_map_for_reading() const { return _impl->get_endpoint_to_token_map_for_reading(); } std::map -token_metadata::get_normal_and_bootstrapping_token_to_endpoint_map() { +token_metadata::get_normal_and_bootstrapping_token_to_endpoint_map() const { return _impl->get_normal_and_bootstrapping_token_to_endpoint_map(); } diff --git a/locator/token_metadata.hh b/locator/token_metadata.hh index 92ed755795..2fa81ef299 100644 --- a/locator/token_metadata.hh +++ b/locator/token_metadata.hh @@ -197,7 +197,7 @@ public: topology& get_topology(); const topology& get_topology() const; - void debug_show(); + void debug_show() const; /** * Store an end-point to host ID mapping. Each ID must be unique, and @@ -230,15 +230,15 @@ public: void remove_endpoint(inet_address endpoint); - bool is_member(inet_address endpoint); + bool is_member(inet_address endpoint) const; - bool is_leaving(inet_address endpoint); + bool is_leaving(inet_address endpoint) const; // Is this node being replaced by another node - bool is_being_replaced(inet_address endpoint); + bool is_being_replaced(inet_address endpoint) const; // Is any node being replaced by another node - bool is_any_node_being_replaced(); + bool is_any_node_being_replaced() const; void add_replacing_endpoint(inet_address existing_node, inet_address replacing_node); @@ -248,24 +248,24 @@ public: * Create a copy of TokenMetadata with only tokenToEndpointMap. That is, pending ranges, * bootstrap tokens and leaving endpoints are not included in the copy. */ - token_metadata clone_only_token_map(); + token_metadata clone_only_token_map() const; /** * Create a copy of TokenMetadata with tokenToEndpointMap reflecting situation after all * current leave operations have finished. * * @return new token metadata */ - token_metadata clone_after_all_left(); + token_metadata clone_after_all_left() const; /** * Create a copy of TokenMetadata with tokenToEndpointMap reflecting situation after all * current leave, and move operations have finished. * * @return new token metadata */ - token_metadata clone_after_all_settled(); - dht::token_range_vector get_primary_ranges_for(std::unordered_set tokens); + token_metadata clone_after_all_settled() const; + dht::token_range_vector get_primary_ranges_for(std::unordered_set tokens) const; - dht::token_range_vector get_primary_ranges_for(token right); + dht::token_range_vector get_primary_ranges_for(token right) const; static boost::icl::interval::interval_type range_to_interval(range r); static range interval_to_range(boost::icl::interval::interval_type i); @@ -294,20 +294,20 @@ public: * NOTE: This is heavy and ineffective operation. This will be done only once when a node * changes state in the cluster, so it should be manageable. */ - future<> calculate_pending_ranges(abstract_replication_strategy& strategy, const sstring& keyspace_name); + future<> calculate_pending_ranges(const abstract_replication_strategy& strategy, const sstring& keyspace_name); future<> calculate_pending_ranges_for_leaving( - abstract_replication_strategy& strategy, + const abstract_replication_strategy& strategy, lw_shared_ptr, inet_address>> new_pending_ranges, - lw_shared_ptr all_left_metadata); + lw_shared_ptr all_left_metadata) const; void calculate_pending_ranges_for_bootstrap( - abstract_replication_strategy& strategy, + const abstract_replication_strategy& strategy, lw_shared_ptr, inet_address>> new_pending_ranges, - lw_shared_ptr all_left_metadata); + lw_shared_ptr all_left_metadata) const; future<> calculate_pending_ranges_for_replacing( abstract_replication_strategy& strategy, - lw_shared_ptr, inet_address>> new_pending_ranges); + lw_shared_ptr, inet_address>> new_pending_ranges) const; - token get_predecessor(token t); + token get_predecessor(token t) const; std::vector get_all_endpoints() const; size_t get_all_endpoints_count() const; @@ -317,17 +317,17 @@ public: size_t count_normal_token_owners() const; - sstring print_pending_ranges(); + sstring print_pending_ranges() const; // returns empty vector if keyspace_name not found. std::vector pending_endpoints_for(const token& token, const sstring& keyspace_name) const; /** @return an endpoint to token multimap representation of tokenToEndpointMap (a copy) */ - std::multimap get_endpoint_to_token_map_for_reading(); + std::multimap get_endpoint_to_token_map_for_reading() const; /** * @return a (stable copy, won't be modified) Token to Endpoint map for all the normal and bootstrapping nodes * in the cluster. */ - std::map get_normal_and_bootstrapping_token_to_endpoint_map(); + std::map get_normal_and_bootstrapping_token_to_endpoint_map() const; long get_ring_version() const; void invalidate_cached_rings(); From 8b63523fb750a14f3e8eba874668f099c8ce0a13 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 09:45:25 +0300 Subject: [PATCH 07/19] token_metadata: rename calculate_pending_ranges to update_pending_ranges Since it sets the token_metadata_impl's pending ranges. Signed-off-by: Benny Halevy --- locator/token_metadata.cc | 8 ++++---- locator/token_metadata.hh | 2 +- service/storage_service.cc | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/locator/token_metadata.cc b/locator/token_metadata.cc index 5b30338d16..973d951439 100644 --- a/locator/token_metadata.cc +++ b/locator/token_metadata.cc @@ -483,7 +483,7 @@ public: * NOTE: This is heavy and ineffective operation. This will be done only once when a node * changes state in the cluster, so it should be manageable. */ - future<> calculate_pending_ranges( + future<> update_pending_ranges( const token_metadata& unpimplified_this, const abstract_replication_strategy& strategy, const sstring& keyspace_name); future<> calculate_pending_ranges_for_leaving( @@ -1469,7 +1469,7 @@ void token_metadata_impl::calculate_pending_ranges_for_bootstrap( } } -future<> token_metadata_impl::calculate_pending_ranges( +future<> token_metadata_impl::update_pending_ranges( const token_metadata& unpimplified_this, const abstract_replication_strategy& strategy, const sstring& keyspace_name) { auto new_pending_ranges = make_lw_shared, inet_address>>(); @@ -1874,8 +1874,8 @@ token_metadata::get_pending_ranges(sstring keyspace_name, inet_address endpoint) } future<> -token_metadata::calculate_pending_ranges(const abstract_replication_strategy& strategy, const sstring& keyspace_name) { - return _impl->calculate_pending_ranges(*this, strategy, keyspace_name); +token_metadata::update_pending_ranges(const abstract_replication_strategy& strategy, const sstring& keyspace_name) { + return _impl->update_pending_ranges(*this, strategy, keyspace_name); } future<> diff --git a/locator/token_metadata.hh b/locator/token_metadata.hh index 2fa81ef299..cf420fcdfe 100644 --- a/locator/token_metadata.hh +++ b/locator/token_metadata.hh @@ -294,7 +294,7 @@ public: * NOTE: This is heavy and ineffective operation. This will be done only once when a node * changes state in the cluster, so it should be manageable. */ - future<> calculate_pending_ranges(const abstract_replication_strategy& strategy, const sstring& keyspace_name); + future<> update_pending_ranges(const abstract_replication_strategy& strategy, const sstring& keyspace_name); future<> calculate_pending_ranges_for_leaving( const abstract_replication_strategy& strategy, lw_shared_ptr, inet_address>> new_pending_ranges, diff --git a/service/storage_service.cc b/service/storage_service.cc index 858a31f60d..0fcef7c598 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -2780,9 +2780,9 @@ future<> storage_service::do_update_pending_ranges() { return do_for_each(keyspaces, [this] (auto& keyspace_name) { auto& ks = this->_db.local().find_keyspace(keyspace_name); auto& strategy = ks.get_replication_strategy(); - slogger.debug("Calculating pending ranges for keyspace={} starts", keyspace_name); - return get_token_metadata().calculate_pending_ranges(strategy, keyspace_name).finally([&keyspace_name] { - slogger.debug("Calculating pending ranges for keyspace={} ends", keyspace_name); + slogger.debug("Updating pending ranges for keyspace={} starts", keyspace_name); + return get_token_metadata().update_pending_ranges(strategy, keyspace_name).finally([&keyspace_name] { + slogger.debug("Updating pending ranges for keyspace={} ends", keyspace_name); }); }); }); @@ -2792,7 +2792,7 @@ future<> storage_service::do_update_pending_ranges() { future<> storage_service::update_pending_ranges() { return get_storage_service().invoke_on(0, [] (auto& ss){ return ss._update_pending_ranges_action.trigger_later().then([&ss] { - // calculate_pending_ranges will modify token_metadata, we need to repliate to other cores + // update_pending_ranges will modify token_metadata, we need to replicate to other cores return ss.replicate_to_all_cores().then([s = ss.shared_from_this()] { }); }); }); From 2fd59e8bba43ef7f168166847b9411b469a52e55 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Thu, 13 Aug 2020 14:47:16 +0300 Subject: [PATCH 08/19] abstract_replication_strategy: get_ranges: accept const token_metadata& Now that calculate_natural_endpoints can be passed a const token_metadata& Signed-off-by: Benny Halevy --- locator/abstract_replication_strategy.cc | 6 +++--- locator/abstract_replication_strategy.hh | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/locator/abstract_replication_strategy.cc b/locator/abstract_replication_strategy.cc index d0102d3cbd..6b75525053 100644 --- a/locator/abstract_replication_strategy.cc +++ b/locator/abstract_replication_strategy.cc @@ -177,17 +177,17 @@ abstract_replication_strategy::get_ranges_in_thread(inet_address ep) const { } dht::token_range_vector -abstract_replication_strategy::get_ranges(inet_address ep, token_metadata& tm) const { +abstract_replication_strategy::get_ranges(inet_address ep, const token_metadata& tm) const { return do_get_ranges(ep, tm, false); } dht::token_range_vector -abstract_replication_strategy::get_ranges_in_thread(inet_address ep, token_metadata& tm) const { +abstract_replication_strategy::get_ranges_in_thread(inet_address ep, const token_metadata& tm) const { return do_get_ranges(ep, tm, true); } dht::token_range_vector -abstract_replication_strategy::do_get_ranges(inet_address ep, token_metadata& tm, bool can_yield) const { +abstract_replication_strategy::do_get_ranges(inet_address ep, const token_metadata& tm, bool can_yield) const { dht::token_range_vector ret; auto prev_tok = tm.sorted_tokens().back(); for (auto tok : tm.sorted_tokens()) { diff --git a/locator/abstract_replication_strategy.hh b/locator/abstract_replication_strategy.hh index 3e49adc6a0..48611ddbda 100644 --- a/locator/abstract_replication_strategy.hh +++ b/locator/abstract_replication_strategy.hh @@ -116,10 +116,10 @@ public: dht::token_range_vector get_ranges_in_thread(inet_address ep) const; // Use the token_metadata provided by the caller instead of _token_metadata - dht::token_range_vector get_ranges(inet_address ep, token_metadata& tm) const; - dht::token_range_vector get_ranges_in_thread(inet_address ep, token_metadata& tm) const; + dht::token_range_vector get_ranges(inet_address ep, const token_metadata& tm) const; + dht::token_range_vector get_ranges_in_thread(inet_address ep, const token_metadata& tm) const; private: - dht::token_range_vector do_get_ranges(inet_address ep, token_metadata& tm, bool can_yield) const; + dht::token_range_vector do_get_ranges(inet_address ep, const token_metadata& tm, bool can_yield) const; public: // get_primary_ranges() returns the list of "primary ranges" for the given From 4dba81cb92cb3608797607a4a7e5cf05e6421eae Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 13:22:44 +0300 Subject: [PATCH 09/19] replication_strategy: keep a const token_metadata& replication strategies don't need to change token_metadata. Signed-off-by: Benny Halevy --- database.cc | 4 ++-- database.hh | 2 +- locator/abstract_replication_strategy.cc | 8 ++++---- locator/abstract_replication_strategy.hh | 8 ++++---- locator/everywhere_replication_strategy.cc | 4 ++-- locator/everywhere_replication_strategy.hh | 2 +- locator/local_strategy.cc | 4 ++-- locator/local_strategy.hh | 2 +- locator/network_topology_strategy.cc | 4 ++-- locator/network_topology_strategy.hh | 2 +- locator/simple_strategy.cc | 4 ++-- locator/simple_strategy.hh | 2 +- 12 files changed, 23 insertions(+), 23 deletions(-) diff --git a/database.cc b/database.cc index 5d7b6dd6d6..b9fcf803e6 100644 --- a/database.cc +++ b/database.cc @@ -871,7 +871,7 @@ bool database::column_family_exists(const utils::UUID& uuid) const { } void -keyspace::create_replication_strategy(locator::token_metadata& tm, const std::map& options) { +keyspace::create_replication_strategy(const locator::token_metadata& tm, const std::map& options) { using namespace locator; _replication_strategy = @@ -1011,7 +1011,7 @@ const column_family& database::find_column_family(const schema_ptr& schema) cons using strategy_class_registry = class_registry< locator::abstract_replication_strategy, const sstring&, - locator::token_metadata&, + const locator::token_metadata&, locator::snitch_ptr&, const std::map&>; diff --git a/database.hh b/database.hh index 00a4579125..4874ff5832 100644 --- a/database.hh +++ b/database.hh @@ -1157,7 +1157,7 @@ public: * boom, it is replaced. */ lw_shared_ptr metadata() const; - void create_replication_strategy(locator::token_metadata& tm, const std::map& options); + void create_replication_strategy(const locator::token_metadata& tm, const std::map& options); /** * This should not really be return by reference, since replication * strategy is also volatile in that it could be replaced at "any" time. diff --git a/locator/abstract_replication_strategy.cc b/locator/abstract_replication_strategy.cc index 6b75525053..aec212ea55 100644 --- a/locator/abstract_replication_strategy.cc +++ b/locator/abstract_replication_strategy.cc @@ -30,7 +30,7 @@ logging::logger abstract_replication_strategy::logger("replication_strategy"); abstract_replication_strategy::abstract_replication_strategy( const sstring& ks_name, - token_metadata& token_metadata, + const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options, replication_strategy_type my_type) @@ -40,12 +40,12 @@ abstract_replication_strategy::abstract_replication_strategy( , _snitch(snitch) , _my_type(my_type) {} -std::unique_ptr abstract_replication_strategy::create_replication_strategy(const sstring& ks_name, const sstring& strategy_name, token_metadata& tk_metadata, const std::map& config_options) { +std::unique_ptr abstract_replication_strategy::create_replication_strategy(const sstring& ks_name, const sstring& strategy_name, const token_metadata& tk_metadata, const std::map& config_options) { assert(locator::i_endpoint_snitch::get_local_snitch_ptr()); try { return create_object&> (strategy_name, ks_name, tk_metadata, @@ -57,7 +57,7 @@ std::unique_ptr abstract_replication_strategy::cr void abstract_replication_strategy::validate_replication_strategy(const sstring& ks_name, const sstring& strategy_name, - token_metadata& token_metadata, + const token_metadata& token_metadata, const std::map& config_options) { auto strategy = create_replication_strategy(ks_name, strategy_name, token_metadata, config_options); diff --git a/locator/abstract_replication_strategy.hh b/locator/abstract_replication_strategy.hh index 48611ddbda..2304fb2ac4 100644 --- a/locator/abstract_replication_strategy.hh +++ b/locator/abstract_replication_strategy.hh @@ -59,7 +59,7 @@ protected: // TODO: Do we need this member at all? //keyspace* _keyspace = nullptr; std::map _config_options; - token_metadata& _token_metadata; + const token_metadata& _token_metadata; snitch_ptr& _snitch; replication_strategy_type _my_type; @@ -83,16 +83,16 @@ protected: public: abstract_replication_strategy( const sstring& keyspace_name, - token_metadata& token_metadata, + const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options, replication_strategy_type my_type); virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const = 0; virtual ~abstract_replication_strategy() {} - static std::unique_ptr create_replication_strategy(const sstring& ks_name, const sstring& strategy_name, token_metadata& token_metadata, const std::map& config_options); + static std::unique_ptr create_replication_strategy(const sstring& ks_name, const sstring& strategy_name, const token_metadata& token_metadata, const std::map& config_options); static void validate_replication_strategy(const sstring& ks_name, const sstring& strategy_name, - token_metadata& token_metadata, + const token_metadata& token_metadata, const std::map& config_options); virtual std::vector get_natural_endpoints(const token& search_token); virtual std::vector get_natural_endpoints_without_node_being_replaced(const token& search_token); diff --git a/locator/everywhere_replication_strategy.cc b/locator/everywhere_replication_strategy.cc index 17e1b094eb..78757f92f5 100644 --- a/locator/everywhere_replication_strategy.cc +++ b/locator/everywhere_replication_strategy.cc @@ -43,7 +43,7 @@ namespace locator { -everywhere_replication_strategy::everywhere_replication_strategy(const sstring& keyspace_name, token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options) : +everywhere_replication_strategy::everywhere_replication_strategy(const sstring& keyspace_name, const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options) : abstract_replication_strategy(keyspace_name, token_metadata, snitch, config_options, replication_strategy_type::everywhere_topology) {} std::vector everywhere_replication_strategy::get_natural_endpoints(const token& search_token) { @@ -53,7 +53,7 @@ std::vector everywhere_replication_strategy::get_natural_endpoints return calculate_natural_endpoints(search_token, _token_metadata); } -using registry = class_registrator&>; +using registry = class_registrator&>; static registry registrator("org.apache.cassandra.locator.EverywhereStrategy"); static registry registrator_short_name("EverywhereStrategy"); } diff --git a/locator/everywhere_replication_strategy.hh b/locator/everywhere_replication_strategy.hh index bfa9862b71..8f0520edf3 100644 --- a/locator/everywhere_replication_strategy.hh +++ b/locator/everywhere_replication_strategy.hh @@ -44,7 +44,7 @@ namespace locator { class everywhere_replication_strategy : public abstract_replication_strategy { public: - everywhere_replication_strategy(const sstring& keyspace_name, token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); + everywhere_replication_strategy(const sstring& keyspace_name, const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const override { return tm.get_all_endpoints(); diff --git a/locator/local_strategy.cc b/locator/local_strategy.cc index 8105ea0961..d461636a43 100644 --- a/locator/local_strategy.cc +++ b/locator/local_strategy.cc @@ -27,7 +27,7 @@ namespace locator { -local_strategy::local_strategy(const sstring& keyspace_name, token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options) : +local_strategy::local_strategy(const sstring& keyspace_name, const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options) : abstract_replication_strategy(keyspace_name, token_metadata, snitch, config_options, replication_strategy_type::local) {} std::vector local_strategy::get_natural_endpoints(const token& t) { @@ -50,7 +50,7 @@ size_t local_strategy::get_replication_factor() const { return 1; } -using registry = class_registrator&>; +using registry = class_registrator&>; static registry registrator("org.apache.cassandra.locator.LocalStrategy"); static registry registrator_short_name("LocalStrategy"); diff --git a/locator/local_strategy.hh b/locator/local_strategy.hh index 4bbf6c624f..34ce78be7d 100644 --- a/locator/local_strategy.hh +++ b/locator/local_strategy.hh @@ -38,7 +38,7 @@ class local_strategy : public abstract_replication_strategy { protected: virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const override; public: - local_strategy(const sstring& keyspace_name, token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); + local_strategy(const sstring& keyspace_name, const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); virtual ~local_strategy() {}; virtual size_t get_replication_factor() const; /** diff --git a/locator/network_topology_strategy.cc b/locator/network_topology_strategy.cc index a58893da8d..3222af3595 100644 --- a/locator/network_topology_strategy.cc +++ b/locator/network_topology_strategy.cc @@ -59,7 +59,7 @@ bool operator==(const endpoint_dc_rack& d1, const endpoint_dc_rack& d2) { network_topology_strategy::network_topology_strategy( const sstring& keyspace_name, - token_metadata& token_metadata, + const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options) : abstract_replication_strategy(keyspace_name, @@ -274,7 +274,7 @@ std::optional> network_topology_strategy::recognized_options() return std::nullopt; } -using registry = class_registrator&>; +using registry = class_registrator&>; static registry registrator("org.apache.cassandra.locator.NetworkTopologyStrategy"); static registry registrator_short_name("NetworkTopologyStrategy"); } diff --git a/locator/network_topology_strategy.hh b/locator/network_topology_strategy.hh index d04bfa57e1..064f7ebbe9 100644 --- a/locator/network_topology_strategy.hh +++ b/locator/network_topology_strategy.hh @@ -49,7 +49,7 @@ class network_topology_strategy : public abstract_replication_strategy { public: network_topology_strategy( const sstring& keyspace_name, - token_metadata& token_metadata, + const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); diff --git a/locator/simple_strategy.cc b/locator/simple_strategy.cc index 2cdc9480f0..b6931d581c 100644 --- a/locator/simple_strategy.cc +++ b/locator/simple_strategy.cc @@ -27,7 +27,7 @@ namespace locator { -simple_strategy::simple_strategy(const sstring& keyspace_name, token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options) : +simple_strategy::simple_strategy(const sstring& keyspace_name, const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options) : abstract_replication_strategy(keyspace_name, token_metadata, snitch, config_options, replication_strategy_type::simple) { for (auto& config_pair : config_options) { auto& key = config_pair.first; @@ -82,7 +82,7 @@ std::optional>simple_strategy::recognized_options() const { return {{ "replication_factor" }}; } -using registry = class_registrator&>; +using registry = class_registrator&>; static registry registrator("org.apache.cassandra.locator.SimpleStrategy"); static registry registrator_short_name("SimpleStrategy"); diff --git a/locator/simple_strategy.hh b/locator/simple_strategy.hh index af7e9badde..259c20c86a 100644 --- a/locator/simple_strategy.hh +++ b/locator/simple_strategy.hh @@ -32,7 +32,7 @@ class simple_strategy : public abstract_replication_strategy { protected: virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const override; public: - simple_strategy(const sstring& keyspace_name, token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); + simple_strategy(const sstring& keyspace_name, const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); virtual ~simple_strategy() {}; virtual size_t get_replication_factor() const override; virtual void validate_options() const override; From e4e4b269c71198ec585903a7183402159f75b044 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Wed, 19 Aug 2020 13:16:48 +0300 Subject: [PATCH 10/19] everywhere_replication_strategy: move methods out of line Move methods depending on token_metadata to source file so we can avoid including token_metadata.hh in header files where spossible. Signed-off-by: Benny Halevy --- locator/everywhere_replication_strategy.cc | 9 +++++++++ locator/everywhere_replication_strategy.hh | 8 ++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/locator/everywhere_replication_strategy.cc b/locator/everywhere_replication_strategy.cc index 78757f92f5..b0bca67b25 100644 --- a/locator/everywhere_replication_strategy.cc +++ b/locator/everywhere_replication_strategy.cc @@ -40,12 +40,17 @@ #include "locator/everywhere_replication_strategy.hh" #include "utils/class_registrator.hh" #include "utils/fb_utilities.hh" +#include "locator/token_metadata.hh" namespace locator { everywhere_replication_strategy::everywhere_replication_strategy(const sstring& keyspace_name, const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options) : abstract_replication_strategy(keyspace_name, token_metadata, snitch, config_options, replication_strategy_type::everywhere_topology) {} +std::vector everywhere_replication_strategy::calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const { + return tm.get_all_endpoints(); +} + std::vector everywhere_replication_strategy::get_natural_endpoints(const token& search_token) { if (_token_metadata.sorted_tokens().empty()) { return std::vector({utils::fb_utilities::get_broadcast_address()}); @@ -53,6 +58,10 @@ std::vector everywhere_replication_strategy::get_natural_endpoints return calculate_natural_endpoints(search_token, _token_metadata); } +size_t everywhere_replication_strategy::get_replication_factor() const { + return _token_metadata.get_all_endpoints_count(); +} + using registry = class_registrator&>; static registry registrator("org.apache.cassandra.locator.EverywhereStrategy"); static registry registrator_short_name("EverywhereStrategy"); diff --git a/locator/everywhere_replication_strategy.hh b/locator/everywhere_replication_strategy.hh index 8f0520edf3..11f9be2227 100644 --- a/locator/everywhere_replication_strategy.hh +++ b/locator/everywhere_replication_strategy.hh @@ -46,9 +46,7 @@ class everywhere_replication_strategy : public abstract_replication_strategy { public: everywhere_replication_strategy(const sstring& keyspace_name, const token_metadata& token_metadata, snitch_ptr& snitch, const std::map& config_options); - virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const override { - return tm.get_all_endpoints(); - } + virtual std::vector calculate_natural_endpoints(const token& search_token, const token_metadata& tm) const override; std::vector get_natural_endpoints(const token& search_token) override; virtual void validate_options() const override { /* noop */ } @@ -58,9 +56,7 @@ public: return std::nullopt; } - virtual size_t get_replication_factor() const override { - return _token_metadata.get_all_endpoints_count(); - } + virtual size_t get_replication_factor() const override; virtual bool allow_remove_node_being_replaced_from_natural_endpoints() const override { return true; From 8b5c32c7a8d4d4e171080b015bfc0b86b20e94e0 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 14:19:51 +0300 Subject: [PATCH 11/19] database: keyspace_metadata: pass const locator::token_metadata& around No need to modify token_metadata on this path. Signed-off-by: Benny Halevy --- database.cc | 4 ++-- database.hh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/database.cc b/database.cc index b9fcf803e6..82c9484249 100644 --- a/database.cc +++ b/database.cc @@ -895,7 +895,7 @@ keyspace::set_replication_strategy(std::unique_ptr ksm) { +void keyspace::update_from(const locator::token_metadata& tm, ::lw_shared_ptr ksm) { _metadata = std::move(ksm); create_replication_strategy(tm, _metadata->strategy_options()); } @@ -1044,7 +1044,7 @@ keyspace_metadata::keyspace_metadata(std::string_view name, } } -void keyspace_metadata::validate(locator::token_metadata& tm) const { +void keyspace_metadata::validate(const locator::token_metadata& tm) const { using namespace locator; abstract_replication_strategy::validate_replication_strategy(name(), strategy_name(), tm, strategy_options()); } diff --git a/database.hh b/database.hh index 4874ff5832..9a911c6434 100644 --- a/database.hh +++ b/database.hh @@ -1080,7 +1080,7 @@ public: std::map options, bool durables_writes, std::vector cf_defs = std::vector{}); - void validate(locator::token_metadata& tm) const; + void validate(const locator::token_metadata& tm) const; const sstring& name() const { return _name; } @@ -1150,7 +1150,7 @@ private: public: explicit keyspace(lw_shared_ptr metadata, config cfg); - void update_from(locator::token_metadata& tm, lw_shared_ptr); + void update_from(const locator::token_metadata& tm, lw_shared_ptr); /** Note: return by shared pointer value, since the meta data is * semi-volatile. I.e. we could do alter keyspace at any time, and From dd6d77133106283aad0b98be4fa77f0cb02df0a5 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 14:16:07 +0300 Subject: [PATCH 12/19] database: keep const token_metadata& No need to modify token_metadata form database code. Also, get rid of mutable get_token_metadata variant. Signed-off-by: Benny Halevy --- database.cc | 2 +- database.hh | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/database.cc b/database.cc index 82c9484249..bc8abd460c 100644 --- a/database.cc +++ b/database.cc @@ -165,7 +165,7 @@ bool string_pair_eq::operator()(spair lhs, spair rhs) const { utils::UUID database::empty_version = utils::UUID_gen::get_name_UUID(bytes{}); -database::database(const db::config& cfg, database_config dbcfg, service::migration_notifier& mn, gms::feature_service& feat, locator::token_metadata& tm, abort_source& as) +database::database(const db::config& cfg, database_config dbcfg, service::migration_notifier& mn, gms::feature_service& feat, const locator::token_metadata& tm, abort_source& as) : _stats(make_lw_shared()) , _cl_stats(std::make_unique()) , _cfg(cfg) diff --git a/database.hh b/database.hh index 9a911c6434..17c71ffe14 100644 --- a/database.hh +++ b/database.hh @@ -1337,7 +1337,7 @@ private: service::migration_notifier& _mnotifier; gms::feature_service& _feat; - locator::token_metadata& _token_metadata; + const locator::token_metadata& _token_metadata; bool _supports_infinite_bound_range_deletions = false; gms::feature::listener_registration _infinite_bound_range_deletions_reg; @@ -1377,7 +1377,7 @@ public: void set_enable_incremental_backups(bool val) { _enable_incremental_backups = val; } future<> parse_system_tables(distributed&, distributed&); - database(const db::config&, database_config dbcfg, service::migration_notifier& mn, gms::feature_service& feat, locator::token_metadata& tm, abort_source& as); + database(const db::config&, database_config dbcfg, service::migration_notifier& mn, gms::feature_service& feat, const locator::token_metadata& tm, abort_source& as); database(database&&) = delete; ~database(); @@ -1403,7 +1403,6 @@ public: } const locator::token_metadata& get_token_metadata() const { return _token_metadata; } - locator::token_metadata& get_token_metadata() { return _token_metadata; } service::migration_notifier& get_notifier() { return _mnotifier; } const service::migration_notifier& get_notifier() const { return _mnotifier; } From dfa5f8ff1ec6405ffd5de0b900834e75dbd24660 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 08:39:45 +0300 Subject: [PATCH 13/19] storage_proxy: get rid of mutable get_token_metadata getter We'd like to strictly control who can modify token metadata and nobody currently needs a mutable reference to storage_proxy::_token_metadata. Signed-off-by: Benny Halevy --- alternator/streams.cc | 2 +- service/storage_proxy.cc | 2 +- service/storage_proxy.hh | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/alternator/streams.cc b/alternator/streams.cc index ec86c3cf9f..e739fb1063 100644 --- a/alternator/streams.cc +++ b/alternator/streams.cc @@ -423,7 +423,7 @@ future executor::describe_stream(client_state& cl // TODO: label // TODO: creation time - auto& tm = _proxy.get_token_metadata(); + const auto& tm = _proxy.get_token_metadata(); // cannot really "resume" query, must iterate all data. because we cannot query neither "time" (pk) > something, // or on expired... // TODO: maybe add secondary index to topology table to enable this? diff --git a/service/storage_proxy.cc b/service/storage_proxy.cc index 0df17a10e4..1acaee900b 100644 --- a/service/storage_proxy.cc +++ b/service/storage_proxy.cc @@ -4607,7 +4607,7 @@ std::vector storage_proxy::intersection(const std::vector Date: Mon, 17 Aug 2020 10:30:38 +0300 Subject: [PATCH 14/19] storage_proxy: keep a const token_metadata& storage_proxy doesn't need to change token_metadata. Signed-off-by: Benny Halevy --- service/storage_proxy.cc | 6 +++--- service/storage_proxy.hh | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/service/storage_proxy.cc b/service/storage_proxy.cc index 1acaee900b..ae4624da2e 100644 --- a/service/storage_proxy.cc +++ b/service/storage_proxy.cc @@ -1289,7 +1289,7 @@ bool paxos_response_handler::learned(gms::inet_address ep) { } static std::vector -replica_ids_to_endpoints(locator::token_metadata& tm, const std::vector& replica_ids) { +replica_ids_to_endpoints(const locator::token_metadata& tm, const std::vector& replica_ids) { std::vector endpoints; endpoints.reserve(replica_ids.size()); @@ -1303,7 +1303,7 @@ replica_ids_to_endpoints(locator::token_metadata& tm, const std::vector -endpoints_to_replica_ids(locator::token_metadata& tm, const std::vector& endpoints) { +endpoints_to_replica_ids(const locator::token_metadata& tm, const std::vector& endpoints) { std::vector replica_ids; replica_ids.reserve(endpoints.size()); @@ -1781,7 +1781,7 @@ using namespace std::literals::chrono_literals; storage_proxy::~storage_proxy() {} storage_proxy::storage_proxy(distributed& db, storage_proxy::config cfg, db::view::node_update_backlog& max_view_update_backlog, - scheduling_group_key stats_key, gms::feature_service& feat, locator::token_metadata& tm) + scheduling_group_key stats_key, gms::feature_service& feat, const locator::token_metadata& tm) : _db(db) , _token_metadata(tm) , _read_smp_service_group(cfg.read_smp_service_group) diff --git a/service/storage_proxy.hh b/service/storage_proxy.hh index 5e2ababd8b..1839324efa 100644 --- a/service/storage_proxy.hh +++ b/service/storage_proxy.hh @@ -250,7 +250,7 @@ public: private: distributed& _db; - locator::token_metadata& _token_metadata; + const locator::token_metadata& _token_metadata; smp_service_group _read_smp_service_group; smp_service_group _write_smp_service_group; smp_service_group _write_ack_smp_service_group; @@ -435,7 +435,7 @@ private: future<> mutate_counters(Range&& mutations, db::consistency_level cl, tracing::trace_state_ptr tr_state, service_permit permit, clock_type::time_point timeout); public: storage_proxy(distributed& db, config cfg, db::view::node_update_backlog& max_view_update_backlog, - scheduling_group_key stats_key, gms::feature_service& feat, locator::token_metadata& tokens); + scheduling_group_key stats_key, gms::feature_service& feat, const locator::token_metadata& tokens); ~storage_proxy(); const distributed& get_db() const { return _db; From 2c61383215930e6c64436ed197a0dd218e3ffd39 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 10:38:30 +0300 Subject: [PATCH 15/19] storage_proxy: delete unused get_restricted_ranges declaration Signed-off-by: Benny Halevy --- service/storage_proxy.hh | 3 --- 1 file changed, 3 deletions(-) diff --git a/service/storage_proxy.hh b/service/storage_proxy.hh index 1839324efa..bd63849c56 100644 --- a/service/storage_proxy.hh +++ b/service/storage_proxy.hh @@ -749,7 +749,4 @@ inline shared_ptr get_local_shared_storage_proxy() { return _the_storage_proxy.local_shared(); } -dht::partition_range_vector get_restricted_ranges(locator::token_metadata&, - const schema&, dht::partition_range); - } From 569f2830c1706801c522efe821198ee00ab7a1ef Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 10:06:52 +0300 Subject: [PATCH 16/19] range_streamer: keep a const token_metadata& range_streamer doesn't need to modify toekn_metadata. Signed-off-by: Benny Halevy --- dht/range_streamer.hh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dht/range_streamer.hh b/dht/range_streamer.hh index 2d97f4998d..e999131460 100644 --- a/dht/range_streamer.hh +++ b/dht/range_streamer.hh @@ -101,7 +101,7 @@ public: } }; - range_streamer(distributed& db, token_metadata& tm, abort_source& abort_source, std::unordered_set tokens, inet_address address, sstring description, streaming::stream_reason reason) + range_streamer(distributed& db, const token_metadata& tm, abort_source& abort_source, std::unordered_set tokens, inet_address address, sstring description, streaming::stream_reason reason) : _db(db) , _metadata(tm) , _abort_source(abort_source) @@ -113,7 +113,7 @@ public: _abort_source.check(); } - range_streamer(distributed& db, token_metadata& tm, abort_source& abort_source, inet_address address, sstring description, streaming::stream_reason reason) + range_streamer(distributed& db, const token_metadata& tm, abort_source& abort_source, inet_address address, sstring description, streaming::stream_reason reason) : range_streamer(db, tm, abort_source, std::unordered_set(), address, description, reason) { } @@ -165,7 +165,7 @@ public: size_t nr_ranges_to_stream(); private: distributed& _db; - token_metadata& _metadata; + const token_metadata& _metadata; abort_source& _abort_source; std::unordered_set _tokens; inet_address _address; From 2f7c529c1cd1ad4627d235e478e6d1ffddfd11b4 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 10:02:59 +0300 Subject: [PATCH 17/19] storage_service: separate get_mutable_token_metadata Use a different getter for a token_metadata& that may be changed so we can better synchronize readers and writers of token_metadata and eventually allow them to yield in asynchronous loops. Signed-off-by: Benny Halevy --- cdc/log.cc | 2 +- cdc/log.hh | 6 +++--- locator/gossiping_property_file_snitch.cc | 2 +- service/storage_service.cc | 2 +- service/storage_service.hh | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cdc/log.cc b/cdc/log.cc index d2f0690f80..93a84b5be9 100644 --- a/cdc/log.cc +++ b/cdc/log.cc @@ -587,7 +587,7 @@ db_context::builder& db_context::builder::with_migration_notifier(service::migra return *this; } -db_context::builder& db_context::builder::with_token_metadata(locator::token_metadata& token_metadata) { +db_context::builder& db_context::builder::with_token_metadata(const locator::token_metadata& token_metadata) { _token_metadata = token_metadata; return *this; } diff --git a/cdc/log.hh b/cdc/log.hh index c427a8061f..a7874078a3 100644 --- a/cdc/log.hh +++ b/cdc/log.hh @@ -100,19 +100,19 @@ public: struct db_context final { service::storage_proxy& _proxy; service::migration_notifier& _migration_notifier; - locator::token_metadata& _token_metadata; + const locator::token_metadata& _token_metadata; cdc::metadata& _cdc_metadata; class builder final { service::storage_proxy& _proxy; std::optional> _migration_notifier; - std::optional> _token_metadata; + std::optional> _token_metadata; std::optional> _cdc_metadata; public: builder(service::storage_proxy& proxy); builder& with_migration_notifier(service::migration_notifier& migration_notifier); - builder& with_token_metadata(locator::token_metadata& token_metadata); + builder& with_token_metadata(const locator::token_metadata& token_metadata); builder& with_cdc_metadata(cdc::metadata&); db_context build(); diff --git a/locator/gossiping_property_file_snitch.cc b/locator/gossiping_property_file_snitch.cc index 8eeee7bab8..2e8194691e 100644 --- a/locator/gossiping_property_file_snitch.cc +++ b/locator/gossiping_property_file_snitch.cc @@ -231,7 +231,7 @@ future<> gossiping_property_file_snitch::reload_configuration() { parallel_for_each(cpus.begin(), cpus.end(), [] (unsigned int c) { return smp::submit_to(c, [] { if (service::get_storage_service().local_is_initialized()) { - auto& tmd = service::get_local_storage_service().get_token_metadata(); + auto& tmd = service::get_local_storage_service().get_mutable_token_metadata(); // initiate the token metadata endpoints cache reset tmd.invalidate_cached_rings(); diff --git a/service/storage_service.cc b/service/storage_service.cc index 0fcef7c598..f9287523bd 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -2781,7 +2781,7 @@ future<> storage_service::do_update_pending_ranges() { auto& ks = this->_db.local().find_keyspace(keyspace_name); auto& strategy = ks.get_replication_strategy(); slogger.debug("Updating pending ranges for keyspace={} starts", keyspace_name); - return get_token_metadata().update_pending_ranges(strategy, keyspace_name).finally([&keyspace_name] { + return get_mutable_token_metadata().update_pending_ranges(strategy, keyspace_name).finally([&keyspace_name] { slogger.debug("Updating pending ranges for keyspace={} ends", keyspace_name); }); }); diff --git a/service/storage_service.hh b/service/storage_service.hh index edcc142827..98d00174bc 100644 --- a/service/storage_service.hh +++ b/service/storage_service.hh @@ -173,7 +173,7 @@ public: return _token_metadata; } - locator::token_metadata& get_token_metadata() { + locator::token_metadata& get_mutable_token_metadata() { return _token_metadata; } From 573142d4c477089d2beb8c504cc26d440679f381 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 10:25:14 +0300 Subject: [PATCH 18/19] gossiper: keep a const token_metadata& gossiper has no need to change token_metadata. Signed-off-by: Benny Halevy --- gms/gossiper.cc | 2 +- gms/gossiper.hh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gms/gossiper.cc b/gms/gossiper.cc index 141a08f1f9..ba75934a7a 100644 --- a/gms/gossiper.cc +++ b/gms/gossiper.cc @@ -123,7 +123,7 @@ public: void on_restart(inet_address, endpoint_state) override {} }; -gossiper::gossiper(abort_source& as, feature_service& features, locator::token_metadata& tokens, db::config& cfg) +gossiper::gossiper(abort_source& as, feature_service& features, const locator::token_metadata& tokens, db::config& cfg) : _abort_source(as) , _feature_service(features) , _token_metadata(tokens) diff --git a/gms/gossiper.hh b/gms/gossiper.hh index 59d32df295..bb2fdde99a 100644 --- a/gms/gossiper.hh +++ b/gms/gossiper.hh @@ -238,7 +238,7 @@ private: // The value must be kept alive until completes and not change. future<> replicate(inet_address, application_state key, const versioned_value& value); public: - explicit gossiper(abort_source& as, feature_service& features, locator::token_metadata& tokens, db::config& cfg); + explicit gossiper(abort_source& as, feature_service& features, const locator::token_metadata& tokens, db::config& cfg); void set_last_processed_message_at(); void set_last_processed_message_at(clk::time_point tp); @@ -559,7 +559,7 @@ private: abort_source& _abort_source; condition_variable _features_condvar; feature_service& _feature_service; - locator::token_metadata& _token_metadata; + const locator::token_metadata& _token_metadata; db::config& _cfg; failure_detector _fd; friend class feature; From 436babdb3dc7c9f9b0d9d19f9166f38c03ceab14 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 17 Aug 2020 10:42:06 +0300 Subject: [PATCH 19/19] api/http_context: keep a const sharded& It has no need of changing token_metadata. Signed-off-by: Benny Halevy --- api/api_init.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/api_init.hh b/api/api_init.hh index 168998c7e4..c6ff35dbb9 100644 --- a/api/api_init.hh +++ b/api/api_init.hh @@ -38,11 +38,11 @@ struct http_context { distributed& db; distributed& sp; service::load_meter& lmeter; - sharded& token_metadata; + const sharded& token_metadata; http_context(distributed& _db, distributed& _sp, - service::load_meter& _lm, sharded& _tm) + service::load_meter& _lm, const sharded& _tm) : db(_db), sp(_sp), lmeter(_lm), token_metadata(_tm) { } };