diff --git a/maxscale-system-test/mariadb_nodes.cpp b/maxscale-system-test/mariadb_nodes.cpp index 5ace71070..e49c5e8d0 100644 --- a/maxscale-system-test/mariadb_nodes.cpp +++ b/maxscale-system-test/mariadb_nodes.cpp @@ -391,12 +391,10 @@ int Mariadb_nodes::cleanup_db_nodes() void Mariadb_nodes::create_users(int node) { - char str[1024]; - char dtr[1024]; + char str[strlen(test_dir) + 17]; // Create users for replication as well as the users that are used by the tests sprintf(str, "%s/create_user.sh", test_dir); - sprintf(dtr, "%s", access_homedir[node]); - copy_to_node(node, str, dtr); + copy_to_node(node, str, access_homedir[node]); ssh_node_f(node, false, "export node_user=\"%s\"; export node_password=\"%s\"; %s/create_user.sh %s", user_name, password, access_homedir[0], socket_cmd[0]); @@ -502,7 +500,7 @@ int Galera_nodes::start_galera() } } - char str[1024]; + char str[strlen(test_dir) + 25]; sprintf(str, "%s/create_user_galera.sh", test_dir); copy_to_node_legacy(str, "~/", 0); @@ -1158,7 +1156,7 @@ int Mariadb_nodes::truncate_mariadb_logs() int Mariadb_nodes::configure_ssl(bool require) { int local_result = 0; - char str[1024]; + char str[strlen(test_dir) + 20]; this->ssl = 1; diff --git a/server/modules/monitor/mariadbmon/cluster_manipulation.cc b/server/modules/monitor/mariadbmon/cluster_manipulation.cc index 93a033838..b0567ab88 100644 --- a/server/modules/monitor/mariadbmon/cluster_manipulation.cc +++ b/server/modules/monitor/mariadbmon/cluster_manipulation.cc @@ -234,6 +234,9 @@ bool MariaDBMonitor::manual_reset_replication(SERVER* master_server, json_t** er } } + // Also record the previous master, needed for scheduled events. + MariaDBServer* old_master = (m_master && m_master->is_master()) ? m_master : nullptr; + bool rval = false; if (new_master) { @@ -333,11 +336,19 @@ bool MariaDBMonitor::manual_reset_replication(SERVER* master_server, json_t** er if (m_handle_event_scheduler) { - if (!new_master->enable_events(error_out)) + if (old_master) { - error = true; - PRINT_MXS_JSON_ERROR(error_out, "Could not enable events on '%s': %s", - new_master->name(), error_msg.c_str()); + if (!new_master->enable_events(old_master->m_enabled_events, error_out)) + { + error = true; + PRINT_MXS_JSON_ERROR(error_out, "Could not enable events on '%s': %s", + new_master->name(), error_msg.c_str()); + } + } + else + { + MXS_WARNING("No scheduled events were enabled on '%s' because previous master is " + "unknown. Check events manually.", new_master->name()); } } @@ -625,8 +636,8 @@ uint32_t MariaDBMonitor::do_rejoin(const ServerArray& joinable_servers, json_t** { // Assume that server is an old master which was failed over. Even if this is not really // the case, the following is unlikely to do damage. - ServerOperation demotion(joinable, true, /* treat as old master */ - m_handle_event_scheduler, m_demote_sql_file, {} /* unused */); + ServerOperation demotion(joinable, true, /* treat as old master */ + m_handle_event_scheduler, m_demote_sql_file); if (joinable->demote(general, demotion)) { MXS_NOTICE("Directing standalone server '%s' to replicate from '%s'.", name, master_name); @@ -1397,8 +1408,8 @@ unique_ptr MariaDBMonitor::failover_prepare(Log auto time_limit = maxbase::Duration((double)m_failover_timeout); bool promoting_to_master = (demotion_target == m_master); ServerOperation promotion(promotion_target, promoting_to_master, - m_handle_event_scheduler, m_promote_sql_file, - demotion_target->m_slave_status); + m_handle_event_scheduler, m_promote_sql_file, + demotion_target->m_slave_status, demotion_target->m_enabled_events); GeneralOpData general(m_replication_user, m_replication_password, error_out, time_limit); rval.reset(new FailoverParams(promotion, demotion_target, general)); } @@ -1687,9 +1698,10 @@ MariaDBMonitor::switchover_prepare(SERVER* promotion_server, SERVER* demotion_se maxbase::Duration time_limit((double)m_switchover_timeout); bool master_swap = (demotion_target == m_master); ServerOperation promotion(promotion_target, master_swap, m_handle_event_scheduler, - m_promote_sql_file, demotion_target->m_slave_status); + m_promote_sql_file, + demotion_target->m_slave_status, demotion_target->m_enabled_events); ServerOperation demotion(demotion_target, master_swap, m_handle_event_scheduler, - m_demote_sql_file, promotion_target->m_slave_status); + m_demote_sql_file, promotion_target->m_slave_status, {} /* unused */); GeneralOpData general(m_replication_user, m_replication_password, error_out, time_limit); rval.reset(new SwitchoverParams(promotion, demotion, general)); } diff --git a/server/modules/monitor/mariadbmon/mariadbmon.cc b/server/modules/monitor/mariadbmon/mariadbmon.cc index 31b16c2cd..469286ada 100644 --- a/server/modules/monitor/mariadbmon/mariadbmon.cc +++ b/server/modules/monitor/mariadbmon/mariadbmon.cc @@ -79,7 +79,8 @@ void MariaDBMonitor::reset_server_info() // Next, initialize the data. for (auto mon_server = m_monitor->monitored_servers; mon_server; mon_server = mon_server->next) { - m_servers.push_back(new MariaDBServer(mon_server, m_servers.size(), m_assume_unique_hostnames)); + m_servers.push_back(new MariaDBServer(mon_server, m_servers.size(), + m_assume_unique_hostnames, m_handle_event_scheduler)); } } diff --git a/server/modules/monitor/mariadbmon/mariadbserver.cc b/server/modules/monitor/mariadbmon/mariadbserver.cc index ca38b834b..e946250a8 100644 --- a/server/modules/monitor/mariadbmon/mariadbserver.cc +++ b/server/modules/monitor/mariadbmon/mariadbserver.cc @@ -26,20 +26,12 @@ using maxscale::string_printf; using maxbase::Duration; using maxbase::StopWatch; -class MariaDBServer::EventInfo -{ -public: - std::string database; - std::string name; - std::string definer; - std::string status; -}; - MariaDBServer::MariaDBServer(MXS_MONITORED_SERVER* monitored_server, int config_index, - bool assume_unique_hostnames) + bool assume_unique_hostnames, bool query_events) : m_server_base(monitored_server) , m_config_index(config_index) , m_assume_unique_hostnames(assume_unique_hostnames) + , m_query_events(query_events) { mxb_assert(monitored_server); } @@ -839,6 +831,10 @@ void MariaDBServer::monitor_server() { query_ok = update_gtids(&errmsg); } + if (query_ok && m_query_events) + { + query_ok = update_enabled_events(); + } } else { @@ -1222,22 +1218,24 @@ const SlaveStatus* MariaDBServer::slave_connection_status_host_port(const MariaD return NULL; } -bool MariaDBServer::enable_events(json_t** error_out) +bool MariaDBServer::enable_events(const EventNameSet& event_names, json_t** error_out) { int found_disabled_events = 0; int events_enabled = 0; - // Helper function which enables a slaveside disabled event. - ManipulatorFunc enabler = [this, &found_disabled_events, &events_enabled](const EventInfo& event, - json_t** error_out) { - if (event.status == "SLAVESIDE_DISABLED") + + // Helper function which enables a disabled event if that event name is found in the events-set. + ManipulatorFunc enabler = [this, event_names, &found_disabled_events, &events_enabled]( + const EventInfo& event, json_t** error_out) { + if (event_names.count(event.name) > 0 + && (event.status == "SLAVESIDE_DISABLED" || event.status == "DISABLED")) + { + found_disabled_events++; + if (alter_event(event, "ENABLE", error_out)) { - found_disabled_events++; - if (alter_event(event, "ENABLE", error_out)) - { - events_enabled++; - } + events_enabled++; } - }; + } + }; bool rval = false; if (events_foreach(enabler, error_out)) @@ -1361,8 +1359,7 @@ bool MariaDBServer::events_foreach(ManipulatorFunc& func, json_t** error_out) while (event_info->next_row()) { EventInfo event; - event.database = event_info->get_string(db_name_ind); - event.name = event_info->get_string(event_name_ind); + event.name = event_info->get_string(db_name_ind) + "." + event_info->get_string(event_name_ind); event.definer = event_info->get_string(event_definer_ind); event.status = event_info->get_string(event_status_ind); func(event, error_out); @@ -1382,51 +1379,40 @@ bool MariaDBServer::alter_event(const EventInfo& event, const string& target_sta { bool rval = false; string error_msg; - // First switch to the correct database. - string use_db_query = string_printf("USE %s;", event.database.c_str()); - if (execute_cmd(use_db_query, &error_msg)) + // An ALTER EVENT by default changes the definer (owner) of the event to the monitor user. + // This causes problems if the monitor user does not have privileges to run + // the event contents. Prevent this by setting definer explicitly. + // The definer may be of the form user@host. If host includes %, then it must be quoted. + // For simplicity, quote the host always. + string quoted_definer; + auto loc_at = event.definer.find('@'); + if (loc_at != string::npos) { - // An ALTER EVENT by default changes the definer (owner) of the event to the monitor user. - // This causes problems if the monitor user does not have privileges to run - // the event contents. Prevent this by setting definer explicitly. - // The definer may be of the form user@host. If host includes %, then it must be quoted. - // For simplicity, quote the host always. - string quoted_definer; - auto loc_at = event.definer.find('@'); - if (loc_at != string::npos) - { - auto host_begin = loc_at + 1; - quoted_definer = event.definer.substr(0, loc_at + 1) - + // host_begin may be the null-char if @ was the last char - "'" + event.definer.substr(host_begin, string::npos) + "'"; - } - else - { - // Just the username - quoted_definer = event.definer; - } - string alter_event_query = string_printf("ALTER DEFINER = %s EVENT %s %s;", - quoted_definer.c_str(), - event.name.c_str(), - target_status.c_str()); - if (execute_cmd(alter_event_query, &error_msg)) - { - rval = true; - const char FMT[] = "Event '%s' of database '%s' on server '%s' set to '%s'."; - MXS_NOTICE(FMT, event.name.c_str(), event.database.c_str(), name(), target_status.c_str()); - } - else - { - const char FMT[] = "Could not alter event '%s' of database '%s' on server '%s': %s"; - PRINT_MXS_JSON_ERROR(error_out, FMT, event.name.c_str(), event.database.c_str(), name(), - error_msg.c_str()); - } + auto host_begin = loc_at + 1; + quoted_definer = event.definer.substr(0, loc_at + 1) + + // host_begin may be the null-char if @ was the last char + "'" + event.definer.substr(host_begin, string::npos) + "'"; } else { - const char FMT[] = "Could not switch to database '%s' on '%s': %s Event '%s' not altered."; - PRINT_MXS_JSON_ERROR(error_out, FMT, event.database.c_str(), name(), error_msg.c_str(), - event.name.c_str()); + // Just the username + quoted_definer = event.definer; + } + + string alter_event_query = string_printf("ALTER DEFINER = %s EVENT %s %s;", + quoted_definer.c_str(), + event.name.c_str(), + target_status.c_str()); + if (execute_cmd(alter_event_query, &error_msg)) + { + rval = true; + const char FMT[] = "Event '%s' on server '%s' set to '%s'."; + MXS_NOTICE(FMT, event.name.c_str(), name(), target_status.c_str()); + } + else + { + const char FMT[] = "Could not alter event '%s' on server '%s': %s"; + PRINT_MXS_JSON_ERROR(error_out, FMT, event.name.c_str(), name(), error_msg.c_str()); } return rval; } @@ -1510,7 +1496,7 @@ bool MariaDBServer::promote(GeneralOpData& general, ServerOperation& promotion, if (promotion.handle_events) { // TODO: Add query replying to enable_events - bool events_enabled = enable_events(error_out); + bool events_enabled = enable_events(promotion.events_to_enable, error_out); general.time_remaining -= timer.restart(); if (!events_enabled) { @@ -2119,3 +2105,33 @@ bool MariaDBServer::redirect_existing_slave_conn(GeneralOpData& op, const SlaveS } // 'stop_slave_conn' prints its own errors return success; } + +bool MariaDBServer::update_enabled_events() +{ + string error_msg; + // Get names of all enabled scheduled events on the server. + auto event_info = execute_query("SELECT Event_schema, Event_name FROM information_schema.EVENTS WHERE " + "Status = 'ENABLED';", &error_msg); + if (event_info.get() == NULL) + { + MXS_ERROR("Could not query events of '%s': %s Event handling can be disabled by " + "setting '%s' to false.", + name(), error_msg.c_str(), CN_HANDLE_EVENTS); + return false; + } + + auto db_name_ind = 0; + auto event_name_ind = 1; + + EventNameSet full_names; + full_names.reserve(event_info->get_row_count()); + + while (event_info->next_row()) + { + string full_name = event_info->get_string(db_name_ind) + "." + event_info->get_string(event_name_ind); + full_names.insert(full_name); // Ignore duplicates, they shouldn't exists. + } + + m_enabled_events = std::move(full_names); + return true; +} \ No newline at end of file diff --git a/server/modules/monitor/mariadbmon/mariadbserver.hh b/server/modules/monitor/mariadbmon/mariadbserver.hh index 374d72f76..ff78ec810 100644 --- a/server/modules/monitor/mariadbmon/mariadbserver.hh +++ b/server/modules/monitor/mariadbmon/mariadbserver.hh @@ -72,6 +72,17 @@ struct NodeData class MariaDBServer { public: + MariaDBServer(MXS_MONITORED_SERVER* monitored_server, int config_index, + bool assume_unique_hostnames, bool query_events); + + class EventInfo + { + public: + std::string name; /**< Event name in form */ + std::string definer; /**< Definer of the event */ + std::string status; /**< Status of the event */ + }; + enum class server_type { UNKNOWN, /* Totally unknown. Server has not been connected to yet. */ @@ -135,10 +146,10 @@ public: * 'update_replication_settings' before use. */ ReplicationSettings m_rpl_settings; - bool m_print_update_errormsg = true; /* Should an update error be printed? */ + bool m_query_events; /* Copy of monitor->m_handle_event_scheduler. TODO: move elsewhere */ + EventNameSet m_enabled_events; /* Enabled scheduled events */ - MariaDBServer(MXS_MONITORED_SERVER* monitored_server, int config_index, - bool assume_unique_hostnames = true); + bool m_print_update_errormsg = true; /* Should an update error be printed? */ /** * Print server information to a json object. @@ -329,12 +340,14 @@ public: bool run_sql_from_file(const std::string& path, json_t** error_out); /** - * Enable any "SLAVESIDE_DISABLED" events. Event scheduler is not touched. + * Enable any "SLAVESIDE_DISABLED" or "DISABLED events. Event scheduler is not touched. Only events + * with names matching an element in the event_names set are enabled. * + * @param event_names Names of events that should be enabled * @param error_out Error output * @return True if all SLAVESIDE_DISABLED events were enabled */ - bool enable_events(json_t** error_out); + bool enable_events(const EventNameSet& event_names, json_t** error_out); /** * Disable any "ENABLED" events. Event scheduler is not touched. @@ -516,7 +529,6 @@ public: void set_status(uint64_t bits); private: - class EventInfo; typedef std::function ManipulatorFunc; enum class StopMode @@ -558,4 +570,6 @@ private: bool set_read_only(ReadOnlySetting value, maxbase::Duration time_limit, json_t** error_out); bool merge_slave_conns(GeneralOpData& op, const SlaveStatusArray& conns_to_merge); std::string generate_change_master_cmd(GeneralOpData& op, const SlaveStatus& slave_conn); + + bool update_enabled_events(); }; diff --git a/server/modules/monitor/mariadbmon/server_utils.cc b/server/modules/monitor/mariadbmon/server_utils.cc index b3422cf47..072dcb2d1 100644 --- a/server/modules/monitor/mariadbmon/server_utils.cc +++ b/server/modules/monitor/mariadbmon/server_utils.cc @@ -155,14 +155,21 @@ bool SlaveStatus::should_be_copied(string* ignore_reason_out) const return accepted; } -ServerOperation::ServerOperation(MariaDBServer* target, bool was_is_master, - bool handle_events, const std::string& sql_file, - const SlaveStatusArray& conns_to_copy) +ServerOperation::ServerOperation(MariaDBServer* target, bool was_is_master, bool handle_events, + const std::string& sql_file, const SlaveStatusArray& conns_to_copy, + const EventNameSet& events_to_enable) : target(target) , to_from_master(was_is_master) , handle_events(handle_events) , sql_file(sql_file) , conns_to_copy(conns_to_copy) + , events_to_enable(events_to_enable) +{ +} + +ServerOperation::ServerOperation(MariaDBServer* target, bool was_is_master, bool handle_events, + const std::string& sql_file) + : ServerOperation(target, was_is_master, handle_events, sql_file, {}, {}) { } diff --git a/server/modules/monitor/mariadbmon/server_utils.hh b/server/modules/monitor/mariadbmon/server_utils.hh index 64d9cd24a..3f3c4b39c 100644 --- a/server/modules/monitor/mariadbmon/server_utils.hh +++ b/server/modules/monitor/mariadbmon/server_utils.hh @@ -13,6 +13,7 @@ #pragma once #include "mariadbmon_common.hh" +#include #include #include #include @@ -209,7 +210,8 @@ public: bool should_be_copied(std::string* ignore_reason_out) const; }; -typedef std::vector SlaveStatusArray; +using SlaveStatusArray = std::vector; +using EventNameSet = std::unordered_set; enum class OperationType { @@ -239,8 +241,15 @@ public: const std::string sql_file; // Path to file with SQL commands to run during op const SlaveStatusArray conns_to_copy; // Slave connections the target should copy/merge + const EventNameSet events_to_enable; // Scheduled event names last seen on master. + ServerOperation(MariaDBServer* target, bool was_is_master, bool handle_events, - const std::string& sql_file, const SlaveStatusArray& conns_to_copy); + const std::string& sql_file, const SlaveStatusArray& conns_to_copy, + const EventNameSet& events_to_enable); + + ServerOperation(MariaDBServer* target, bool was_is_master, bool handle_events, + const std::string& sql_file); + }; /** diff --git a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc index e206bb4dd..76b96cbd7 100644 --- a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc +++ b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc @@ -162,7 +162,7 @@ void MariaDBMonitor::Test::init_servers(int count) MXS_MONITORED_SERVER* mon_server = new MXS_MONITORED_SERVER; // Contents mostly undefined mon_server->server = base_server; - MariaDBServer* mariadb_server = new MariaDBServer(mon_server, i - 1, m_use_hostnames); + MariaDBServer* mariadb_server = new MariaDBServer(mon_server, i - 1, m_use_hostnames, false); if (m_use_hostnames) {