From 03ffaea7687cf6da85486a89858a351b0a309040 Mon Sep 17 00:00:00 2001 From: Vlad Zolotarov Date: Thu, 11 Jun 2015 21:44:53 +0300 Subject: [PATCH 1/3] locator: introduce i_endpoint_snitch::create_snitch() - Kill make_snitch(). - i_endpoint_snitch::create_snitch() uses the utilities from class_registrator.hh. Signed-off-by: Vlad Zolotarov --- configure.py | 2 + database.cc | 4 +- locator/gossiping_property_file_snitch.cc | 7 +++ locator/gossiping_property_file_snitch.hh | 5 +- locator/rack_inferring_snitch.cc | 6 ++ locator/rack_inferring_snitch.hh | 7 +-- locator/simple_snitch.cc | 7 +++ locator/simple_snitch.hh | 12 ++-- locator/snitch_base.cc | 3 + locator/snitch_base.hh | 56 +++++++++++-------- .../gossiping_property_file_snitch_test.cc | 4 +- 11 files changed, 70 insertions(+), 43 deletions(-) create mode 100644 locator/rack_inferring_snitch.cc create mode 100644 locator/simple_snitch.cc diff --git a/configure.py b/configure.py index 1427ee3191..5b73480439 100755 --- a/configure.py +++ b/configure.py @@ -452,6 +452,8 @@ urchin_core = (['database.cc', 'locator/token_metadata.cc', 'locator/locator.cc', 'locator/snitch_base.cc', + 'locator/simple_snitch.cc', + 'locator/rack_inferring_snitch.cc', 'locator/gossiping_property_file_snitch.cc', 'message/messaging_service.cc', 'service/storage_service.cc', diff --git a/database.cc b/database.cc index f138e62035..810da87b7c 100644 --- a/database.cc +++ b/database.cc @@ -721,7 +721,9 @@ keyspace::create_replication_strategy() { tm.update_normal_token({dht::token::kind::key, {d2t(2.0/4).data(), 8}}, to_sstring("127.0.0.3")); tm.update_normal_token({dht::token::kind::key, {d2t(3.0/4).data(), 8}}, to_sstring("127.0.0.4")); - return make_snitch().then( + // Fixme + return i_endpoint_snitch::create_snitch( + "org.apache.cassandra.locator.SimpleSnitch").then( [this] (snitch_ptr&& s) { _replication_strategy = abstract_replication_strategy::create_replication_strategy( diff --git a/locator/gossiping_property_file_snitch.cc b/locator/gossiping_property_file_snitch.cc index a2724a86fb..e9852f5528 100644 --- a/locator/gossiping_property_file_snitch.cc +++ b/locator/gossiping_property_file_snitch.cc @@ -279,4 +279,11 @@ void gossiping_property_file_snitch::reload_gossiper_state() #endif // else this will eventually rerun at gossiperStarting() } + +namespace locator { +using registry = class_registrator; +static registry registrator("org.apache.cassandra.locator.GossipingPropertyFileSnitch"); +} } // namespace locator diff --git a/locator/gossiping_property_file_snitch.hh b/locator/gossiping_property_file_snitch.hh index 7630a765c7..bedfb30ba2 100644 --- a/locator/gossiping_property_file_snitch.hh +++ b/locator/gossiping_property_file_snitch.hh @@ -54,13 +54,10 @@ public: virtual void gossiper_starting() override; virtual future<> stop() override; -private: - template - friend future make_snitch(A&&... a); - gossiping_property_file_snitch( const sstring& fname = snitch_properties_filename); +private: static logging::logger& logger() { static thread_local logging::logger l("gossiping_property_file_snitch"); diff --git a/locator/rack_inferring_snitch.cc b/locator/rack_inferring_snitch.cc new file mode 100644 index 0000000000..2f97aced37 --- /dev/null +++ b/locator/rack_inferring_snitch.cc @@ -0,0 +1,6 @@ +#include "locator/rack_inferring_snitch.hh" + +namespace locator { +using registry = class_registrator; +static registry registrator("org.apache.cassandra.locator.RackInferringSnitch"); +} diff --git a/locator/rack_inferring_snitch.hh b/locator/rack_inferring_snitch.hh index bc6eb2ba3c..b307abae11 100644 --- a/locator/rack_inferring_snitch.hh +++ b/locator/rack_inferring_snitch.hh @@ -34,11 +34,7 @@ using inet_address = gms::inet_address; * A simple endpoint snitch implementation that assumes datacenter and rack information is encoded * in the 2nd and 3rd octets of the ip address, respectively. */ -class rack_inferring_snitch : public snitch_base { -private: - template - friend future make_snitch(A&&... a); - +struct rack_inferring_snitch : public snitch_base { rack_inferring_snitch() { _my_dc = get_datacenter(utils::fb_utilities::get_broadcast_address()); _my_rack = get_rack(utils::fb_utilities::get_broadcast_address()); @@ -47,7 +43,6 @@ private: _snitch_is_ready.set_value(); } -public: virtual sstring get_rack(inet_address endpoint) override { return std::to_string((endpoint.raw_addr() >> 8) & 0xFF); } diff --git a/locator/simple_snitch.cc b/locator/simple_snitch.cc new file mode 100644 index 0000000000..dabc7400ec --- /dev/null +++ b/locator/simple_snitch.cc @@ -0,0 +1,7 @@ +#include "locator/simple_snitch.hh" +#include "utils/class_registrator.hh" + +namespace locator { +using registry = class_registrator; +static registry registrator("org.apache.cassandra.locator.SimpleSnitch"); +} diff --git a/locator/simple_snitch.hh b/locator/simple_snitch.hh index 65770efebb..8979f15877 100644 --- a/locator/simple_snitch.hh +++ b/locator/simple_snitch.hh @@ -27,14 +27,11 @@ namespace locator { /** - * A simple endpoint snitch implementation that treats Strategy order as proximity, - * allowing non-read-repaired reads to prefer a single endpoint, which improves - * cache locality. + * A simple endpoint snitch implementation that treats Strategy order as + * proximity, allowing non-read-repaired reads to prefer a single endpoint, + * which improves cache locality. */ -class simple_snitch : public snitch_base { - template - friend future make_snitch(A&&... a); - +struct simple_snitch : public snitch_base { simple_snitch() { _my_dc = get_datacenter(utils::fb_utilities::get_broadcast_address()); _my_rack = get_rack(utils::fb_utilities::get_broadcast_address()); @@ -43,7 +40,6 @@ class simple_snitch : public snitch_base { _snitch_is_ready.set_value(); } -public: virtual sstring get_rack(inet_address endpoint) override { return "rack1"; } diff --git a/locator/snitch_base.cc b/locator/snitch_base.cc index 1b9b5b7c3e..14e4b552d0 100644 --- a/locator/snitch_base.cc +++ b/locator/snitch_base.cc @@ -23,6 +23,9 @@ namespace locator { +thread_local logging::logger +i_endpoint_snitch::snitch_logger("snitch_logger"); + std::vector snitch_base::get_sorted_list_by_proximity( inet_address address, std::unordered_set& unsorted_address) { diff --git a/locator/snitch_base.hh b/locator/snitch_base.hh index a649edeef0..c97ba1a2f9 100644 --- a/locator/snitch_base.hh +++ b/locator/snitch_base.hh @@ -26,6 +26,7 @@ #include "gms/inet_address.hh" #include "core/shared_ptr.hh" +#include "utils/class_registrator.hh" namespace locator { @@ -34,11 +35,11 @@ struct i_endpoint_snitch; typedef gms::inet_address inet_address; typedef std::unique_ptr snitch_ptr; -template -static future make_snitch(A&&... a); - struct i_endpoint_snitch { + template + static future create_snitch(const sstring& snitch_name, A&&... a); + /** * returns a String representing the rack this endpoint belongs to */ @@ -90,10 +91,8 @@ struct i_endpoint_snitch virtual future<> stop() = 0; - template - friend future make_snitch(A&&... a); - protected: + static thread_local logging::logger snitch_logger; promise<> _snitch_is_ready; enum class snitch_state { initializing, @@ -103,27 +102,38 @@ protected: } _state = snitch_state::initializing; }; -template -static future make_snitch(A&&... a) { - snitch_ptr s(new SnitchClass(std::forward(a)...)); - auto fu = s->_snitch_is_ready.get_future(); - return fu.then_wrapped([s = std::move(s)] (auto&& f) mutable { - try { - f.get(); +template +future i_endpoint_snitch::create_snitch( + const sstring& snitch_name, A&&... a) { - return make_ready_future(std::move(s)); - } catch (...) { - auto eptr = std::current_exception(); - auto fu = s->stop(); + try { + snitch_ptr s(std::move(create_object( + snitch_name, std::forward(a)...))); + + auto fu = s->_snitch_is_ready.get_future(); + return fu.then_wrapped([s = std::move(s)] (auto&& f) mutable { + try { + f.get(); - return fu.then([eptr, s = std::move(s)] () mutable { - std::rethrow_exception(eptr); - // just to make a compiler happy return make_ready_future(std::move(s)); - }); - } - }); + } catch (...) { + auto eptr = std::current_exception(); + auto fu = s->stop(); + + return fu.then([eptr, s = std::move(s)] () mutable { + std::rethrow_exception(eptr); + // just to make a compiler happy + return make_ready_future(std::move(s)); + }); + } + }); + } catch (no_such_class& e) { + snitch_logger.error("{}", e.what()); + throw; + } catch (...) { + throw; + } } class snitch_base : public i_endpoint_snitch { diff --git a/tests/urchin/gossiping_property_file_snitch_test.cc b/tests/urchin/gossiping_property_file_snitch_test.cc index ef8593592b..21978b6904 100644 --- a/tests/urchin/gossiping_property_file_snitch_test.cc +++ b/tests/urchin/gossiping_property_file_snitch_test.cc @@ -25,7 +25,9 @@ future<> one_test(const std::string& property_fname, bool exp_result) { path fname(test_files_subdir); fname /= path(property_fname); - return make_snitch(sstring(fname.string())) + return i_endpoint_snitch::create_snitch( + "org.apache.cassandra.locator.GossipingPropertyFileSnitch", + sstring(fname.string())) .then([exp_result] (snitch_ptr sptr) { BOOST_CHECK(exp_result); return sptr->stop(); From e045d8465c82922deaa25b69f41e00b7acbe78b9 Mon Sep 17 00:00:00 2001 From: Vlad Zolotarov Date: Sun, 14 Jun 2015 13:16:16 +0300 Subject: [PATCH 2/3] db: use snitch name from the configuration file Signed-off-by: Vlad Zolotarov --- database.cc | 13 ++++++++----- database.hh | 4 +++- db/legacy_schema_tables.cc | 4 +++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/database.cc b/database.cc index 810da87b7c..ad676169db 100644 --- a/database.cc +++ b/database.cc @@ -591,7 +591,7 @@ create_keyspace(distributed& db, const lw_shared_ptrname(), std::move(ks)); @@ -704,7 +704,7 @@ const column_family& database::find_column_family(const utils::UUID& uuid) const } future<> -keyspace::create_replication_strategy() { +keyspace::create_replication_strategy(const sstring& snitch_name) { using namespace locator; static thread_local token_metadata tm; @@ -722,8 +722,7 @@ keyspace::create_replication_strategy() { tm.update_normal_token({dht::token::kind::key, {d2t(3.0/4).data(), 8}}, to_sstring("127.0.0.4")); // Fixme - return i_endpoint_snitch::create_snitch( - "org.apache.cassandra.locator.SimpleSnitch").then( + return i_endpoint_snitch::create_snitch(snitch_name).then( [this] (snitch_ptr&& s) { _replication_strategy = abstract_replication_strategy::create_replication_strategy( @@ -782,7 +781,7 @@ database::create_keyspace(const lw_shared_ptr& ksm) { } keyspace ks(ksm, std::move(make_keyspace_config(*ksm))); - auto fu = ks.create_replication_strategy(); + auto fu = ks.create_replication_strategy(get_snitch_name()); return fu.then([ks = std::move(ks), ksm, this] () mutable { _keyspaces.emplace(ksm->name(), std::move(ks)); @@ -1064,3 +1063,7 @@ database::stop() { return val_pair.second.stop(); }); } + +const sstring& database::get_snitch_name() const { + return _cfg->endpoint_snitch(); +} diff --git a/database.hh b/database.hh index 7c085bca06..3ba974e669 100644 --- a/database.hh +++ b/database.hh @@ -218,7 +218,7 @@ public: const lw_shared_ptr& metadata() const { return _metadata; } - future<> create_replication_strategy(); + future<> create_replication_strategy(const sstring& snitch_name); locator::abstract_replication_strategy& get_replication_strategy(); column_family::config make_column_family_config(const schema& s) const; future<> make_directory_for_column_family(const sstring& name, utils::UUID uuid); @@ -310,6 +310,8 @@ public: future> query(const query::read_command& cmd, const std::vector& ranges); future<> apply(const frozen_mutation&); keyspace::config make_keyspace_config(const keyspace_metadata& ksm) const; + const sstring& get_snitch_name() const; + friend std::ostream& operator<<(std::ostream& out, const database& db); friend future<> create_keyspace(distributed&, const lw_shared_ptr&); }; diff --git a/db/legacy_schema_tables.cc b/db/legacy_schema_tables.cc index 0f0a636c8e..513377afdc 100644 --- a/db/legacy_schema_tables.cc +++ b/db/legacy_schema_tables.cc @@ -34,6 +34,8 @@ #include "core/do_with.hh" #include "json.hh" +#include "db/config.hh" + #include #include @@ -591,7 +593,7 @@ std::vector ALL { KEYSPACES, COLUMNFAMILIES, COLUMNS, TRIGGERS, USE auto ksm = create_keyspace_from_schema_partition(val); std::unique_ptr k_ptr(new keyspace(ksm, db.make_keyspace_config(*ksm))); - auto fu = k_ptr->create_replication_strategy(); + auto fu = k_ptr->create_replication_strategy(db.get_snitch_name()); temp_vec_ptr->emplace_back(ksm, std::move(k_ptr)); return fu; From 2f14c53f4e1617a59007c712e0bcd9c519b1dca9 Mon Sep 17 00:00:00 2001 From: Vlad Zolotarov Date: Sun, 14 Jun 2015 15:30:49 +0300 Subject: [PATCH 3/3] gossiping_property_file_snitch: use a logger from i_endpoint_snitch Signed-off-by: Vlad Zolotarov --- locator/gossiping_property_file_snitch.hh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/locator/gossiping_property_file_snitch.hh b/locator/gossiping_property_file_snitch.hh index bedfb30ba2..2263ed0f0f 100644 --- a/locator/gossiping_property_file_snitch.hh +++ b/locator/gossiping_property_file_snitch.hh @@ -59,9 +59,7 @@ public: private: static logging::logger& logger() { - static thread_local logging::logger l("gossiping_property_file_snitch"); - - return l; + return i_endpoint_snitch::snitch_logger; } template