diff --git a/locator/ec2_multi_region_snitch.cc b/locator/ec2_multi_region_snitch.cc index 8db909520b..53db54db78 100644 --- a/locator/ec2_multi_region_snitch.cc +++ b/locator/ec2_multi_region_snitch.cc @@ -109,9 +109,9 @@ future<> ec2_multi_region_snitch::gossiper_starting() { return g.add_local_application_state(application_state::INTERNAL_IP, ss.value_factory.internal_ip(_local_private_address)).then([this] { - if (!_helper_added) { + if (!_gossip_started) { gms::get_local_gossiper().register_(make_shared(_my_dc)); - _helper_added = true; + _gossip_started = true; } }); diff --git a/locator/ec2_multi_region_snitch.hh b/locator/ec2_multi_region_snitch.hh index 180d9a3c59..ab48c2f38a 100644 --- a/locator/ec2_multi_region_snitch.hh +++ b/locator/ec2_multi_region_snitch.hh @@ -51,6 +51,5 @@ private: static constexpr const char* PUBLIC_IP_QUERY_REQ = "/latest/meta-data/public-ipv4"; static constexpr const char* PRIVATE_IP_QUERY_REQ = "/latest/meta-data/local-ipv4"; sstring _local_private_address; - bool _helper_added = false; }; } // namespace locator diff --git a/locator/gossiping_property_file_snitch.cc b/locator/gossiping_property_file_snitch.cc index 12cd27f0c7..45a37e080a 100644 --- a/locator/gossiping_property_file_snitch.cc +++ b/locator/gossiping_property_file_snitch.cc @@ -214,12 +214,14 @@ future<> gossiping_property_file_snitch::reload_configuration() { if (engine().cpu_id() != _file_reader_cpu_id) { local_s->set_my_dc(_my_dc); local_s->set_my_rack(_my_rack); + local_s->set_prefer_local(_prefer_local); } }).then([this] { return seastar::async([this] { // reload Gossiper state (executed on CPU0 only) - smp::submit_to(0, [this] { - this->reload_gossiper_state(); + smp::submit_to(0, [] { + auto& local_snitch_ptr = get_local_snitch_ptr(); + local_snitch_ptr->reload_gossiper_state(); }).get(); // update Storage Service on each shard @@ -239,10 +241,13 @@ future<> gossiping_property_file_snitch::reload_configuration() { // spread the word... - smp::submit_to(0, [this] { - if (this->_gossip_started && service::get_storage_service().local_is_initialized()) { - service::get_local_storage_service().gossip_snitch_info(); + smp::submit_to(0, [] { + auto& local_snitch_ptr = get_local_snitch_ptr(); + if (local_snitch_ptr->local_gossiper_started() && service::get_storage_service().local_is_initialized()) { + return service::get_local_storage_service().gossip_snitch_info(); } + + return make_ready_future<>(); }).get(); }); }); diff --git a/locator/gossiping_property_file_snitch.hh b/locator/gossiping_property_file_snitch.hh index ba34246313..27bab69970 100644 --- a/locator/gossiping_property_file_snitch.hh +++ b/locator/gossiping_property_file_snitch.hh @@ -79,6 +79,27 @@ public: const sstring& fname = "", unsigned io_cpuid = 0); + /** + * This function register a Gossiper subscriber to reconnect according to + * the new "prefer_local" value, namely use either an internal or extenal IP + * address. + * + * @note Currently in order to be backward compatible we are mimicking the C* + * behavior, which is a bit strange: while allowing the change of + * prefer_local value during the same run it won't actually trigger + * disconnect from all remote nodes as would be logical (in order to + * connect using a new configuration). On the contrary, if the new + * prefer_local value is TRUE, it will trigger the reconnect only when + * there is a corresponding gossip event (e.g. on_change()) from the + * corresponding node has been accepted. If the new value is FALSE + * then it won't trigger disconnect at all! And in any case a remote + * node will be reconnected using the PREFERED_IP value stored in the + * system_table.peer. + * + * This is currently relevant to EC2/GCE(?) only. + */ + virtual void reload_gossiper_state() override; + private: void periodic_reader_callback(); @@ -103,15 +124,6 @@ private: */ future<> read_property_file(); - /** - * TODO: this function is expected to trigger a Gossiper to reconnect - * according to the new "prefer_local" value, namely use either an internal - * or extenal IP address. - * - * This is currently relevant to EC2/GCE(?) only. - */ - void reload_gossiper_state(); - /** * Indicate that the snitch has stopped its I/O. */ @@ -124,8 +136,6 @@ private: timer<> _file_reader; std::experimental::optional _last_file_mod; std::istringstream _istrm; - bool _gossip_started = false; - bool _prefer_local = false; bool _file_reader_runs = false; unsigned _file_reader_cpu_id; shared_ptr _reconnectable_helper; diff --git a/locator/production_snitch_base.hh b/locator/production_snitch_base.hh index a9d02700c1..ded70dfd38 100644 --- a/locator/production_snitch_base.hh +++ b/locator/production_snitch_base.hh @@ -163,6 +163,10 @@ private: _my_rack = new_rack; } + virtual void set_prefer_local(bool prefer_local) override { + _prefer_local = prefer_local; + } + void parse_property_file(); protected: diff --git a/locator/snitch_base.hh b/locator/snitch_base.hh index 1a59bebf70..35145ea453 100644 --- a/locator/snitch_base.hh +++ b/locator/snitch_base.hh @@ -102,7 +102,10 @@ public: * called after Gossiper instance exists immediately before it starts * gossiping */ - virtual future<> gossiper_starting() = 0; + virtual future<> gossiper_starting() { + _gossip_started = true; + return make_ready_future<>(); + } /** * Returns whether for a range query doing a query against merged is likely @@ -142,6 +145,7 @@ public: // noop by default virtual void set_my_dc(const sstring& new_dc) {}; virtual void set_my_rack(const sstring& new_rack) {}; + virtual void set_prefer_local(bool prefer_local) {}; virtual void set_local_private_addr(const sstring& addr_str) {}; static distributed& snitch_instance() { @@ -165,6 +169,14 @@ public: //noop by default } + bool local_gossiper_started() { + return _gossip_started; + } + + virtual void reload_gossiper_state() { + // noop by default + } + protected: static logging::logger& logger() { static logging::logger snitch_logger("snitch_logger"); @@ -185,6 +197,7 @@ protected: stopping, stopped } _state = snitch_state::initializing; + bool _gossip_started = false; }; struct snitch_ptr { @@ -406,9 +419,6 @@ public: virtual int compare_endpoints( inet_address& address, inet_address& a1, inet_address& a2) override; - // noop by default - virtual future<> gossiper_starting() override { return make_ready_future<>(); } - virtual bool is_worth_merging_for_range_query( std::vector& merged, std::vector& l1, @@ -420,6 +430,7 @@ private: protected: sstring _my_dc; sstring _my_rack; + bool _prefer_local = false; }; } // namespace locator