Merge "locator: use the correct snitch instance in gossiping_property_file_snitch::reload_configuration()" from Vlad

"Commit 4cd9c4c0c5441cf55e280c6f2f2e5529426b9c98 introduced a minor
issue: a wrong snitch instance may be used when updating a Gossiper state
(if I/O CPU is different from CPU0).

In order to fix this issue a local snitch instance on CPU0 should be used,
just like a Gossiper local instance.

We have to move some interfaces to i_endpoint_snitch
from being private in a gossiping_property_file_snitch in order to be
able to access it using snitch_ptr handle."
This commit is contained in:
Avi Kivity
2015-11-02 14:09:27 +02:00
6 changed files with 52 additions and 23 deletions

View File

@@ -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<reconnectable_snitch_helper>(_my_dc));
_helper_added = true;
_gossip_started = true;
}
});

View File

@@ -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

View File

@@ -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();
});
});

View File

@@ -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<timespec> _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_snitch_helper> _reconnectable_helper;

View File

@@ -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:

View File

@@ -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_ptr>& 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<inet_address>& merged,
std::vector<inet_address>& l1,
@@ -420,6 +430,7 @@ private:
protected:
sstring _my_dc;
sstring _my_rack;
bool _prefer_local = false;
};
} // namespace locator