From 37adcd3ecf7cedc9336f6bec864dcbca105a91c8 Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Wed, 20 Mar 2024 12:52:50 +0200 Subject: [PATCH 1/2] storage_service: add my_host_id Shorthand for getting this node's host_id from token_metadata.topology, similar to the `get_broadcast_address` helper. Signed-off-by: Benny Halevy --- service/storage_service.cc | 2 +- service/storage_service.hh | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/service/storage_service.cc b/service/storage_service.cc index 45c02090d9..07eb28303c 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -6701,7 +6701,7 @@ future<> storage_service::start_maintenance_mode() { set_mode(mode::MAINTENANCE); return mutate_token_metadata([this] (mutable_token_metadata_ptr token_metadata) -> future<> { - return token_metadata->update_normal_tokens({ dht::token{} }, get_token_metadata_ptr()->get_topology().my_host_id()); + return token_metadata->update_normal_tokens({ dht::token{} }, my_host_id()); }, acquire_merge_lock::yes); } diff --git a/service/storage_service.hh b/service/storage_service.hh index 4b43a8c559..578105a137 100644 --- a/service/storage_service.hh +++ b/service/storage_service.hh @@ -284,6 +284,9 @@ private: inet_address get_broadcast_address() const noexcept { return get_token_metadata_ptr()->get_topology().my_address(); } + locator::host_id my_host_id() const noexcept { + return get_token_metadata_ptr()->get_topology().my_host_id(); + } bool is_me(inet_address addr) const noexcept { return get_token_metadata_ptr()->get_topology().is_me(addr); } From fceb1183d38714d1d9e52276377ab1b1a6cd69ba Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Wed, 20 Mar 2024 12:48:38 +0200 Subject: [PATCH 2/2] cdc: should_propose_first_generation: get my_host_id from caller There is no need to map this node's inet_address to host_id. The storage_service can easily just pass the local host_id. While at it, get the other node's host_id directly from their endpoint_state instead of looking it up yet again in the gossiper, using the nodes' address. Refs #12283 Signed-off-by: Benny Halevy --- cdc/generation.cc | 7 +++---- cdc/generation.hh | 2 +- service/storage_service.cc | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/cdc/generation.cc b/cdc/generation.cc index a881a74d6a..cca12927cc 100644 --- a/cdc/generation.cc +++ b/cdc/generation.cc @@ -194,10 +194,9 @@ static std::vector create_stream_ids( return result; } -bool should_propose_first_generation(const gms::inet_address& me, const gms::gossiper& g) { - auto my_host_id = g.get_host_id(me); - return g.for_each_endpoint_state_until([&] (const gms::inet_address& node, const gms::endpoint_state& eps) { - return stop_iteration(my_host_id < g.get_host_id(node)); +bool should_propose_first_generation(const locator::host_id& my_host_id, const gms::gossiper& g) { + return g.for_each_endpoint_state_until([&] (const gms::inet_address&, const gms::endpoint_state& eps) { + return stop_iteration(my_host_id < eps.get_host_id()); }) == stop_iteration::no; } diff --git a/cdc/generation.hh b/cdc/generation.hh index a4f34db5df..8cfb18613c 100644 --- a/cdc/generation.hh +++ b/cdc/generation.hh @@ -133,7 +133,7 @@ public: * The chosen condition is arbitrary, it only tries to make sure that no two nodes propose a generation of streams * when upgrading, and nothing bad happens if they for some reason do (it's mostly an optimization). */ -bool should_propose_first_generation(const gms::inet_address& me, const gms::gossiper&); +bool should_propose_first_generation(const locator::host_id& me, const gms::gossiper&); /* * Checks if the CDC generation is optimal, which is true if its `topology_description` is consistent diff --git a/service/storage_service.cc b/service/storage_service.cc index 07eb28303c..8d90fb8339 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -1798,7 +1798,7 @@ future<> storage_service::join_token_ring(sharded