From 5fc2c1f49ccc937917d0d50fd12ea0ba01613e39 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Wed, 19 Dec 2018 13:06:26 +0200 Subject: [PATCH] MXS-2220 Store server ports as integers and modify them in methods --- include/maxscale/server.hh | 23 ++++++--- server/core/config_runtime.cc | 4 +- server/core/internal/service.hh | 2 +- server/core/mysql_utils.cc | 2 +- server/core/server.cc | 47 ++----------------- server/core/service.cc | 2 +- .../routing/binlogrouter/blr_master.cc | 2 +- .../modules/routing/binlogrouter/blr_slave.cc | 8 ++-- 8 files changed, 31 insertions(+), 59 deletions(-) diff --git a/include/maxscale/server.hh b/include/maxscale/server.hh index 934206974..b41794483 100644 --- a/include/maxscale/server.hh +++ b/include/maxscale/server.hh @@ -86,9 +86,9 @@ public: char* protocol = nullptr; /**< Backend protocol module name */ char* authenticator = nullptr; /**< Authenticator module name */ - char address[MAX_ADDRESS_LEN] = {'\0'}; /**< Server hostname/IP-address */ - unsigned short port = 0; /**< Server port */ - unsigned short extra_port = 0; /**< Alternative monitor port if normal port fails */ + char address[MAX_ADDRESS_LEN] = {'\0'}; /**< Server hostname/IP-address */ + int port = -1; /**< Server port */ + int extra_port = -1; /**< Alternative monitor port if normal port fails */ // Other settings char monuser[MAX_MONUSER_LEN] = {'\0'}; /**< Monitor username, overrides monitor setting */ @@ -190,6 +190,20 @@ public: */ virtual std::string version_string() const = 0; + /** + * Update the server port. TODO: Move this to internal class once blr is gone. + * + * @param new_port New port. The value is not checked but should generally be 1 -- 65535. + */ + void update_port(int new_port); + + /** + * Update the server extra port. TODO: Move this to internal class once blr is gone. + * + * @param new_port New port. The value is not checked but should generally be 1 -- 65535. + */ + void update_extra_port(int new_port); + protected: SERVER() { @@ -440,7 +454,6 @@ void server_add_response_average(SERVER* server, double ave, int num_samples); extern int server_free(SERVER* server); extern SERVER* server_find_by_unique_name(const char* name); extern int server_find_by_unique_names(char** server_names, int size, SERVER*** output); -extern SERVER* server_find(const char* servname, unsigned short port); extern void server_clear_set_status_nolock(SERVER* server, uint64_t bits_to_clear, uint64_t bits_to_set); extern void server_set_status_nolock(SERVER* server, uint64_t bit); extern void server_clear_status_nolock(SERVER* server, uint64_t bit); @@ -448,8 +461,6 @@ extern void server_transfer_status(SERVER* dest_server, const SERVER* source_ extern void server_add_mon_user(SERVER* server, const char* user, const char* passwd); extern void server_update_credentials(SERVER* server, const char* user, const char* passwd); extern void server_update_address(SERVER* server, const char* address); -extern void server_update_port(SERVER* server, unsigned short port); -extern void server_update_extra_port(SERVER* server, unsigned short port); extern uint64_t server_map_status(const char* str); extern void printServer(const SERVER*); diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index acbbff376..9a4194479 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -486,12 +486,12 @@ bool runtime_alter_server(Server* server, const char* key, const char* value) { if (long ival = get_positive_int(value)) { - server_update_port(server, ival); + server->update_port(ival); } } else if (strcmp(key, CN_EXTRA_PORT) == 0) { - server_update_extra_port(server, atoi(value)); + server->update_extra_port(atoi(value)); } else if (strcmp(key, CN_MONITORUSER) == 0) { diff --git a/server/core/internal/service.hh b/server/core/internal/service.hh index a9946e163..90c0c9a40 100644 --- a/server/core/internal/service.hh +++ b/server/core/internal/service.hh @@ -324,7 +324,7 @@ SListener service_find_listener(Service* service, * @param port The port to check * @return True if a MaxScale service uses the port */ -bool service_port_is_used(unsigned short port); +bool service_port_is_used(int port); /** * @brief Check if the service has a listener with a matching name diff --git a/server/core/mysql_utils.cc b/server/core/mysql_utils.cc index fe9ec3688..d71ad790b 100644 --- a/server/core/mysql_utils.cc +++ b/server/core/mysql_utils.cc @@ -180,7 +180,7 @@ MYSQL* mxs_mysql_real_connect(MYSQL* con, SERVER* server, const char* user, cons MYSQL* mysql = mysql_real_connect(con, server->address, user, passwd, NULL, server->port, NULL, 0); auto extra_port = mxb::atomic::load(&server->extra_port, mxb::atomic::RELAXED); - if (!mysql && extra_port) + if (!mysql && extra_port > 0) { mysql = mysql_real_connect(con, server->address, user, passwd, NULL, extra_port, NULL, 0); MXS_WARNING("Could not connect with normal port to server '%s', using extra_port", server->name); diff --git a/server/core/server.cc b/server/core/server.cc index e6fea3df3..558ccbf74 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -402,26 +402,6 @@ int server_find_by_unique_names(char** server_names, int size, SERVER*** output) return found; } -/** - * Find an existing server - * - * @param servname The Server name or address - * @param port The server port - * @return The server or NULL if not found - */ -SERVER* server_find(const char* servname, unsigned short port) -{ - Guard guard(this_unit.all_servers_lock); - for (Server* server : this_unit.all_servers) - { - if (server->is_active && strcmp(server->address, servname) == 0 && server->port == port) - { - return server; - } - } - return nullptr; -} - /** * Print details of an individual server * @@ -932,33 +912,14 @@ void server_update_address(SERVER* server, const char* address) } } -/* - * Update the port value of a specific server - * - * @param server The server to update - * @param port The new port value - * - */ -void server_update_port(SERVER* server, unsigned short port) +void SERVER::update_port(int new_port) { - Guard guard(this_unit.all_servers_lock); - - if (server && port > 0) - { - server->port = port; - } + mxb::atomic::store(&port, new_port, mxb::atomic::RELAXED); } -/* - * Update the extra_port value of a specific server - * - * @param server The server to update - * @param port The new extra_port value - * - */ -void server_update_extra_port(SERVER* server, unsigned short port) +void SERVER::update_extra_port(int new_port) { - mxb::atomic::store(&server->extra_port, port, mxb::atomic::RELAXED); + mxb::atomic::store(&extra_port, new_port, mxb::atomic::RELAXED); } static struct diff --git a/server/core/service.cc b/server/core/service.cc index 06f7998ce..eca768f22 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -1552,7 +1552,7 @@ void service_print_users(DCB* dcb, const SERVICE* service) } } -bool service_port_is_used(unsigned short port) +bool service_port_is_used(int port) { bool rval = false; LockGuard guard(this_unit.lock); diff --git a/server/modules/routing/binlogrouter/blr_master.cc b/server/modules/routing/binlogrouter/blr_master.cc index 1213d07ba..7aa22097a 100644 --- a/server/modules/routing/binlogrouter/blr_master.cc +++ b/server/modules/routing/binlogrouter/blr_master.cc @@ -3211,7 +3211,7 @@ void blr_master_set_config(ROUTER_INSTANCE* inst, const ChangeMasterConfig& conf if (config.port) { - server_update_port(backend_server, config.port); + backend_server->update_port(config.port); } if (!config.user.empty()) diff --git a/server/modules/routing/binlogrouter/blr_slave.cc b/server/modules/routing/binlogrouter/blr_slave.cc index b47aa096d..fa1296fb4 100644 --- a/server/modules/routing/binlogrouter/blr_slave.cc +++ b/server/modules/routing/binlogrouter/blr_slave.cc @@ -4684,7 +4684,7 @@ static int blr_set_master_port(ROUTER_INSTANCE* router, int port) { if (port > 0) { - server_update_port(router->service->dbref->server, port); + router->service->dbref->server->update_port(port); MXS_INFO("%s: New MASTER_PORT is [%i]", router->service->name, @@ -4884,7 +4884,7 @@ static void blr_master_restore_config(ROUTER_INSTANCE* router, const MasterServerConfig& prev_master) { server_update_address(router->service->dbref->server, prev_master.host.c_str()); - server_update_port(router->service->dbref->server, prev_master.port); + router->service->dbref->server->update_port(prev_master.port); router->ssl_enabled = prev_master.ssl_enabled; if (!prev_master.ssl_version.empty()) @@ -4904,7 +4904,7 @@ static void blr_master_restore_config(ROUTER_INSTANCE* router, static void blr_master_set_empty_config(ROUTER_INSTANCE* router) { server_update_address(router->service->dbref->server, "none"); - server_update_port(router->service->dbref->server, (unsigned short)3306); + router->service->dbref->server->update_port(3306); router->current_pos = 4; router->binlog_position = 4; @@ -4926,7 +4926,7 @@ static void blr_master_set_empty_config(ROUTER_INSTANCE* router) static void blr_master_apply_config(ROUTER_INSTANCE* router, const MasterServerConfig& prev_master) { server_update_address(router->service->dbref->server, prev_master.host.c_str()); - server_update_port(router->service->dbref->server, prev_master.port); + router->service->dbref->server->update_port(prev_master.port); router->current_pos = prev_master.pos; router->binlog_position = prev_master.safe_pos; router->current_safe_event = prev_master.safe_pos;