Skip to content

Commit

Permalink
Merge "Simplify gossiper state map API" from Pavel E
Browse files Browse the repository at this point in the history
"
There's a enpoint->state map member of the gossiper class. First
ugly thing about it is that the member is public.

Next, there's a whole bunch of helpers around that map that export
various bits of information from it. All of those helpers reshard
to shard-0 to read from the state mape ignoring the fact that the
map is replicated on all shards internally. Also, some of those
helpers effectively duplicate each other for no real gain. Finally,
most of them are specific to api/ code, and open-coding them often
makes api/ handlers shorter and simpler.

This set removes the unused, api-only or trivial state map accessors
and marks the state map itself private (underscore prefix included).

tests: https://jenkins.scylladb.com/job/releng/job/Scylla-CI/233/
"

* 'br-gossiper-sanitize-api-2' of https://github.com/xemul/scylla:
  gossiper: Add underscores to new private members
  code: Indentation fix after previous patch
  gossiper, code: Relax get_up/down/all_counters() helpers
  api: Fix indentation after previous patch
  gossiper, api: Remove get_arrival_samples()
  gossiper, api: Remove get/set phi convict threshold helpers
  gossiper, api: Move get_simple_states() into API code
  gossiper: In-line std::optional<> get_endpoint_state_for_endpoint() overload
  gossiper, api: Remove get_endpoint_state() helpers
  gossiper: Make state and locks maps private
  gossiper: Remove dead code
  • Loading branch information
avikivity committed May 8, 2022
2 parents 94f677b + 9d364f1 commit 81af934
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 209 deletions.
2 changes: 1 addition & 1 deletion alternator/executor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4293,7 +4293,7 @@ static std::map<sstring, sstring> get_network_topology_options(gms::gossiper& go
// manually create the keyspace to override this predefined behavior.
static future<std::vector<mutation>> create_keyspace(std::string_view keyspace_name, service::migration_manager& mm, gms::gossiper& gossiper, api::timestamp_type ts) {
sstring keyspace_name_str(keyspace_name);
int endpoint_count = co_await gms::get_all_endpoint_count(gossiper);
int endpoint_count = gossiper.get_endpoint_states().size();
int rf = 3;
if (endpoint_count < rf) {
rf = 1;
Expand Down
59 changes: 29 additions & 30 deletions api/failure_detector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace fd = httpd::failure_detector_json;
void set_failure_detector(http_context& ctx, routes& r, gms::gossiper& g) {
fd::get_all_endpoint_states.set(r, [&g](std::unique_ptr<request> req) {
std::vector<fd::endpoint_state> res;
for (auto i : g.endpoint_state_map) {
for (auto i : g.get_endpoint_states()) {
fd::endpoint_state val;
val.addrs = boost::lexical_cast<std::string>(i.first);
val.is_alive = i.second.is_alive();
Expand All @@ -40,54 +40,53 @@ void set_failure_detector(http_context& ctx, routes& r, gms::gossiper& g) {
});

fd::get_up_endpoint_count.set(r, [&g](std::unique_ptr<request> req) {
return gms::get_up_endpoint_count(g).then([](int res) {
return make_ready_future<json::json_return_type>(res);
});
int res = g.get_up_endpoint_count();
return make_ready_future<json::json_return_type>(res);
});

fd::get_down_endpoint_count.set(r, [&g](std::unique_ptr<request> req) {
return gms::get_down_endpoint_count(g).then([](int res) {
return make_ready_future<json::json_return_type>(res);
});
int res = g.get_down_endpoint_count();
return make_ready_future<json::json_return_type>(res);
});

fd::get_phi_convict_threshold.set(r, [] (std::unique_ptr<request> req) {
return gms::get_phi_convict_threshold().then([](double res) {
return make_ready_future<json::json_return_type>(res);
});
return make_ready_future<json::json_return_type>(8);
});

fd::get_simple_states.set(r, [&g] (std::unique_ptr<request> req) {
return gms::get_simple_states(g).then([](const std::map<sstring, sstring>& map) {
return make_ready_future<json::json_return_type>(map_to_key_value<fd::mapper>(map));
});
std::map<sstring, sstring> nodes_status;
for (auto& entry : g.get_endpoint_states()) {
nodes_status.emplace(entry.first.to_sstring(), entry.second.is_alive() ? "UP" : "DOWN");
}
return make_ready_future<json::json_return_type>(map_to_key_value<fd::mapper>(nodes_status));
});

fd::set_phi_convict_threshold.set(r, [](std::unique_ptr<request> req) {
double phi = atof(req->get_query_param("phi").c_str());
return gms::set_phi_convict_threshold(phi).then([]() {
return make_ready_future<json::json_return_type>("");
});
return make_ready_future<json::json_return_type>("");
});

fd::get_endpoint_state.set(r, [&g] (std::unique_ptr<request> req) {
return get_endpoint_state(g, req->param["addr"]).then([](const sstring& state) {
return make_ready_future<json::json_return_type>(state);
});
auto* state = g.get_endpoint_state_for_endpoint_ptr(gms::inet_address(req->param["addr"]));
if (!state) {
return make_ready_future<json::json_return_type>(format("unknown endpoint {}", req->param["addr"]));
}
std::stringstream ss;
g.append_endpoint_state(ss, *state);
return make_ready_future<json::json_return_type>(sstring(ss.str()));
});

fd::get_endpoint_phi_values.set(r, [](std::unique_ptr<request> req) {
return gms::get_arrival_samples().then([](std::map<gms::inet_address, gms::arrival_window> map) {
std::vector<fd::endpoint_phi_value> res;
auto now = gms::arrival_window::clk::now();
for (auto& p : map) {
fd::endpoint_phi_value val;
val.endpoint = p.first.to_sstring();
val.phi = p.second.phi(now);
res.emplace_back(std::move(val));
}
return make_ready_future<json::json_return_type>(res);
});
std::map<gms::inet_address, gms::arrival_window> map;
std::vector<fd::endpoint_phi_value> res;
auto now = gms::arrival_window::clk::now();
for (auto& p : map) {
fd::endpoint_phi_value val;
val.endpoint = p.first.to_sstring();
val.phi = p.second.phi(now);
res.emplace_back(std::move(val));
}
return make_ready_future<json::json_return_type>(res);
});
}

Expand Down
2 changes: 1 addition & 1 deletion db/system_keyspace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1824,7 +1824,7 @@ class cluster_status_table : public memtable_filling_virtual_table {
return _ss.get_ownership().then([&, mutation_sink] (std::map<gms::inet_address, float> ownership) {
const locator::token_metadata& tm = _ss.get_token_metadata();

for (auto&& e : _gossiper.endpoint_state_map) {
for (auto&& e : _gossiper.get_endpoint_states()) {
auto endpoint = e.first;

mutation m(schema(), partition_key::from_single_value(*schema(), data_value(endpoint).serialize_nonnull()));
Expand Down
Loading

0 comments on commit 81af934

Please sign in to comment.