Merge branch '2.2' into develop

Documentation/Routers/ReadConnRoute.md

@@ -21,7 +21,7 @@ Here is a list of all possible values for the `router_options`.
 Role|Description
 ------|---------
 master|A server assigned as a master by one of MariaDB MaxScale monitors. Depending on the monitor implementation, this could be a master server of a Master-Slave replication cluster or a Write-Master of a Galera cluster.
-slave|A server assigned as a slave of a master.
+slave|A server assigned as a slave of a master. If all slaves are down, but the master is still available, then the router will use the master.
 synced| A Galera cluster node which is in a synced state with the cluster.
 ndb|A MySQL Replication Cluster node
 running|A server that is up and running. All servers that MariaDB MaxScale can connect to are labeled as running.

maxscale-system-test/CMakeLists.txt

@@ -980,6 +980,10 @@ add_test_executable(mxs1836_show_eventTimes.cpp mxs1836_show_eventTimes mxs1836_
 # MXS-173 throttling filter
 add_test_executable(mxs173.cpp mxs173_throttle_filter mxs173_throttle_filter LABELS REPL_BACKEND)
 
+ # MXS-1889: A single remaining master is valid for readconnroute configured with 'router_options=slave'
+ # https://jira.mariadb.org/browse/MXS-1889
+add_test_executable(mxs1889.cpp mxs1889 mxs1889 LABELS REPL_BACKEND)
+
 configure_file(templates.h.in templates.h @ONLY)
 
 include(CTest)

maxscale-system-test/cnf/maxscale.cnf.template.mxs1889

@@ -0,0 +1,89 @@
+[maxscale]
+threads=###threads###
+log_info=on
+
+[MySQL Monitor]
+type=monitor
+module=mysqlmon
+###repl51###
+servers=server1,server2,server3,server4
+user=maxskysql
+passwd=skysql
+monitor_interval=1000
+
+[RW Split Router]
+type=service
+router=readwritesplit
+servers=server1,server2,server3,server4
+user=maxskysql
+passwd=skysql
+router_options=slave_selection_criteria=LEAST_GLOBAL_CONNECTIONS
+max_slave_connections=1
+
+[Read Connection Router Slave]
+type=service
+router=readconnroute
+router_options=slave
+servers=server1,server2,server3,server4
+user=maxskysql
+passwd=skysql
+
+[Read Connection Router Master]
+type=service
+router=readconnroute
+router_options=master
+servers=server1,server2,server3,server4
+user=maxskysql
+passwd=skysql
+
+[RW Split Listener]
+type=listener
+service=RW Split Router
+protocol=MySQLClient
+port=4006
+
+[Read Connection Listener Slave]
+type=listener
+service=Read Connection Router Slave
+protocol=MySQLClient
+port=4009
+
+[Read Connection Listener Master]
+type=listener
+service=Read Connection Router Master
+protocol=MySQLClient
+port=4008
+
+[CLI]
+type=service
+router=cli
+
+[CLI Listener]
+type=listener
+service=CLI
+protocol=maxscaled
+socket=default
+
+[server1]
+type=server
+address=###node_server_IP_1###
+port=###node_server_port_1###
+protocol=MySQLBackend
+
+[server2]
+type=server
+address=###node_server_IP_2###
+port=###node_server_port_2###
+protocol=MySQLBackend
+
+[server3]
+type=server
+address=###node_server_IP_3###
+port=###node_server_port_3###
+protocol=MySQLBackend
+
+[server4]
+type=server
+address=###node_server_IP_4###
+port=###node_server_port_4###
+protocol=MySQLBackend

maxscale-system-test/mxs1889.cpp

@@ -0,0 +1,85 @@
+/**
+ * MXS-1889: A single remaining master is valid for readconnroute configured with 'router_options=slave'
+ *
+ * https://jira.mariadb.org/browse/MXS-1889
+ */
+
+#include "testconnections.h"
+#include <iostream>
+#include <string>
+
+using namespace std;
+
+namespace
+{
+
+string get_server_id(TestConnections& test, MYSQL* pMysql)
+{
+    string id;
+
+    int rv = mysql_query(pMysql, "SELECT @@server_id");
+    test.add_result(rv, "Could not execute query.");
+
+    if (rv == 0)
+    {
+        MYSQL_RES* pResult = mysql_store_result(pMysql);
+        test.assert(pResult, "Could not store result.");
+
+        if (pResult)
+        {
+            unsigned int n = mysql_field_count(pMysql);
+            test.assert(n == 1, "Unexpected number of fields.");
+
+            MYSQL_ROW pzRow = mysql_fetch_row(pResult);
+            test.assert(pzRow, "Returned row was NULL.");
+
+            if (pzRow)
+            {
+                id = pzRow[0];
+            }
+
+            mysql_free_result(pResult);
+        }
+    }
+
+    return id;
+}
+
+}
+
+int main(int argc, char** argv)
+{
+    TestConnections test(argc, argv);
+
+    test.tprintf("Taking down all slaves.");
+    test.repl->stop_node(1);
+    test.repl->stop_node(2);
+    test.repl->stop_node(3);
+
+    test.tprintf("Giving monitor time to detect the situation...");
+    sleep(5);
+
+    test.maxscales->connect();
+
+    // All slaves down, so we expect a connection to the master.
+    string master_id = get_server_id(test, test.maxscales->conn_slave[0]);
+    test.tprintf("Master id: %s", master_id.c_str());
+
+    test.maxscales->disconnect();
+
+    test.tprintf("Starting all slaves.");
+    test.repl->start_node(3);
+    test.repl->start_node(2);
+    test.repl->start_node(1);
+
+    test.tprintf("Giving monitor time to detect the situation...");
+    sleep(5);
+
+    test.maxscales->connect();
+
+    string slave_id = get_server_id(test, test.maxscales->conn_slave[0]);
+    test.tprintf("Server id: %s", slave_id.c_str());
+    test.assert(slave_id != master_id, "Expected something else but %s", master_id.c_str());
+
+    return test.global_result;
+}

server/modules/routing/readconnroute/readconnection.h

@@ -49,6 +49,7 @@ typedef struct router_client_session
     SERVER_REF *backend; /*< Backend used by the client session */
     DCB *backend_dcb; /*< DCB Connection to the backend      */
     DCB *client_dcb; /**< Client DCB */
+    unsigned int bitvalue; /*< Session specific required value of server->status */
     struct router_client_session *next;
 #if defined(SS_DEBUG)
     skygw_chk_t rses_chk_tail;

server/modules/routing/readconnroute/readconnroute.cc

@@ -286,6 +286,7 @@ newSession(MXS_ROUTER *instance, MXS_SESSION *session)
     client_rses->rses_chk_tail = CHK_NUM_ROUTER_SES;
 #endif
     client_rses->client_dcb = session->client_dcb;
+    client_rses->bitvalue = inst->bitvalue;
 
     /**
      * Find the Master host from available servers
@@ -397,6 +398,17 @@ newSession(MXS_ROUTER *instance, MXS_SESSION *session)
         if (master_host)
         {
             candidate = master_host;
+            // Even if we had 'router_options=slave' in the configuration file, we
+            // will still end up here if there are no slaves, but a sole master. So
+            // that the server will be considered valid in connection_is_valid(), we
+            // turn on the SERVER_MASTER bit.
+            //
+            // We must do that so that readconnroute in MaxScale 2.2 will again behave
+            // the same way as it did up until 2.1.12.
+            if (client_rses->bitvalue & SERVER_SLAVE)
+            {
+                client_rses->bitvalue |= SERVER_MASTER;
+            }
         }
         else
         {
@@ -542,10 +554,17 @@ static inline bool connection_is_valid(ROUTER_INSTANCE* inst, ROUTER_CLIENT_SES*
 {
     bool rval = false;
 
+    // inst->bitvalue and router_cli_ses->bitvalue are different, if we had
+    // 'router_options=slave' in the configuration file and there was only
+    // the sole master available at session creation time.
+
     if (SERVER_IS_RUNNING(router_cli_ses->backend->server) &&
-        (router_cli_ses->backend->server->status & inst->bitmask & inst->bitvalue))
+        (router_cli_ses->backend->server->status & inst->bitmask & router_cli_ses->bitvalue))
     {
-        if ((inst->bitvalue == SERVER_MASTER) && router_cli_ses->backend->active)
+        // Note the use of '==' and not '|'. We must use the former to exclude a
+        // 'router_options=slave' that uses the master due to no slave having been
+        // available at session creation time. Its bitvalue is (SERVER_MASTER | SERVER_SLAVE).
+        if ((router_cli_ses->bitvalue == SERVER_MASTER) && router_cli_ses->backend->active)
         {
             // If we're using an active master server, verify that it is still a master
             rval = router_cli_ses->backend == get_root_master(inst->service->dbref);