From bf6679906fb5200756aa5d01aeedab8abcdb454d Mon Sep 17 00:00:00 2001 From: Kamil Braun Date: Tue, 6 Dec 2022 15:19:31 +0100 Subject: [PATCH] gms, service: stop gossiping and storing RAFT_SERVER_ID It is equal to (if present) HOST_ID and no longer used for anything. The application state was only gossiped if `experimental-features` contained `raft`, so we can free this slot. Similarly, `raft_server_id`s were only persisted in `system.peers` if the `SUPPORTS_RAFT` cluster feature was enabled, which happened only when `experimental-features` contained `raft`. The `raft_server_id` field in the schema was also introduced recently in `master` and didn't get to be in a release yet. Given either of these reasons, we can remove this field safely. --- db/system_keyspace.cc | 12 +--------- db/system_keyspace.hh | 4 ---- gms/application_state.cc | 1 - gms/application_state.hh | 5 ---- gms/gossiper.cc | 4 ---- gms/versioned_value.hh | 4 ---- service/raft/raft_group0.cc | 46 ------------------------------------- service/raft/raft_group0.hh | 4 ---- service/storage_service.cc | 11 --------- 9 files changed, 1 insertion(+), 90 deletions(-) diff --git a/db/system_keyspace.cc b/db/system_keyspace.cc index 9a3a70862f..29abdeb123 100644 --- a/db/system_keyspace.cc +++ b/db/system_keyspace.cc @@ -355,7 +355,7 @@ schema_ptr system_keyspace::built_indexes() { } /*static*/ schema_ptr system_keyspace::peers() { - constexpr uint16_t schema_version_offset = 1; // raft_server_id + constexpr uint16_t schema_version_offset = 0; static thread_local auto peers = [] { schema_builder builder(generate_legacy_id(NAME, PEERS), NAME, PEERS, // partition key @@ -373,7 +373,6 @@ schema_ptr system_keyspace::built_indexes() { {"schema_version", uuid_type}, {"tokens", set_type_impl::get_instance(utf8_type, true)}, {"supported_features", utf8_type}, - {"raft_server_id", uuid_type}, }, // static columns {}, @@ -1543,15 +1542,6 @@ future> system_keyspace: }); } -future> system_keyspace::get_peer_raft_id_if_known(gms::inet_address ip_addr) { - sstring req = format("SELECT raft_server_id FROM system.{} WHERE peer = ?", PEERS); - shared_ptr res = co_await execute_cql(req, ip_addr.addr()); - if (res->empty() || !res->one().has("raft_server_id")) { - co_return std::nullopt; - } - co_return res->one().get_as("raft_server_id"); -} - future> system_keyspace::load_peers() { auto res = co_await execute_cql(format("SELECT peer, tokens FROM system.{}", PEERS)); assert(res); diff --git a/db/system_keyspace.hh b/db/system_keyspace.hh index 7d9bfec391..3dfa5ae06a 100644 --- a/db/system_keyspace.hh +++ b/db/system_keyspace.hh @@ -347,10 +347,6 @@ public: */ future> load_host_ids(); - // Load the list of raft server ids and their ips known to - // gossip. - future> get_peer_raft_id_if_known(gms::inet_address ip_addr); - future> load_peers(); /* diff --git a/gms/application_state.cc b/gms/application_state.cc index 7737f48718..58abe7d4ce 100644 --- a/gms/application_state.cc +++ b/gms/application_state.cc @@ -25,7 +25,6 @@ static const std::map application_state_names = { {application_state::REMOVAL_COORDINATOR, "REMOVAL_COORDINATOR"}, {application_state::INTERNAL_IP, "INTERNAL_IP"}, {application_state::RPC_ADDRESS, "RPC_ADDRESS"}, - {application_state::RAFT_SERVER_ID, "RAFT_SERVER_ID"}, {application_state::SEVERITY, "SEVERITY"}, {application_state::NET_VERSION, "NET_VERSION"}, {application_state::HOST_ID, "HOST_ID"}, diff --git a/gms/application_state.hh b/gms/application_state.hh index aacde1a38e..069d211814 100644 --- a/gms/application_state.hh +++ b/gms/application_state.hh @@ -38,11 +38,6 @@ enum class application_state { IGNORE_MSB_BITS, CDC_GENERATION_ID, SNITCH_NAME, - // RAFT ID is a server identifier which is maintained - // and gossiped in addition to HOST_ID because it's truly - // unique: any new node gets a new RAFT ID, while may keep - // its existing HOST ID, e.g. if it's replacing an existing node. - RAFT_SERVER_ID, }; std::ostream& operator<<(std::ostream& os, const application_state& m); diff --git a/gms/gossiper.cc b/gms/gossiper.cc index 9b56bcb5d6..71e00eea96 100644 --- a/gms/gossiper.cc +++ b/gms/gossiper.cc @@ -2003,10 +2003,6 @@ future<> gossiper::add_saved_endpoint(inet_address ep) { if (host_id) { ep_state.add_application_state(gms::application_state::HOST_ID, versioned_value::host_id(host_id.value())); } - auto raft_id = co_await _sys_ks.local().get_peer_raft_id_if_known(ep); - if (raft_id) { - ep_state.add_application_state(gms::application_state::RAFT_SERVER_ID, versioned_value::raft_server_id(raft_id.value())); - } ep_state.mark_dead(); _endpoint_state_map[ep] = ep_state; co_await replicate(ep, ep_state); diff --git a/gms/versioned_value.hh b/gms/versioned_value.hh index 8b099cb5c6..f3abd657f7 100644 --- a/gms/versioned_value.hh +++ b/gms/versioned_value.hh @@ -151,10 +151,6 @@ public: return versioned_value(host_id.to_sstring()); } - static versioned_value raft_server_id(const utils::UUID& id) { - return versioned_value(id.to_sstring()); - } - static versioned_value tokens(const std::unordered_set& tokens) { return versioned_value(make_full_token_string(tokens)); } diff --git a/service/raft/raft_group0.cc b/service/raft/raft_group0.cc index eb18ea41ff..824ba68e45 100644 --- a/service/raft/raft_group0.cc +++ b/service/raft/raft_group0.cc @@ -591,51 +591,6 @@ future<> raft_group0::setup_group0( co_await _client.set_group0_upgrade_state(group0_upgrade_state::use_post_raft_procedures); } -future<> raft_group0::persist_initial_raft_address_map() { - static constexpr auto max_concurrency = 10; - std::vector endpoints; - const auto& states = _gossiper.get_endpoint_states(); - endpoints.reserve(states.size()); - std::transform(states.begin(), states.end(), std::back_inserter(endpoints), [](const auto& it) { return it.first; }); - // - // Gossiper only persists node state when it switches to - // NORMAL. When persisting a node state, Raft ID is only - // persisted if the cluster supports raft. Some nodes switch - // to NORMAL before the whole cluster enables raft. For these - // nodes we may never persist their Raft ID unless, here, as - // soon as we discover that the entire cluster supports Raft, - // we go over these nodes and explicitly persist their RAFT - // IDs. - - co_await max_concurrent_for_each(endpoints, max_concurrency, [this] (const gms::inet_address& ip_addr) -> future<> { - auto* state = _gossiper.get_endpoint_state_for_endpoint_ptr(ip_addr); - if (state == nullptr) { - co_return; - } - if (!state->is_normal()) { - // Handle only NORMAL states, others should not be - // persisted to avoid garbage in the peers table if - // bootstrap fails. - co_return; - } - auto* value = state->get_application_state_ptr(gms::application_state::RAFT_SERVER_ID); - if (value == nullptr) { - co_return; - } - auto server_id = utils::UUID(value->value); - if (server_id == utils::UUID{}) { - upgrade_log.error("empty raft server id for host {} ", ip_addr); - co_return; - } - try { - co_await _sys_ks.update_peer_info(ip_addr, "raft_server_id", server_id); - } catch (...) { - upgrade_log.error("failed to persist raft_server_id for {}: {}", - ip_addr, std::current_exception()); - } - }); -} - void raft_group0::load_initial_raft_address_map() { for (auto& [ip_addr, state] : _gossiper.get_endpoint_states()) { auto* value = state.get_application_state_ptr(gms::application_state::HOST_ID); @@ -1476,7 +1431,6 @@ future<> raft_group0::upgrade_to_group0() { break; case group0_upgrade_state::use_pre_raft_procedures: upgrade_log.info("starting in `use_pre_raft_procedures` state."); - co_await persist_initial_raft_address_map(); break; } diff --git a/service/raft/raft_group0.hh b/service/raft/raft_group0.hh index fd34759eac..3d75e49ad9 100644 --- a/service/raft/raft_group0.hh +++ b/service/raft/raft_group0.hh @@ -250,10 +250,6 @@ private: // Remove the node from raft config, retries raft::commit_status_unknown. future<> remove_from_raft_config(raft::server_id id); - // Persist the initial Raft <-> IP address map as seen by - // the gossiper. - future<> persist_initial_raft_address_map(); - // Load the initial Raft <-> IP address map as seen by // the gossiper. void load_initial_raft_address_map(); diff --git a/service/storage_service.cc b/service/storage_service.cc index 8ee33e4fe0..e8cf1ea40c 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -414,10 +414,6 @@ future<> storage_service::join_token_ring(cdc::generation_service& cdc_gen_servi app_states.emplace(gms::application_state::NET_VERSION, versioned_value::network_version()); app_states.emplace(gms::application_state::HOST_ID, versioned_value::host_id(local_host_id)); app_states.emplace(gms::application_state::RPC_ADDRESS, versioned_value::rpcaddress(broadcast_rpc_address)); - if (_group0->is_raft_enabled()) { - auto my_id = _group0->load_my_id(); - app_states.emplace(gms::application_state::RAFT_SERVER_ID, versioned_value::raft_server_id(my_id.id)); - } app_states.emplace(gms::application_state::RELEASE_VERSION, versioned_value::release_version()); app_states.emplace(gms::application_state::SUPPORTED_FEATURES, versioned_value::supported_features(features)); app_states.emplace(gms::application_state::CACHE_HITRATES, versioned_value::cache_hitrates("")); @@ -1327,13 +1323,6 @@ future<> storage_service::do_update_system_peers_table(gms::inet_address endpoin co_await update_table(endpoint, "schema_version", utils::UUID(value.value)); } else if (state == application_state::HOST_ID) { co_await update_table(endpoint, "host_id", utils::UUID(value.value)); - } else if (state == application_state::RAFT_SERVER_ID && _feature_service.supports_raft_cluster_mgmt) { - auto server_id = utils::UUID(value.value); - if (server_id == utils::UUID{}) { - slogger.error("empty raft server id in application state for host {} ", endpoint); - } else { - co_await update_table(endpoint, "raft_server_id", server_id); - } } else if (state == application_state::SUPPORTED_FEATURES) { co_await update_table(endpoint, "supported_features", value.value); }