Merge branch '2.3' into develop

Documentation/Routers/SchemaRouter.md

@@ -139,24 +139,20 @@ refresh_interval=60
 
 This functionality was introduced in 2.3.0.
 
-If the same database exists on multiple servers, but the database contains
+If the same database exists on multiple servers, but the database contains different
-different tables in each server, the SchemaRouter is capable of
+tables in each server, SchemaRouter is capable of routing queries to the right server,
-transparently routing queries to the right server, depending on which table
+depending on which table is being addressed.
-is being addressed.
 
-For instance, suppose the database `db` exists on servers _server1_ and
+As an example, suppose the database `db` exists on servers _server1_ and _server2_, but
-_server2_, but that the database on _server1_ contains the table `tbl1` and
+that the database on _server1_ contains the table `tbl1` and on _server2_ contains the
-on _server2_ contains the table `tbl2`.
+table `tbl2`. The query `SELECT * FROM db.tbl1` will be routed to _server1_ and the query
-
+`SELECT * FROM db.tbl2` will be routed to _server2_. As in the example queries, the table
-In that case, the query
+names must be qualified with the database names for table-level sharding to work.
+Specifically, the query series below is not supported.
 ```
-SELECT * FROM db.tbl1
+USE db;
+SELECT * FROM tbl1; // May be routed to an incorrect backend if using table sharding.
 ```
-will be routed to _server1_ and the query
-```
-SELECT * FROM db.tbl2
-```
-will be routed to _server2_.
 
 ## Router Options
 

server/modules/monitor/mariadbmon/mariadbmon.cc

