topology: drop key_kind, host_id is now the primary key

locator/token_metadata.cc

@@ -92,9 +92,9 @@ private:
     struct shallow_copy {};
 public:
     token_metadata_impl(shallow_copy, const token_metadata_impl& o) noexcept
-        : _topology(topology::config{}, topology::key_kind::host_id)
+        : _topology(topology::config{})
     {}
-    token_metadata_impl(token_metadata::config cfg) noexcept : _topology(std::move(cfg.topo_cfg), topology::key_kind::host_id) {};
+    token_metadata_impl(token_metadata::config cfg) noexcept : _topology(std::move(cfg.topo_cfg)) {};
     token_metadata_impl(const token_metadata_impl&) = delete; // it's too huge for direct copy, use clone_async()
     token_metadata_impl(token_metadata_impl&&) noexcept = default;
     const std::vector<token>& sorted_tokens() const;

locator/topology.cc

@@ -70,11 +70,10 @@ future<> topology::clear_gently() noexcept {
     co_await utils::clear_gently(_nodes);
 }
 
-topology::topology(config cfg, key_kind k)
+topology::topology(config cfg)
         : _shard(this_shard_id())
         , _cfg(cfg)
         , _sort_by_proximity(!cfg.disable_proximity_sorting)
-        , _key_kind(k)
 {
     tlogger.trace("topology[{}]: constructing using config: endpoint={} dc={} rack={}", fmt::ptr(this),
             cfg.this_endpoint, cfg.local_dc_rack.dc, cfg.local_dc_rack.rack);
@@ -93,7 +92,6 @@ topology::topology(topology&& o) noexcept
     , _dc_racks(std::move(o._dc_racks))
     , _sort_by_proximity(o._sort_by_proximity)
     , _datacenters(std::move(o._datacenters))
-    , _key_kind(o._key_kind)
 {
     assert(_shard == this_shard_id());
     tlogger.trace("topology[{}]: move from [{}]", fmt::ptr(this), fmt::ptr(&o));
@@ -114,7 +112,7 @@ topology& topology::operator=(topology&& o) noexcept {
 }
 
 future<topology> topology::clone_gently() const {
-    topology ret(_cfg, _key_kind);
+    topology ret(_cfg);
     tlogger.debug("topology[{}]: clone_gently to {} from shard {}", fmt::ptr(this), fmt::ptr(&ret), _shard);
     for (const auto& nptr : _nodes) {
         if (nptr) {
@@ -452,30 +450,14 @@ const node* topology::add_or_update_endpoint(std::optional<inet_address> opt_ep,
             current_backtrace());
     }
 
-    const node* n;
-    switch (_key_kind) {
-    case topology::key_kind::host_id:
-        if (!opt_id) {
-            on_internal_error(tlogger, format("topology: host_id is not set, ep={}", opt_ep));
-        }
-        n = find_node(*opt_id);
-        if (n) {
-            return update_node(make_mutable(n), std::nullopt, opt_ep, std::move(opt_dr), std::move(opt_st), std::move(shard_count));
-        } else if (opt_ep && (n = find_node(*opt_ep))) {
-            return update_node(make_mutable(n), opt_id, std::nullopt, std::move(opt_dr), std::move(opt_st), std::move(shard_count));
-        }
-        break;
-    case topology::key_kind::inet_address:
-        if (!opt_ep) {
-            on_internal_error(tlogger, format("topology: endpoint is not set, id={}", opt_id));
-        }
-        n = find_node(*opt_ep);
-        if (n) {
-            return update_node(make_mutable(n), opt_id, std::nullopt, std::move(opt_dr), std::move(opt_st), std::move(shard_count));
-        } else if (opt_id && (n = find_node(*opt_id))) {
-            return update_node(make_mutable(n), std::nullopt, opt_ep, std::move(opt_dr), std::move(opt_st), std::move(shard_count));
-        }
-        break;
+    if (!opt_id) {
+        on_internal_error(tlogger, format("topology: host_id is not set, ep={}", opt_ep));
+    }
+    const auto* n = find_node(*opt_id);
+    if (n) {
+        return update_node(make_mutable(n), std::nullopt, opt_ep, std::move(opt_dr), std::move(opt_st), std::move(shard_count));
+    } else if (opt_ep && (n = find_node(*opt_ep))) {
+        return update_node(make_mutable(n), opt_id, std::nullopt, std::move(opt_dr), std::move(opt_st), std::move(shard_count));
     }
 
     return add_node(opt_id.value_or(host_id::create_null_id()),

locator/topology.hh

@@ -159,11 +159,6 @@ private:
 
 class topology {
 public:
-    enum class key_kind {
-        inet_address,
-        host_id,
-    };
-
     struct config {
         inet_address this_endpoint;
         inet_address this_cql_address;   // corresponds to broadcast_rpc_address
@@ -173,7 +168,7 @@ public:
 
         bool operator==(const config&) const = default;
     };
-    topology(config cfg, key_kind k);
+    topology(config cfg);
     topology(topology&&) noexcept;
 
     topology& operator=(topology&&) noexcept;
@@ -416,8 +411,6 @@ private:
     // pre-calculated
     std::unordered_set<sstring> _datacenters;
 
-    key_kind _key_kind;
-
     void calculate_datacenters();
 
     const std::unordered_map<inet_address, const node*>& get_nodes_by_endpoint() const noexcept {

test/boost/locator_topology_test.cc

@@ -36,7 +36,7 @@ SEASTAR_THREAD_TEST_CASE(test_add_node) {
         .local_dc_rack = endpoint_dc_rack::default_location,
     };
 
-    auto topo = topology(cfg, topology::key_kind::inet_address);
+    auto topo = topology(cfg);
 
     set_abort_on_internal_error(false);
     auto reset_on_internal_abort = seastar::defer([] {
@@ -73,7 +73,7 @@ SEASTAR_THREAD_TEST_CASE(test_moving) {
         .local_dc_rack = endpoint_dc_rack::default_location,
     };
 
-    auto topo = topology(cfg, topology::key_kind::inet_address);
+    auto topo = topology(cfg);
 
     topo.add_node(id1, ep1, endpoint_dc_rack::default_location, node::state::normal);
 
@@ -99,22 +99,23 @@ SEASTAR_THREAD_TEST_CASE(test_update_node) {
 
     topology::config cfg = {
         .this_endpoint = ep1,
+        .this_host_id = id1,
         .local_dc_rack = endpoint_dc_rack::default_location,
     };
 
-    auto topo = topology(cfg, topology::key_kind::inet_address);
+    auto topo = topology(cfg);
 
     set_abort_on_internal_error(false);
     auto reset_on_internal_abort = seastar::defer([] {
         set_abort_on_internal_error(true);
     });
 
-    topo.add_or_update_endpoint(ep1, endpoint_dc_rack::default_location, node::state::normal);
+    topo.add_or_update_endpoint(std::nullopt, id1, endpoint_dc_rack::default_location, node::state::normal);
 
     auto node = topo.this_node();
     auto mutable_node = const_cast<locator::node*>(node);
 
-    node = topo.update_node(mutable_node, id1, std::nullopt, std::nullopt, std::nullopt);
+    node = topo.update_node(mutable_node, std::nullopt, ep1, std::nullopt, std::nullopt);
     BOOST_REQUIRE_EQUAL(topo.find_node(id1), node);
     mutable_node = const_cast<locator::node*>(node);
 
@@ -182,7 +183,7 @@ SEASTAR_THREAD_TEST_CASE(test_add_or_update_by_host_id) {
     // We need to make the second node 'being_decommissioned', so that
     // it gets removed from ip index and we don't get the non-unique IP error.
 
-    auto topo = topology({}, topology::key_kind::host_id);
+    auto topo = topology({});
     //auto topo = topology({});
     topo.add_node(id1, gms::inet_address{}, endpoint_dc_rack::default_location, node::state::normal);
     topo.add_node(id2, ep1, endpoint_dc_rack::default_location, node::state::being_decommissioned);
@@ -226,7 +227,7 @@ SEASTAR_THREAD_TEST_CASE(test_remove_endpoint) {
         .local_dc_rack = dc_rack1
     };
 
-    auto topo = topology(cfg, topology::key_kind::inet_address);
+    auto topo = topology(cfg);
 
     topo.add_node(id1, ep1, dc_rack1, node::state::normal);
     topo.add_node(id2, ep2, dc_rack2, node::state::normal);