Miscellaneous cleanup

Removes needless status assignments and unused code. Moves and modifies some comments.
This commit is contained in:
Esa Korhonen 2018-08-20 18:41:34 +03:00
parent ab9a9f92cb
commit 3777da96bd
5 changed files with 48 additions and 100 deletions

View File

@ -896,26 +896,6 @@ bool MariaDBMonitor::wait_cluster_stabilization(MariaDBServer* new_master, const
return rval;
}
/**
* Check that the given slave is a valid promotion candidate.
*
* @param preferred Preferred new master
* @param err_out Json object for error printing. Can be NULL.
* @return True, if given slave is a valid promotion candidate.
*/
bool MariaDBMonitor::switchover_check_preferred_master(MariaDBServer* preferred, json_t** err_out)
{
mxb_assert(preferred);
bool rval = true;
if (!preferred->update_slave_info() || !preferred->check_replication_settings())
{
PRINT_MXS_JSON_ERROR(err_out, "The requested server '%s' is not a valid promotion candidate.",
preferred->name());
rval = false;
}
return rval;
}
/**
* Prepares a server for the replication master role.
*

View File

@ -334,33 +334,34 @@ json_t* MariaDBMonitor::diagnostics_to_json() const
*
* @param server The server to update
*/
void MariaDBMonitor::update_server(MariaDBServer& server)
void MariaDBMonitor::update_server(MariaDBServer* server)
{
MXS_MONITORED_SERVER* mon_srv = server.m_server_base;
MXS_MONITORED_SERVER* mon_srv = server->m_server_base;
mxs_connect_result_t conn_status = mon_ping_or_connect_to_db(m_monitor, mon_srv);
MYSQL* conn = mon_srv->con; // mon_ping_or_connect_to_db() may have reallocated the MYSQL struct.
if (mon_connection_is_ok(conn_status))
{
server.set_status(SERVER_RUNNING);
server->set_status(SERVER_RUNNING);
if (conn_status == MONITOR_CONN_NEWCONN_OK)
{
// Is a new connection or a reconnection. Check server version.
server.update_server_version();
server->update_server_version();
}
if (server.m_version == MariaDBServer::version::MARIADB_MYSQL_55 ||
server.m_version == MariaDBServer::version::MARIADB_100 ||
server.m_version == MariaDBServer::version::BINLOG_ROUTER)
auto server_vrs = server->m_version;
if (server_vrs == MariaDBServer::version::MARIADB_MYSQL_55 ||
server_vrs == MariaDBServer::version::MARIADB_100 ||
server_vrs == MariaDBServer::version::BINLOG_ROUTER)
{
// Check permissions if permissions failed last time or if this is a new connection.
if (server.had_status(SERVER_AUTH_ERROR) || conn_status == MONITOR_CONN_NEWCONN_OK)
if (server->had_status(SERVER_AUTH_ERROR) || conn_status == MONITOR_CONN_NEWCONN_OK)
{
server.check_permissions();
server->check_permissions();
}
// If permissions are ok, continue.
if (!server.has_status(SERVER_AUTH_ERROR))
if (!server->has_status(SERVER_AUTH_ERROR))
{
if (should_update_disk_space_status(mon_srv))
{
@ -368,7 +369,7 @@ void MariaDBMonitor::update_server(MariaDBServer& server)
}
// Query MariaDBServer specific data
server.monitor_server();
server->monitor_server();
}
}
}
@ -376,24 +377,24 @@ void MariaDBMonitor::update_server(MariaDBServer& server)
{
/* The current server is not running. Clear all but the stale master bit as it is used to detect
* masters that went down but came up. */
server.clear_status(~SERVER_WAS_MASTER);
server->clear_status(~SERVER_WAS_MASTER);
auto conn_errno = mysql_errno(conn);
if (conn_errno == ER_ACCESS_DENIED_ERROR || conn_errno == ER_ACCESS_DENIED_NO_PASSWORD_ERROR)
{
server.set_status(SERVER_AUTH_ERROR);
server->set_status(SERVER_AUTH_ERROR);
}
/* Log connect failure only once, that is, if server was RUNNING or MAINTENANCE during last
* iteration. */
if (mon_srv->mon_prev_status & (SERVER_RUNNING | SERVER_MAINT))
if (server->had_status(SERVER_RUNNING) || server->had_status(SERVER_MAINT))
{
mon_log_connect_error(mon_srv, conn_status);
}
}
/** Increase or reset the error count of the server. */
bool is_running = server.is_running();
bool in_maintenance = server.is_in_maintenance();
bool is_running = server->is_running();
bool in_maintenance = server->is_in_maintenance();
mon_srv->mon_err_count = (is_running || in_maintenance) ? 0 : mon_srv->mon_err_count + 1;
}
@ -434,10 +435,9 @@ void MariaDBMonitor::tick()
}
// Query all servers for their status.
for (auto iter = m_servers.begin(); iter != m_servers.end(); iter++)
for (MariaDBServer* server : m_servers)
{
MariaDBServer* server = *iter;
update_server(*server);
update_server(server);
if (server->m_topology_changed)
{
m_cluster_topology_changed = true;
@ -466,8 +466,7 @@ void MariaDBMonitor::tick()
// Sanity check. Master may not be both slave and master.
mxb_assert(m_master == NULL || !m_master->has_status(SERVER_SLAVE | SERVER_MASTER));
// Update shared status. The next functions read the shared status. TODO: change the following
// functions to read "pending_status" instead.
// Update shared status.
for (auto server : m_servers)
{
SERVER* srv = server->m_server_base->server;

View File

@ -192,7 +192,7 @@ private:
json_t* diagnostics_to_json() const;
// Cluster discovery and status assignment methods
void update_server(MariaDBServer& server);
void update_server(MariaDBServer* server);
void find_graph_cycles();
void update_topology();
void log_master_changes();
@ -218,7 +218,6 @@ private:
MariaDBServer** new_master_out, MariaDBServer** current_master_out,
json_t** error_out);
bool do_switchover(MariaDBServer* demotion_target, MariaDBServer* promotion_target, json_t** error_out);
bool switchover_check_preferred_master(MariaDBServer* preferred, json_t** err_out);
bool switchover_demote_master(MariaDBServer* current_master,
json_t** err_out);
bool switchover_wait_slaves_catchup(const ServerArray& slaves, const GtidList& gtid, int total_timeout,

View File

@ -279,22 +279,18 @@ bool MariaDBServer::update_gtids(string* errmsg_out)
return rval;
}
bool MariaDBServer::update_replication_settings(std::string* error_out)
bool MariaDBServer::update_replication_settings(std::string* errmsg_out)
{
static const string query = "SELECT @@gtid_strict_mode, @@log_bin, @@log_slave_updates;";
string query_error;
const string query = "SELECT @@gtid_strict_mode, @@log_bin, @@log_slave_updates;";
bool rval = false;
auto result = execute_query(query, &query_error);
auto result = execute_query(query, errmsg_out);
if (result.get() != NULL && result->next_row())
{
rval = true;
m_rpl_settings.gtid_strict_mode = result->get_bool(0);
m_rpl_settings.log_bin = result->get_bool(1);
m_rpl_settings.log_slave_updates = result->get_bool(2);
rval = true;
}
else if (error_out)
{
*error_out = query_error;
}
return rval;
}
@ -582,13 +578,6 @@ bool MariaDBServer::uses_gtid(std::string* error_out)
return using_gtid;
}
bool MariaDBServer::update_slave_info()
{
// TODO: fix for multisource repl
return (!m_slave_status.empty() && m_slave_status[0].slave_sql_running && update_replication_settings() &&
update_gtids() && do_show_slave_status());
}
bool MariaDBServer::can_replicate_from(MariaDBServer* master, string* error_out)
{
bool rval = false;
@ -782,9 +771,6 @@ bool MariaDBServer::run_sql_from_file(const string& path, json_t** error_out)
return !error;
}
/**
* Query this server.
*/
void MariaDBServer::monitor_server()
{
string errmsg;
@ -818,7 +804,7 @@ void MariaDBServer::monitor_server()
* ways. */
else if (!errmsg.empty() && m_print_update_errormsg)
{
MXS_WARNING("Error during monitor update of server '%s'. %s.", name(), errmsg.c_str());
MXS_WARNING("Error during monitor update of server '%s': %s", name(), errmsg.c_str());
m_print_update_errormsg = false;
}
return;
@ -832,19 +818,9 @@ void MariaDBServer::monitor_server()
*/
bool MariaDBServer::update_slave_status(string* errmsg_out)
{
/** Clear old states */
clear_status(SERVER_SLAVE | SERVER_MASTER | SERVER_RELAY | SERVER_SLAVE_OF_EXT_MASTER);
bool rval = false;
if (do_show_slave_status(errmsg_out))
bool rval = do_show_slave_status(errmsg_out);
if (rval)
{
rval = true;
/* If all configured slaves are running set this node as slave */
if (m_n_slaves_running > 0 && m_n_slaves_running == m_slave_status.size())
{
set_status(SERVER_SLAVE);
}
/** Store master_id of current node. */
m_server_base->server->master_id = !m_slave_status.empty() ?
m_slave_status[0].master_server_id : SERVER_ID_UNKNOWN;
@ -852,10 +828,6 @@ bool MariaDBServer::update_slave_status(string* errmsg_out)
return rval;
}
/**
* Update information which changes rarely. This method should be called after (re)connecting to a backend.
* Calling this every monitoring loop is overkill.
*/
void MariaDBServer::update_server_version()
{
m_version = version::UNKNOWN;
@ -896,15 +868,14 @@ void MariaDBServer::update_server_version()
}
}
/**
* Checks monitor permissions on the server. Sets/clears the SERVER_AUTH_ERROR bit.
*/
void MariaDBServer::check_permissions()
{
MYSQL* conn = m_server_base->con;
// Test with a typical query to make sure the monitor has sufficient permissions.
const char* query = "SHOW SLAVE STATUS;";
if (mxs_mysql_query(conn, query) != 0)
const string query = "SHOW SLAVE STATUS;";
string err_msg;
auto result = execute_query(query, &err_msg);
if (result.get() == NULL)
{
/* In theory, this could be due to other errors as well, but that is quite unlikely since the
* connection was just checked. The end result is in any case that the server is not updated,
@ -913,18 +884,13 @@ void MariaDBServer::check_permissions()
// Only print error if last round was ok.
if (!had_status(SERVER_AUTH_ERROR))
{
MXS_WARNING("Query '%s' to server '%s' failed when checking monitor permissions: '%s'. ",
query, name(), mysql_error(conn));
MXS_WARNING("Error during monitor permissions test for server '%s': %s",
name(), err_msg.c_str());
}
}
else
{
clear_status(SERVER_AUTH_ERROR);
MYSQL_RES* result = mysql_store_result(conn);
if (result)
{
mysql_free_result(result);
}
}
}

View File

@ -162,8 +162,20 @@ public:
MariaDBServer(MXS_MONITORED_SERVER* monitored_server, int config_index);
/**
* Query this server.
*/
void monitor_server();
/**
* Update information which changes rarely. This method should be called after (re)connecting to a backend.
* Calling this every monitoring loop is overkill.
*/
void update_server_version();
/**
* Checks monitor permissions on the server. Sets/clears the SERVER_AUTH_ERROR bit.
*/
void check_permissions();
/**
@ -356,14 +368,6 @@ public:
*/
bool uses_gtid(std::string* error_out = NULL);
/**
* Update replication settings, gtid:s and slave status of the server.
*
* @param server Slave to update
* @return True on success. False on error, or if server is not a slave (slave SQL not running).
*/
bool update_slave_info();
/**
* Checks if this server can replicate from master. Only considers gtid:s and only detects obvious errors.
* The non-detected errors will mostly be detected once the slave tries to start replicating.