Make the servers-array in Monitor private

This prevents derived classes from modifying the array directly,
which would be unsafe.
This commit is contained in:
Esa Korhonen 2019-05-15 14:57:28 +03:00
parent 6317a86c69
commit cf46004bd8
6 changed files with 33 additions and 27 deletions

View File

@ -249,6 +249,8 @@ private:
class Monitor
{
public:
using ServerVector = std::vector<MonitorServer*>;
Monitor(const std::string& name, const std::string& module);
virtual ~Monitor();
@ -301,6 +303,8 @@ public:
const char* name() const;
const ServerVector& servers() const;
/**
* Configure the monitor. Called by monitor creation and altering code. Any inheriting classes
* should override this with their own configuration processing function. The overriding function
@ -396,8 +400,6 @@ public:
const std::string m_name; /**< Monitor instance name. */
const std::string m_module; /**< Name of the monitor module */
std::vector<MonitorServer*> m_servers; /**< Monitored servers */
protected:
/**
* Stop the monitor. If the monitor uses a polling thread, the thread should be stopped.
@ -581,8 +583,9 @@ private:
std::atomic_bool m_status_change_pending {false}; /**< Set when admin requests a status change. */
uint8_t m_journal_hash[SHA_DIGEST_LENGTH]; /**< SHA1 hash of the latest written journal */
MXS_CONFIG_PARAMETER m_parameters; /**< Configuration parameters in text form */
Settings m_settings; /**< Base class settings */
ServerVector m_servers; /**< Monitored servers */
MXS_CONFIG_PARAMETER m_parameters; /**< Configuration parameters in text form */
Settings m_settings; /**< Base class settings */
};
/**

View File

@ -246,7 +246,7 @@ void store_data(Monitor* monitor, MonitorServer* master, uint8_t* data, uint32_t
*ptr++ = MMB_SCHEMA_VERSION;
/** Store the states of all servers */
for (MonitorServer* db : monitor->m_servers)
for (MonitorServer* db : monitor->servers())
{
*ptr++ = (char)SVT_SERVER; // Value type
memcpy(ptr, db->server->name(), strlen(db->server->name()));// Name of the server
@ -299,7 +299,7 @@ static bool has_null_terminator(const char* data, const char* end)
*/
const char* process_server(Monitor* monitor, const char* data, const char* end)
{
for (MonitorServer* db : monitor->m_servers)
for (MonitorServer* db : monitor->servers())
{
if (strcmp(db->server->name(), data) == 0)
{
@ -328,7 +328,7 @@ const char* process_master(Monitor* monitor, MonitorServer** master,
{
if (master)
{
for (MonitorServer* db : monitor->m_servers)
for (MonitorServer* db : monitor->servers())
{
if (strcmp(db->server->name(), data) == 0)
{
@ -691,8 +691,7 @@ json_t* Monitor::parameters_to_json() const
bool Monitor::test_permissions(const string& query)
{
auto monitor = this;
if (monitor->m_servers.empty() || config_get_global_options()->skip_permission_checks)
if (m_servers.empty() || config_get_global_options()->skip_permission_checks)
{
return true;
}
@ -700,13 +699,13 @@ bool Monitor::test_permissions(const string& query)
char* dpasswd = decrypt_password(m_settings.conn_settings.password.c_str());
bool rval = false;
for (MonitorServer* mondb : monitor->m_servers)
for (MonitorServer* mondb : m_servers)
{
if (!connection_is_ok(mondb->ping_or_connect(m_settings.conn_settings)))
{
MXS_ERROR("[%s] Failed to connect to server '%s' ([%s]:%d) when"
" checking monitor user credentials and permissions: %s",
monitor->name(),
name(),
mondb->server->name(),
mondb->server->address,
mondb->server->port,
@ -751,7 +750,7 @@ bool Monitor::test_permissions(const string& query)
if (res == NULL)
{
MXS_ERROR("[%s] Result retrieval failed when checking monitor permissions: %s",
monitor->name(),
name(),
mysql_error(mondb->con));
}
else
@ -1908,6 +1907,11 @@ bool Monitor::server_status_request_waiting() const
return m_status_change_pending.load(std::memory_order_acquire);
}
const Monitor::ServerVector& Monitor::servers() const
{
return m_servers;
}
MonitorWorker::MonitorWorker(const string& name, const string& module)
: Monitor(name, module)
, m_thread_running(false)
@ -2129,7 +2133,7 @@ bool MonitorWorker::has_sufficient_permissions()
void MonitorWorker::flush_server_status()
{
for (MonitorServer* pMs : m_servers)
for (MonitorServer* pMs : servers())
{
if (!pMs->server->is_in_maint())
{
@ -2164,7 +2168,7 @@ void MonitorWorkerSimple::tick()
const bool should_update_disk_space = check_disk_space_this_tick();
for (MonitorServer* pMs : m_servers)
for (MonitorServer* pMs : servers())
{
if (!pMs->server->is_in_maint())
{

View File

@ -480,7 +480,7 @@ json_t* MonitorManager::monitor_relations_to_server(const SERVER* server, const
std::vector<std::string> names;
this_unit.foreach_monitor([&names, server](Monitor* mon) {
// The serverlist of an individual monitor should not change while a monitor is running.
for (MonitorServer* db : mon->m_servers)
for (MonitorServer* db : mon->servers())
{
if (db->server == server)
{

View File

@ -394,7 +394,7 @@ bool ClustrixMonitor::choose_dynamic_hub(Clustrix::Softfailed softfailed, std::s
bool ClustrixMonitor::choose_bootstrap_hub(Clustrix::Softfailed softfailed, std::set<string>& ips_checked)
{
for (auto* pMs : m_servers)
for (auto* pMs : servers())
{
if (ips_checked.find(pMs->server->address) == ips_checked.end())
{
@ -692,7 +692,7 @@ void ClustrixMonitor::check_bootstrap_servers()
set<string> current_bootstrap_servers;
for (const auto* pMs : m_servers)
for (const auto* pMs : servers())
{
SERVER* pServer = pMs->server;
@ -749,7 +749,7 @@ void ClustrixMonitor::persist_bootstrap_servers()
{
string values;
for (const auto* pMs : m_servers)
for (const auto* pMs : servers())
{
if (!values.empty())
{
@ -911,7 +911,7 @@ void ClustrixMonitor::populate_from_bootstrap_servers()
{
int id = 1;
for (auto ms : m_servers)
for (auto ms : servers())
{
SERVER* pServer = ms->server;
@ -940,9 +940,9 @@ void ClustrixMonitor::populate_from_bootstrap_servers()
void ClustrixMonitor::update_server_statuses()
{
mxb_assert(!m_servers.empty());
mxb_assert(!servers().empty());
for (auto* pMs : m_servers)
for (auto* pMs : servers())
{
pMs->stash_current_status();

View File

@ -104,7 +104,7 @@ json_t* GaleraMonitor::diagnostics_json() const
json_t* arr = json_array();
for (auto ptr : m_servers)
for (auto ptr : servers())
{
auto it = m_info.find(ptr);
@ -361,7 +361,7 @@ void GaleraMonitor::post_tick()
m_master = set_cluster_master(m_master, candidate_master, m_disableMasterFailback);
for (auto ptr : m_servers)
for (auto ptr : servers())
{
const int repl_bits = (SERVER_SLAVE | SERVER_MASTER | SERVER_MASTER_STICKINESS);
if ((ptr->pending_status & SERVER_JOINED) && !m_disableMasterRoleSetting)
@ -487,7 +487,7 @@ MonitorServer* GaleraMonitor::get_candidate_master()
int minval = INT_MAX;
int currval;
/* set min_id to the lowest value of moitor_servers->server->node_id */
for (auto moitor_servers : m_servers)
for (auto moitor_servers : servers())
{
if (!moitor_servers->server->is_in_maint()
&& (moitor_servers->pending_status & SERVER_JOINED))
@ -603,7 +603,6 @@ static MonitorServer* set_cluster_master(MonitorServer* current_master,
*/
void GaleraMonitor::update_sst_donor_nodes(int is_cluster)
{
MonitorServer* ptr;
MYSQL_ROW row;
MYSQL_RES* result;
bool ignore_priority = true;
@ -631,7 +630,7 @@ void GaleraMonitor::update_sst_donor_nodes(int is_cluster)
strcpy(donor_list, DONOR_LIST_SET_VAR);
/* Create an array of slave nodes */
for (auto ptr : m_servers)
for (auto ptr : servers())
{
if ((ptr->pending_status & SERVER_JOINED) && (ptr->pending_status & SERVER_SLAVE))
{

View File

@ -78,7 +78,7 @@ void MariaDBMonitor::reset_server_info()
clear_server_info();
// Next, initialize the data.
for (auto mon_server : Monitor::m_servers)
for (auto mon_server : servers())
{
m_servers.push_back(new MariaDBServer(mon_server, m_servers.size(), m_settings.shared));
}