From 9cac92754223286f2cf74e6d7b6fe14e73a06012 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Tue, 8 Jan 2019 10:17:07 +0200 Subject: [PATCH] MXS-2220 Move server response calculation functions inside class --- include/maxscale/server.hh | 40 +++++++++------ server/core/internal/server.hh | 15 ------ server/core/server.cc | 49 ++++++------------- .../readwritesplit/rwsplit_select_backends.cc | 4 +- .../routing/readwritesplit/rwsplitsession.cc | 10 ++-- 5 files changed, 45 insertions(+), 73 deletions(-) diff --git a/include/maxscale/server.hh b/include/maxscale/server.hh index 65e7b6d01..7c7e4ffd5 100644 --- a/include/maxscale/server.hh +++ b/include/maxscale/server.hh @@ -14,9 +14,11 @@ #include +#include #include -#include #include +#include +#include // A mapping from a path to a percentage, e.g.: "/disk" -> 80. typedef std::unordered_map MxsDiskSpaceThreshold; @@ -474,27 +476,35 @@ public: */ void clear_status(uint64_t bit); + int response_time_num_samples() const + { + return m_response_time.num_samples(); + } + + double response_time_average() const + { + return m_response_time.average(); + } + + /** + * Add a response time measurement to the global server value. + * + * @param ave The value to add + * @param num_samples The weight of the new value, that is, the number of measurement points it represents + */ + void response_time_add(double ave, int num_samples); + protected: SERVER() + : m_response_time(maxbase::EMAverage {0.04, 0.35, 500}) { } private: - static const int DEFAULT_CHARSET = 0x08; /** The latin1 charset */ + static const int DEFAULT_CHARSET = 0x08; /**< The latin1 charset */ + maxbase::EMAverage m_response_time; /**< Response time calculations for this server */ + std::mutex m_average_write_mutex; /**< Protects response time from concurrent writing */ }; -/** - * @brief Add a response average to the server response average. - * - * @param server The server. - * @param ave Average. - * @param num_samples Number of samples the average consists of. - * - */ -void server_add_response_average(SERVER* server, double ave, int num_samples); - -int server_response_time_num_samples(const SERVER* server); -double server_response_time_average(const SERVER* server); - namespace maxscale { diff --git a/server/core/internal/server.hh b/server/core/internal/server.hh index 381cfd2a4..e7a75b80e 100644 --- a/server/core/internal/server.hh +++ b/server/core/internal/server.hh @@ -20,7 +20,6 @@ #include #include -#include #include #include #include @@ -34,7 +33,6 @@ public: Server(const std::string& name, const std::string& protocol = "", const std::string& authenticator = "") : SERVER() , m_name(name) - , m_response_time(maxbase::EMAverage {0.04, 0.35, 500}) { m_settings.protocol = protocol; m_settings.authenticator = authenticator; @@ -46,18 +44,6 @@ public: std::string value; }; - int response_time_num_samples() const - { - return m_response_time.num_samples(); - } - - double response_time_average() const - { - return m_response_time.average(); - } - - void response_time_add(double ave, int num_samples); - long persistpoolmax() const { return m_settings.persistpoolmax; @@ -371,7 +357,6 @@ private: const std::string m_name; /**< Server config name */ Settings m_settings; /**< Server settings */ VersionInfo info; /**< Server version and type information */ - maxbase::EMAverage m_response_time; /**< Response time calculations for this server */ }; /** diff --git a/server/core/server.cc b/server/core/server.cc index 812a274e4..0ecc35543 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -74,7 +74,6 @@ namespace struct ThisUnit { - std::mutex ave_write_mutex; /**< TODO: Move to Server */ std::mutex all_servers_lock; /**< Protects access to all_servers */ std::list all_servers; /**< Global list of all servers */ } this_unit; @@ -517,9 +516,9 @@ void Server::print_to_dcb(DCB* dcb) const dcb_printf(dcb, "\tCurrent no. of operations: %d\n", server->stats.n_current_ops); dcb_printf(dcb, "\tNumber of routed packets: %lu\n", server->stats.packets); std::ostringstream ave_os; - if (server_response_time_num_samples(server)) + if (response_time_num_samples()) { - maxbase::Duration dur(server_response_time_average(server)); + maxbase::Duration dur(response_time_average()); ave_os << dur; } else @@ -1101,7 +1100,7 @@ json_t* Server::server_json_attributes(const Server* server) json_object_set_new(stats, "active_operations", json_integer(server->stats.n_current_ops)); json_object_set_new(stats, "routed_packets", json_integer(server->stats.packets)); - maxbase::Duration response_ave(server_response_time_average(server)); + maxbase::Duration response_ave(server->response_time_average()); json_object_set_new(stats, "adaptive_avg_select_time", json_string(to_string(response_ave).c_str())); json_object_set_new(attr, "statistics", stats); @@ -1174,38 +1173,20 @@ bool Server::set_disk_space_threshold(const char* disk_space_threshold) return rv; } -void server_add_response_average(SERVER* srv, double ave, int num_samples) -{ - Server* server = static_cast(srv); - Guard guard(this_unit.ave_write_mutex); - server->response_time_add(ave, num_samples); -} - -int server_response_time_num_samples(const SERVER* srv) -{ - const Server* server = static_cast(srv); - return server->response_time_num_samples(); -} - -double server_response_time_average(const SERVER* srv) -{ - const Server* server = static_cast(srv); - return server->response_time_average(); -} - -/** Apply backend average and adjust sample_max, which determines the weight of a new average - * applied to EMAverage. - * Sample max is raised if the server is fast, aggresively lowered if the incoming average is clearly - * lower than the EMA, else just lowered a bit. The normal increase and decrease, drifting, of the max - * is done to follow the speed of a server. The important part is the lowering of max, to allow for a - * server that is speeding up to be adjusted and used. - * - * Three new magic numbers to replace the sample max magic number... - * - */ -void Server::response_time_add(double ave, int num_samples) +void SERVER::response_time_add(double ave, int num_samples) { + /** + * Apply backend average and adjust sample_max, which determines the weight of a new average + * applied to EMAverage. + * + * Sample max is raised if the server is fast, aggressively lowered if the incoming average is clearly + * lower than the EMA, else just lowered a bit. The normal increase and decrease, drifting, of the max + * is done to follow the speed of a server. The important part is the lowering of max, to allow for a + * server that is speeding up to be adjusted and used. + * + * Three new magic numbers to replace the sample max magic number... */ constexpr double drift {1.1}; + Guard guard(m_average_write_mutex); int current_max = m_response_time.sample_max(); int new_max {0}; diff --git a/server/modules/routing/readwritesplit/rwsplit_select_backends.cc b/server/modules/routing/readwritesplit/rwsplit_select_backends.cc index f9865f82b..cd36a1d1f 100644 --- a/server/modules/routing/readwritesplit/rwsplit_select_backends.cc +++ b/server/modules/routing/readwritesplit/rwsplit_select_backends.cc @@ -129,7 +129,7 @@ PRWBackends::iterator backend_cmp_response_time(PRWBackends& sBackends) for (int i = 0; i < SZ; ++i) { SERVER_REF* server = (*sBackends[i]).backend(); - double ave = server_response_time_average(server->server); + double ave = server->server->response_time_average(); if (ave == 0) { @@ -302,7 +302,7 @@ static void log_server_connections(select_criteria_t criteria, const PRWBackends case ADAPTIVE_ROUTING: { - maxbase::Duration response_ave(server_response_time_average(b->server)); + maxbase::Duration response_ave(b->server->response_time_average()); std::ostringstream os; os << response_ave; MXS_INFO("adaptive avg. select time: %s from \t[%s]:%d %s", diff --git a/server/modules/routing/readwritesplit/rwsplitsession.cc b/server/modules/routing/readwritesplit/rwsplitsession.cc index 00fa66831..f410ffc13 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.cc +++ b/server/modules/routing/readwritesplit/rwsplitsession.cc @@ -120,9 +120,7 @@ void RWSplitSession::close() if (stat.make_valid()) { - server_add_response_average(backend->server(), - stat.average().secs(), - stat.num_samples()); + backend->server()->response_time_add(stat.average().secs(), stat.num_samples()); } backend->response_stat().reset(); @@ -637,11 +635,9 @@ void RWSplitSession::clientReply(GWBUF* writebuf, DCB* backend_dcb) ResponseStat& stat = backend->response_stat(); stat.query_ended(); if (stat.is_valid() && (stat.sync_time_reached() - || server_response_time_num_samples(backend->server()) == 0)) + || backend->server()->response_time_num_samples() == 0)) { - server_add_response_average(backend->server(), - stat.average().secs(), - stat.num_samples()); + backend->server()->response_time_add(stat.average().secs(), stat.num_samples()); stat.reset(); }