token_metadata: get_endpoint_for_host_id -> get_endpoint_for_host_id_…

…if_known

This commit fixes an inconsistency in method names:
get_host_id and get_host_id_if_known are
(internal_error, returns null), but there was only
one method for the opposite conversion - get_endpoint_for_host_id,
and it returns null. In this commit we change it to on_internal_error
if it can't find the argument and add another method
get_endpoint_for_host_id_if_known which returns null in this case.

We can't use get_endpoint_for_host_id/get_host_id
in host_id_or_endpoint::resolve since it's called
from storage_service::parse_node_list
-> token_metadata::parse_host_id_and_endpoint,
and exceptions are caught and handled in
`storage_service::parse_node_list`.

db/view/view.cc

@@ -2579,7 +2579,7 @@ future<bool> check_view_build_ongoing(db::system_distributed_keyspace& sys_dist_
     return sys_dist_ks.view_status(ks_name, cf_name).then([&tm] (view_statuses_type&& view_statuses) {
         return boost::algorithm::any_of(view_statuses, [&tm] (const view_statuses_type::value_type& view_status) {
             // Only consider status of known hosts.
-            return view_status.second == "STARTED" && tm.get_endpoint_for_host_id(view_status.first);
+            return view_status.second == "STARTED" && tm.get_endpoint_for_host_id_if_known(view_status.first);
         });
     });
 }

locator/tablets.cc