@@ -265,33 +265,19 @@ bool MariaDBMonitor::configure(const MXS_CONFIG_PARAMETER* params)
 void MariaDBMonitor::diagnostics(DCB* dcb) const
 {
     /* The problem with diagnostic printing is that some of the printed elements are array-like and their
-     * length could change during a monitor loop. Thus, the variables should only be read by the monitor
+     * length could change during a monitor loop. Such variables are protected by mutexes. Locking is
-     * thread and not the admin thread. Because the diagnostic must be printable even when the monitor is
+     * only required when the monitor thread writes to such a variable and when the admin thread is
-     * not running, the printing must be done outside the normal loop. */
+     * reading it. */
 
     mxb_assert(mxs_rworker_get_current() == mxs_rworker_get(MXS_RWORKER_MAIN));
-    /* The 'dcb' is owned by the admin thread (the thread executing this function), and probably
+    dcb_printf(dcb, "%s", diagnostics_to_string().c_str());
-     * should not be written to by any other thread. To prevent this, have the monitor thread
-     * print the diagnostics to a string. */
-    string diag_str;
-
-    // 'execute' is not a const method, although the task we are sending is.
-    MariaDBMonitor* mutable_ptr = const_cast<MariaDBMonitor*>(this);
-    auto func = [this, &diag_str] {
-            diag_str = diagnostics_to_string();
-        };
-
-    if (!mutable_ptr->call(func, Worker::EXECUTE_AUTO))
-    {
-        diag_str = DIAG_ERROR;
-    }
-
-    dcb_printf(dcb, "%s", diag_str.c_str());
 }
 
 string MariaDBMonitor::diagnostics_to_string() const
 {
     string rval;
+    rval.reserve(1000); // Enough for basic output.
+
     rval += string_printf("Automatic failover:     %s\n", m_auto_failover ? "Enabled" : "Disabled");
     rval += string_printf("Failcount:              %d\n", m_failcount);
     rval += string_printf("Failover timeout:       %u\n", m_failover_timeout);
@@ -317,24 +303,16 @@ string MariaDBMonitor::diagnostics_to_string() const
 json_t* MariaDBMonitor::diagnostics_json() const
 {
     mxb_assert(mxs_rworker_get_current() == mxs_rworker_get(MXS_RWORKER_MAIN));
-    json_t* rval = NULL;
+    return to_json();
-    MariaDBMonitor* mutable_ptr = const_cast<MariaDBMonitor*>(this);
-    auto func = [this, &rval] {
-            rval = to_json();
-        };
-
-    if (!mutable_ptr->call(func, Worker::EXECUTE_AUTO))
-    {
-        rval = mxs_json_error_append(rval, "%s", DIAG_ERROR);
-    }
-
-    return rval;
 }
 
 json_t* MariaDBMonitor::to_json() const
 {
     json_t* rval = MonitorWorker::diagnostics_json();
-    json_object_set_new(rval, "master", m_master == NULL ? json_null() : json_string(m_master->name()));
+
+    // The m_master-pointer can be modified during a tick, but the pointed object cannot be deleted.
+    auto master = mxb::atomic::load(&m_master, mxb::atomic::RELAXED);
+    json_object_set_new(rval, "master", master == nullptr ? json_null() : json_string(master->name()));
     json_object_set_new(rval,
                         "master_gtid_domain_id",
                         m_master_gtid_domain == GTID_DOMAIN_UNKNOWN ? json_null() :
@@ -690,7 +668,7 @@ void MariaDBMonitor::log_master_changes()
 
 void MariaDBMonitor::assign_new_master(MariaDBServer* new_master)
 {
-    m_master = new_master;
+    mxb::atomic::store(&m_master, new_master, mxb::atomic::RELAXED);
     update_master_cycle_info();
     m_warn_current_master_invalid = true;
     m_warn_have_better_master = true;
@@ -824,8 +802,6 @@ bool MariaDBMonitor::run_manual_reset_replication(SERVER* master_server, json_t*
     return send_ok && rval;
 }
 
-
-
 /**
  * Command handler for 'switchover'
  *

server/modules/monitor/mariadbmon/mariadbserver.cc

@@ -27,6 +27,7 @@ using maxbase::string_printf;
 using maxbase::Duration;
 using maxbase::StopWatch;
 using maxsql::QueryResult;
+using Guard = std::lock_guard<std::mutex>;
 
 MariaDBServer::MariaDBServer(MXS_MONITORED_SERVER* monitored_server, int config_index,
                              bool assume_unique_hostnames, bool query_events)
@@ -399,6 +400,7 @@ bool MariaDBServer::do_show_slave_status(string* errmsg_out)
 
         // Always write to m_slave_status. Even if the new status is equal by topology,
         // gtid:s etc may have changed.
+        Guard guard(m_arraylock);
         m_slave_status = std::move(slave_status_new);
     }
 
@@ -415,6 +417,8 @@ bool MariaDBServer::update_gtids(string* errmsg_out)
     auto result = execute_query(query, errmsg_out);
     if (result.get() != NULL)
     {
+        Guard guard(m_arraylock);
+
         rval = true;
         if (result->next_row())
         {
@@ -677,9 +681,12 @@ string MariaDBServer::diagnostics() const
     const char fmt_int64[] = "%-23s %" PRIi64 "\n";
 
     string rval;
+    rval.reserve(300); // Enough for most common ouput.
+
     rval += string_printf(fmt_string, "Server:", name());
     rval += string_printf(fmt_int64, "Server ID:", m_server_id);
     rval += string_printf(fmt_string, "Read only:", (m_read_only ? "Yes" : "No"));
+    Guard guard(m_arraylock);
     if (!m_gtid_current_pos.empty())
     {
         rval += string_printf(fmt_string, "Gtid current position:", m_gtid_current_pos.to_string().c_str());
@@ -708,6 +715,7 @@ json_t* MariaDBServer::to_json() const
     json_object_set_new(result, "server_id", json_integer(m_server_id));
     json_object_set_new(result, "read_only", json_boolean(m_read_only));
 
+    Guard guard(m_arraylock);
     json_object_set_new(result,
                         "gtid_current_pos",
                         m_gtid_current_pos.empty() ? json_null() :

server/modules/monitor/mariadbmon/mariadbserver.hh

@@ -15,6 +15,7 @@
 #include <functional>
 #include <string>
 #include <memory>
+#include <mutex>
 #include <maxbase/stopwatch.hh>
 #include <maxsql/mariadb.hh>
 #include <maxscale/monitor.hh>
@@ -115,6 +116,7 @@ public:
         bool log_slave_updates = false; /* Does the slave write replicated events to binlog? */
     };
 
+
     /* Monitored server base class/struct. MariaDBServer does not own the struct, it is not freed
      * (or connection closed) when a MariaDBServer is destroyed. */
     MXS_MONITORED_SERVER* m_server_base = NULL;
@@ -549,6 +551,12 @@ private:
         DISABLE
     };
 
+    /* Protects array-like fields from concurrent access. This is only required for fields which can be
+     * read from another thread while the monitor is running. In practice, these are fields read during
+     * diagnostics-methods. Reading inside monitor thread does not need to be mutexed, as outside threads
+     * only read the values. */
+    mutable std::mutex m_arraylock;
+
     bool               update_slave_status(std::string* errmsg_out = NULL);
     bool               sstatus_array_topology_equal(const SlaveStatusArray& new_slave_status);
     const SlaveStatus* sstatus_find_previous_row(const SlaveStatus& new_row, size_t guess);