From ffbfa3b542e88d628307bc694cb888c2900cb5bd Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 2 Mar 2022 14:14:20 +0300 Subject: [PATCH] storage_service: Remove _joined and is_joined() The is_joined() status can be get with get_operation_mode(). Since it indicates that the operation mode is JOINING, NORMAL or anything above, the operation mode the enum class should be shuffled to get the simple >= comparison. Another needed change is to set mode few steps earlier than it happens now to cover the non-bootstrap startup case. And the third change is to partially revert the d49aa7ab that made the .is_joined() method be future-less. Nowadays the is_joined() is called only from the API which is happy with being future-full in all other storage service state checks. Signed-off-by: Pavel Emelyanov --- api/storage_service.cc | 4 +++- service/storage_service.cc | 8 ++------ service/storage_service.hh | 9 +-------- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/api/storage_service.cc b/api/storage_service.cc index 62c9381273..6c96bfba00 100644 --- a/api/storage_service.cc +++ b/api/storage_service.cc @@ -850,7 +850,9 @@ void set_storage_service(http_context& ctx, routes& r, sharded req) { - return make_ready_future(ss.local().is_joined()); + return ss.local().get_operation_mode().then([] (auto mode) { + return make_ready_future(mode >= service::storage_service::mode::JOINING); + }); }); ss::set_stream_throughput_mb_per_sec.set(r, [](std::unique_ptr req) { diff --git a/service/storage_service.cc b/service/storage_service.cc index ea30f768fa..897b9cd99d 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -413,11 +413,7 @@ future<> storage_service::wait_for_ring_to_settle(std::chrono::milliseconds dela // Runs inside seastar::async context void storage_service::join_token_ring(std::chrono::milliseconds delay) { - // This function only gets called on shard 0, but we want to set _joined - // on all shards, so this variable can be later read locally. - container().invoke_on_all([] (auto&& ss) { - ss._joined = true; - }).get(); + set_mode(mode::JOINING, "joining the ring", true); _group0->join_group0().get(); @@ -437,7 +433,7 @@ void storage_service::join_token_ring(std::chrono::milliseconds delay) { } else { db::system_keyspace::set_bootstrap_state(db::system_keyspace::bootstrap_state::IN_PROGRESS).get(); } - set_mode(mode::JOINING, "waiting for ring information", true); + slogger.info("waiting for ring information"); // if our schema hasn't matched yet, keep sleeping until it does // (post CASSANDRA-1391 we don't expect this to be necessary very often, but it doesn't hurt to be careful) diff --git a/service/storage_service.hh b/service/storage_service.hh index bba86cbe78..28925fb839 100644 --- a/service/storage_service.hh +++ b/service/storage_service.hh @@ -279,10 +279,8 @@ private: bool _initialized = false; - bool _joined = false; - public: - enum class mode { STARTING, NORMAL, JOINING, LEAVING, DECOMMISSIONED, MOVING, DRAINING, DRAINED }; + enum class mode { STARTING, JOINING, NORMAL, LEAVING, DECOMMISSIONED, MOVING, DRAINING, DRAINED }; private: mode _operation_mode = mode::STARTING; friend std::ostream& operator<<(std::ostream& os, const mode& mode); @@ -398,11 +396,6 @@ private: void join_token_ring(std::chrono::milliseconds); void maybe_start_sys_dist_ks(); public: - inline bool is_joined() const { - // Every time we set _joined, we do it on all shards, so we can read its - // value locally. - return _joined; - } future<> rebuild(sstring source_dc);