From b6061bb97d4ec8e4f210088b72ca801377e32e9b Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 19 Sep 2022 16:15:23 +0300 Subject: [PATCH] topology: Move all post-configuration to topology::config Because of snitch ex-dependencies some bits on topology were initialized with nasty post-start calls. Now it all can be removed and the initial topology information can be provided by topology::config Signed-off-by: Pavel Emelyanov --- locator/token_metadata.cc | 19 ++---------------- locator/token_metadata.hh | 12 ++---------- main.cc | 41 +++++++++++++-------------------------- test/lib/cql_test_env.cc | 5 +---- tools/schema_loader.cc | 1 + 5 files changed, 20 insertions(+), 58 deletions(-) diff --git a/locator/token_metadata.cc b/locator/token_metadata.cc index 8a19fd5367..0d743e180e 100644 --- a/locator/token_metadata.cc +++ b/locator/token_metadata.cc @@ -282,7 +282,6 @@ public: size_t count_normal_token_owners() const; private: future<> update_normal_token_owners(); - void init_local_endpoint(endpoint_dc_rack) noexcept; public: // returns empty vector if keyspace_name not found. @@ -1256,7 +1255,9 @@ inline future<> topology::clear_gently() noexcept { } topology::topology(config cfg) + : _sort_by_proximity(!cfg.disable_proximity_sorting) { + _pending_locations[utils::fb_utilities::get_broadcast_address()] = std::move(cfg.local_dc_rack); } topology::topology(const topology& other) @@ -1431,20 +1432,4 @@ future<> shared_token_metadata::mutate_token_metadata(seastar::noncopyable_funct set(make_token_metadata_ptr(std::move(tm))); } -void shared_token_metadata::init_local_endpoint(endpoint_dc_rack dr) noexcept { - _shared->init_local_endpoint(std::move(dr)); -} - -void token_metadata::init_local_endpoint(endpoint_dc_rack dr) noexcept { - _impl->init_local_endpoint(std::move(dr)); -} - -void token_metadata_impl::init_local_endpoint(endpoint_dc_rack dr) noexcept { - _topology.init_local_endpoint(std::move(dr)); -} - -void topology::init_local_endpoint(endpoint_dc_rack dr) noexcept { - _pending_locations[utils::fb_utilities::get_broadcast_address()] = std::move(dr); -} - } // namespace locator diff --git a/locator/token_metadata.hh b/locator/token_metadata.hh index f6dfca4b88..f938952039 100644 --- a/locator/token_metadata.hh +++ b/locator/token_metadata.hh @@ -46,6 +46,8 @@ struct endpoint_dc_rack { class topology { public: struct config { + endpoint_dc_rack local_dc_rack; + bool disable_proximity_sorting = false; }; topology(config cfg); topology(const topology& other); @@ -105,12 +107,6 @@ public: */ void sort_by_proximity(inet_address address, inet_address_vector_replica_set& addresses) const; - void disable_proximity_sorting() noexcept { - _sort_by_proximity = false; - } - - void init_local_endpoint(endpoint_dc_rack) noexcept; - private: /** * compares two endpoints in relation to the target endpoint, returning as @@ -171,7 +167,6 @@ private: public: token_metadata(config cfg); - void init_local_endpoint(endpoint_dc_rack) noexcept; explicit token_metadata(std::unique_ptr impl); token_metadata(token_metadata&&) noexcept; // Can't use "= default;" - hits some static_assert in unique_ptr token_metadata& operator=(token_metadata&&) noexcept; @@ -385,9 +380,6 @@ public: return _lock_func(); } - // FIXME -- snitch should start early and provide this info via constructor - void init_local_endpoint(endpoint_dc_rack) noexcept; - // mutate_token_metadata acquires the shared_token_metadata lock, // clones the token_metadata (using clone_async) // and calls an asynchronous functor on diff --git a/main.cc b/main.cc index fccf5579e7..d6f578c799 100644 --- a/main.cc +++ b/main.cc @@ -728,6 +728,20 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl supervisor::notify("starting tokens manager"); locator::token_metadata::config tm_cfg; + tm_cfg.topo_cfg.local_dc_rack = { snitch.local()->get_datacenter(), snitch.local()->get_rack() }; + if (snitch.local()->get_name() == "org.apache.cassandra.locator.SimpleSnitch") { + // + // Simple snitch wants sort_by_proximity() not to reorder nodes anyhow + // + // "Making all endpoints equal ensures we won't change the original + // ordering." - quote from C* code. + // + // The snitch_base implementation should handle the above case correctly. + // I'm leaving the this implementation anyway since it's the C*'s + // implementation and some installations may depend on it. + // + tm_cfg.topo_cfg.disable_proximity_sorting = true; + } token_metadata.start([] () noexcept { return db::schema_tables::hold_merge_lock(); }, tm_cfg).get(); // storage_proxy holds a reference on it and is not yet stopped. // what's worse is that the calltrace @@ -903,33 +917,6 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl }); gossiper.invoke_on_all(&gms::gossiper::start).get(); - if (snitch.local()->get_name() == "org.apache.cassandra.locator.SimpleSnitch") { - // - // Simple snitch wants sort_by_proximity() not to reorder nodes anyhow - // - // "Making all endpoints equal ensures we won't change the original - // ordering." - quote from C* code. - // - // The snitch_base implementation should handle the above case correctly. - // I'm leaving the this implementation anyway since it's the C*'s - // implementation and some installations may depend on it. - // - token_metadata.invoke_on_all([] (shared_token_metadata& tm) mutable { - const auto& topo = tm.get()->get_topology(); - // There's no real need in mutate_token_metadata here as it just - // sets a single boolean bit on topology object, this change is - // never ever performed again and by this point no code uses neither - // topology nor the token metadata itself - const_cast(topo).disable_proximity_sorting(); - }).get(); - } - - // FIXME -- token metadata should get local DC/RACK via constructor, - // but snitch cannot (yet) start early enough to provide it - token_metadata.invoke_on_all([&snitch] (auto& tm) { - tm.init_local_endpoint({ snitch.local()->get_datacenter(), snitch.local()->get_rack() }); - }).get(); - static direct_fd_clock fd_clock; static sharded fd; supervisor::notify("starting direct failure detector service"); diff --git a/test/lib/cql_test_env.cc b/test/lib/cql_test_env.cc index 8a37082f21..29d1352bc4 100644 --- a/test/lib/cql_test_env.cc +++ b/test/lib/cql_test_env.cc @@ -532,6 +532,7 @@ public: sharded token_metadata; locator::token_metadata::config tm_cfg; + tm_cfg.topo_cfg.local_dc_rack = { snitch.local()->get_datacenter(), snitch.local()->get_rack() }; token_metadata.start([] () noexcept { return db::schema_tables::hold_merge_lock(); }, tm_cfg).get(); auto stop_token_metadata = defer([&token_metadata] { token_metadata.stop().get(); }); @@ -609,10 +610,6 @@ public: }); gossiper.invoke_on_all(&gms::gossiper::start).get(); - token_metadata.invoke_on_all([&snitch] (auto& tm) { - tm.init_local_endpoint({ snitch.local()->get_datacenter(), snitch.local()->get_rack() }); - }).get(); - distributed& proxy = service::get_storage_proxy(); distributed mm; sharded cql_config; diff --git a/tools/schema_loader.cc b/tools/schema_loader.cc index ad330c2c5e..14eb036396 100644 --- a/tools/schema_loader.cc +++ b/tools/schema_loader.cc @@ -198,6 +198,7 @@ std::vector do_load_schemas(std::string_view schema_str) { feature_service.enable(feature_service.supported_feature_set()); sharded token_metadata; + utils::fb_utilities::set_broadcast_address(gms::inet_address("localhost")); token_metadata.start([] () noexcept { return db::schema_tables::hold_merge_lock(); }, locator::token_metadata::config{}).get(); auto stop_token_metadata = deferred_stop(token_metadata);