@@ -334,18 +334,11 @@ class tablet_effective_replication_map : public effective_replication_map {
     table_id _table;
     tablet_sharder _sharder;
 private:
-    gms::inet_address get_endpoint_for_host_id(host_id host) const {
-        auto endpoint_opt = _tmptr->get_endpoint_for_host_id(host);
-        if (!endpoint_opt) {
-            on_internal_error(tablet_logger, format("Host ID {} not found in the cluster", host));
-        }
-        return *endpoint_opt;
-    }
     inet_address_vector_replica_set to_replica_set(const tablet_replica_set& replicas) const {
         inet_address_vector_replica_set result;
         result.reserve(replicas.size());
         for (auto&& replica : replicas) {
-            result.emplace_back(get_endpoint_for_host_id(replica.host));
+            result.emplace_back(_tmptr->get_endpoint_for_host_id(replica.host));
         }
         return result;
     }
@@ -406,7 +399,7 @@ class tablet_effective_replication_map : public effective_replication_map {
             case write_replica_set_selector::both:
                 tablet_logger.trace("get_pending_endpoints({}): table={}, tablet={}, replica={}",
                                     search_token, _table, tablet, info->pending_replica);
-                return {get_endpoint_for_host_id(info->pending_replica.host)};
+                return {_tmptr->get_endpoint_for_host_id(info->pending_replica.host)};
             case write_replica_set_selector::next:
                 return {};
         }

locator/token_metadata.cc

@@ -167,8 +167,11 @@ class token_metadata_impl final {
     /// Return the unique host ID for an end-point or nullopt if not found.
     std::optional<host_id> get_host_id_if_known(inet_address endpoint) const;
 
-    /** Return the end-point for a unique host ID */
-    std::optional<inet_address> get_endpoint_for_host_id(host_id) const;
+    /** Return the end-point for a unique host ID or nullopt if not found.*/
+    std::optional<inet_address> get_endpoint_for_host_id_if_known(host_id) const;
+
+    /** Return the end-point for a unique host ID.*/
+    inet_address get_endpoint_for_host_id(host_id) const;
 
     /** @return a copy of the endpoint-to-id map for read-only operations */
     std::unordered_map<inet_address, host_id> get_endpoint_to_host_id_map_for_reading() const;
@@ -574,14 +577,23 @@ std::optional<host_id> token_metadata_impl<NodeId>::get_host_id_if_known(inet_ad
 }
 
 template <typename NodeId>
-std::optional<inet_address> token_metadata_impl<NodeId>::get_endpoint_for_host_id(host_id host_id) const {
+std::optional<inet_address> token_metadata_impl<NodeId>::get_endpoint_for_host_id_if_known(host_id host_id) const {
     if (const auto* node = _topology.find_node(host_id)) [[likely]] {
         return node->endpoint();
     } else {
         return std::nullopt;
     }
 }
 
+template <typename NodeId>
+inet_address token_metadata_impl<NodeId>::get_endpoint_for_host_id(host_id host_id) const {
+    if (const auto* node = _topology.find_node(host_id)) [[likely]] {
+        return node->endpoint();
+    } else {
+        on_internal_error(tlogger, format("endpoint for host_id {} is not found", host_id));
+    }
+}
+
 template <typename NodeId>
 std::unordered_map<inet_address, host_id> token_metadata_impl<NodeId>::get_endpoint_to_host_id_map_for_reading() const {
     const auto& nodes = _topology.get_nodes_by_endpoint();
@@ -1097,6 +1109,12 @@ generic_token_metadata<NodeId>::get_host_id_if_known(inet_address endpoint) cons
 
 template <typename NodeId>
 std::optional<typename generic_token_metadata<NodeId>::inet_address>
+generic_token_metadata<NodeId>::get_endpoint_for_host_id_if_known(host_id host_id) const {
+    return _impl->get_endpoint_for_host_id_if_known(host_id);
+}
+
+template <typename NodeId>
+typename generic_token_metadata<NodeId>::inet_address
 generic_token_metadata<NodeId>::get_endpoint_for_host_id(host_id host_id) const {
     return _impl->get_endpoint_for_host_id(host_id);
 }
@@ -1445,7 +1463,7 @@ host_id_or_endpoint::host_id_or_endpoint(const sstring& s, param_type restrict)
 template <typename NodeId>
 void host_id_or_endpoint::resolve(const generic_token_metadata<NodeId>& tm) {
     if (id) {
-        auto endpoint_opt = tm.get_endpoint_for_host_id(id);
+        auto endpoint_opt = tm.get_endpoint_for_host_id_if_known(id);
         if (!endpoint_opt) {
             throw std::runtime_error(format("Host ID {} not found in the cluster", id));
         }

locator/token_metadata.hh

@@ -219,8 +219,11 @@ public:
     /// Return the unique host ID for an end-point or nullopt if not found.
     std::optional<host_id> get_host_id_if_known(inet_address endpoint) const;
 
+    /** Return the end-point for a unique host ID or nullopt if not found. */
+    std::optional<inet_address> get_endpoint_for_host_id_if_known(locator::host_id host_id) const;
+
     /** Return the end-point for a unique host ID */
-    std::optional<inet_address> get_endpoint_for_host_id(locator::host_id host_id) const;
+    inet_address get_endpoint_for_host_id(locator::host_id host_id) const;
 
     /// Parses the \c host_id_string either as a host uuid or as an ip address and returns the mapping.
     /// Throws std::invalid_argument on parse error or std::runtime_error if the host_id wasn't found.

service/storage_proxy.cc

@@ -2291,7 +2291,7 @@ replica_ids_to_endpoints(const locator::token_metadata& tm, const std::vector<lo
     endpoints.reserve(replica_ids.size());
 
     for (const auto& replica_id : replica_ids) {
-        if (auto endpoint_opt = tm.get_endpoint_for_host_id(replica_id)) {
+        if (auto endpoint_opt = tm.get_endpoint_for_host_id_if_known(replica_id)) {
             endpoints.push_back(*endpoint_opt);
         }
     }

service/storage_service.cc

@@ -3686,7 +3686,7 @@ future<> storage_service::handle_state_normal(inet_address endpoint, gms::permit
     };
     // Order Matters, TM.updateHostID() should be called before TM.updateNormalToken(), (see CASSANDRA-4300).
     auto host_id = _gossiper.get_host_id(endpoint);
-    auto existing = tmptr->get_endpoint_for_host_id(host_id);
+    auto existing = tmptr->get_endpoint_for_host_id_if_known(host_id);
     if (existing && *existing != endpoint) {
         if (*existing == get_broadcast_address()) {
             slogger.warn("Not updating host ID {} for {} because it's mine", host_id, endpoint);
@@ -5136,7 +5136,7 @@ future<> storage_service::removenode(locator::host_id host_id, std::list<locator
             auto stop_ctl = deferred_stop(ctl);
             auto uuid = ctl.uuid();
             const auto& tmptr = ctl.tmptr;
-            auto endpoint_opt = tmptr->get_endpoint_for_host_id(host_id);
+            auto endpoint_opt = tmptr->get_endpoint_for_host_id_if_known(host_id);
             assert(ss._group0);
             auto raft_id = raft::server_id{host_id.uuid()};
             bool raft_available = ss._group0->wait_for_raft().get();

test/boost/network_topology_strategy_test.cc

@@ -194,17 +194,11 @@ void full_ring_check(const tablet_map& tmap,
     auto& tm = *tmptr;
     const auto& topo = tm.get_topology();
 
-    auto get_endpoint_for_host_id = [&] (host_id host) {
-        auto endpoint_opt = tm.get_endpoint_for_host_id(host);
-        assert(endpoint_opt);
-        return *endpoint_opt;
-    };
-
     auto to_endpoint_set = [&] (const tablet_replica_set& replicas) {
         inet_address_vector_replica_set result;
         result.reserve(replicas.size());
         for (auto&& replica : replicas) {
-            result.emplace_back(get_endpoint_for_host_id(replica.host));
+            result.emplace_back(tm.get_endpoint_for_host_id(replica.host));
         }
         return result;
     };