token_metadata: topology: cleanup add_or_update_endpoint

Make host_id parameter non-optional and
move it to the beginning of the arguments list.

Delete unused overloads of add_or_update_endpoint.

Delete unused overload of token_metadata::update_topology
with inet_address argument.

locator/token_metadata.cc

@@ -115,12 +115,8 @@ public:
         return _bootstrap_tokens;
     }
 
-    void update_topology(inet_address ep, std::optional<endpoint_dc_rack> opt_dr, std::optional<node::state> opt_st, std::optional<shard_id> shard_count = std::nullopt) {
-        _topology.add_or_update_endpoint(ep, std::nullopt, std::move(opt_dr), std::move(opt_st), std::move(shard_count));
-    }
-
-    void update_topology(host_id ep, std::optional<endpoint_dc_rack> opt_dr, std::optional<node::state> opt_st, std::optional<shard_id> shard_count = std::nullopt) {
-        _topology.add_or_update_endpoint(std::nullopt, ep, std::move(opt_dr), std::move(opt_st), std::move(shard_count));
+    void update_topology(host_id id, std::optional<endpoint_dc_rack> opt_dr, std::optional<node::state> opt_st, std::optional<shard_id> shard_count = std::nullopt) {
+        _topology.add_or_update_endpoint(id, std::nullopt, std::move(opt_dr), std::move(opt_st), std::move(shard_count));
     }
 
     /**
@@ -520,7 +516,7 @@ void token_metadata_impl::debug_show() const {
 }
 
 void token_metadata_impl::update_host_id(const host_id& host_id, inet_address endpoint) {
-    _topology.add_or_update_endpoint(endpoint, host_id);
+    _topology.add_or_update_endpoint(host_id, endpoint);
 }
 
 host_id token_metadata_impl::get_host_id(inet_address endpoint) const {

locator/topology.cc

@@ -442,25 +442,22 @@ const node* topology::find_node(node::idx_type idx) const noexcept {
     return _nodes.at(idx).get();
 }
 
-const node* topology::add_or_update_endpoint(std::optional<inet_address> opt_ep, std::optional<host_id> opt_id, std::optional<endpoint_dc_rack> opt_dr, std::optional<node::state> opt_st, std::optional<shard_id> shard_count)
+const node* topology::add_or_update_endpoint(host_id id, std::optional<inet_address> opt_ep, std::optional<endpoint_dc_rack> opt_dr, std::optional<node::state> opt_st, std::optional<shard_id> shard_count)
 {
     if (tlogger.is_enabled(log_level::trace)) {
-        tlogger.trace("topology[{}]: add_or_update_endpoint: ep={} host_id={} dc={} rack={} state={} shards={}, at {}", fmt::ptr(this),
-            opt_ep, opt_id.value_or(host_id::create_null_id()), opt_dr.value_or(endpoint_dc_rack{}).dc, opt_dr.value_or(endpoint_dc_rack{}).rack, opt_st.value_or(node::state::none), shard_count,
+        tlogger.trace("topology[{}]: add_or_update_endpoint: host_id={} ep={} dc={} rack={} state={} shards={}, at {}", fmt::ptr(this),
+            id, opt_ep, opt_dr.value_or(endpoint_dc_rack{}).dc, opt_dr.value_or(endpoint_dc_rack{}).rack, opt_st.value_or(node::state::none), shard_count,
             current_backtrace());
     }
 
-    if (!opt_id) {
-        on_internal_error(tlogger, format("topology: host_id is not set, ep={}", opt_ep));
-    }
-    const auto* n = find_node(*opt_id);
+    const auto* n = find_node(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 update_node(make_mutable(n), 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()),
+    return add_node(id,
                     opt_ep.value_or(inet_address{}),
                     opt_dr.value_or(endpoint_dc_rack::default_location),
                     opt_st.value_or(node::state::normal),

locator/topology.hh

@@ -234,19 +234,11 @@ public:
      *
      * Adds or updates a node with given endpoint
      */
-    const node* add_or_update_endpoint(std::optional<inet_address> ep, std::optional<host_id> opt_id,
-                                       std::optional<endpoint_dc_rack> opt_dr,
-                                       std::optional<node::state> opt_st,
+    const node* add_or_update_endpoint(host_id id, std::optional<inet_address> opt_ep,
+                                       std::optional<endpoint_dc_rack> opt_dr = std::nullopt,
+                                       std::optional<node::state> opt_st = std::nullopt,
                                        std::optional<shard_id> shard_count = std::nullopt);
 
-    // Legacy entry point from token_metadata::update_topology
-    const node* add_or_update_endpoint(inet_address ep, endpoint_dc_rack dr, std::optional<node::state> opt_st) {
-        return add_or_update_endpoint(ep, std::nullopt, std::move(dr), std::move(opt_st), std::nullopt);
-    }
-    const node* add_or_update_endpoint(inet_address ep, host_id id) {
-        return add_or_update_endpoint(ep, id, std::nullopt, std::nullopt, std::nullopt);
-    }
-
     /**
      * Removes current DC/rack assignment for ep
      * Returns true if the node was found and removed.

main.cc

@@ -1211,7 +1211,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl
               // Raft topology discard the endpoint-to-id map, so the local id can
               // still be found in the config.
               tm.get_topology().set_host_id_cfg(host_id);
-              tm.get_topology().add_or_update_endpoint(endpoint, host_id);
+              tm.get_topology().add_or_update_endpoint(host_id, endpoint);
               return make_ready_future<>();
           }).get();
 

test/boost/locator_topology_test.cc

@@ -110,7 +110,7 @@ SEASTAR_THREAD_TEST_CASE(test_update_node) {
         set_abort_on_internal_error(true);
     });
 
-    topo.add_or_update_endpoint(std::nullopt, id1, endpoint_dc_rack::default_location, node::state::normal);
+    topo.add_or_update_endpoint(id1, std::nullopt, endpoint_dc_rack::default_location, node::state::normal);
 
     auto node = topo.this_node();
     auto mutable_node = const_cast<locator::node*>(node);
@@ -188,7 +188,7 @@ SEASTAR_THREAD_TEST_CASE(test_add_or_update_by_host_id) {
     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);
 
-    topo.add_or_update_endpoint(ep1, id1, std::nullopt, node::state::bootstrapping);
+    topo.add_or_update_endpoint(id1, ep1, std::nullopt, node::state::bootstrapping);
 
     auto* n = topo.find_node(id1);
     BOOST_REQUIRE_EQUAL(n->get_state(), node::state::bootstrapping);

test/boost/network_topology_strategy_test.cc

@@ -955,7 +955,7 @@ SEASTAR_THREAD_TEST_CASE(test_topology_tracks_local_node) {
 
     stm.mutate_token_metadata([&] (token_metadata& tm) -> future<> {
         co_await tm.clear_gently();
-        tm.get_topology().add_or_update_endpoint(ip1, host1, ip1_dc_rack_v2, node::state::being_decommissioned);
+        tm.get_topology().add_or_update_endpoint(host1, ip1, ip1_dc_rack_v2, node::state::being_decommissioned);
     }).get();
 
     n1 = stm.get()->get_topology().find_node(host1);

test/boost/tablets_test.cc

@@ -434,7 +434,7 @@ SEASTAR_TEST_CASE(test_sharder) {
         auto table1 = table_id(utils::UUID_gen::get_time_UUID());
 
         token_metadata tokm(token_metadata::config{ .topo_cfg{ .this_host_id = h1 } });
-        tokm.get_topology().add_or_update_endpoint(tokm.get_topology().my_address(), h1);
+        tokm.get_topology().add_or_update_endpoint(h1, tokm.get_topology().my_address());
 
         std::vector<tablet_id> tablet_ids;
         {

test/lib/cql_test_env.cc

@@ -643,11 +643,11 @@ private:
             locator::shared_token_metadata::mutate_on_all_shards(_token_metadata, [hostid = host_id, &cfg_in] (locator::token_metadata& tm) {
                 auto& topo = tm.get_topology();
                 topo.set_host_id_cfg(hostid);
-                topo.add_or_update_endpoint(cfg_in.broadcast_address,
-                                                hostid,
-                                                std::nullopt,
-                                                locator::node::state::normal,
-                                                smp::count);
+                topo.add_or_update_endpoint(hostid,
+                                            cfg_in.broadcast_address,
+                                            std::nullopt,
+                                            locator::node::state::normal,
+                                            smp::count);
                 return make_ready_future<>();
             }).get();