Changes to Galera monitoring logic and tidying up following code review.

server/core/monitor.c

@@ -45,6 +45,20 @@
 #include <externcmd.h>
 #include <mysql/mysqld_error.h>
 
+/*
+ *  Create declarations of the enum for monitor events and also the array of
+ *  structs containing the matching names. The data is taken from def_monitor_event.h
+ *
+ */
+
+#undef ADDITEM
+#define ADDITEM( _event_type, _event_name ) { #_event_name }
+const monitor_def_t monitor_event_definitions[MAX_MONITOR_EVENT] =
+{
+#include "def_monitor_event.h"
+};
+#undef ADDITEM
+
 static MONITOR	*allMonitors = NULL;
 static SPINLOCK	monLock = SPINLOCK_INIT;
 
@@ -648,10 +662,10 @@ mon_get_event_type(MONITOR_SERVERS* node)
                         (present & SERVER_NDB) ? NDB_UP_EVENT :
                             SERVER_UP_EVENT;
         case DOWN_EVENT:
-            return (present & SERVER_MASTER) ? MASTER_DOWN_EVENT :
+            return (prev & SERVER_MASTER) ? MASTER_DOWN_EVENT :
-                (present & SERVER_SLAVE) ? SLAVE_DOWN_EVENT :
+                (prev & SERVER_SLAVE) ? SLAVE_DOWN_EVENT :
-                    (present & SERVER_JOINED) ? SYNCED_DOWN_EVENT :
+                    (prev & SERVER_JOINED) ? SYNCED_DOWN_EVENT :
-                        (present & SERVER_NDB) ? NDB_DOWN_EVENT :
+                        (prev & SERVER_NDB) ? NDB_DOWN_EVENT :
                             SERVER_DOWN_EVENT;
         case LOSS_EVENT:
             return (prev & SERVER_MASTER) ? LOST_MASTER_EVENT :
@@ -686,7 +700,7 @@ mon_get_event_name(MONITOR_SERVERS* node)
  * @result  monitor_event_t     Monitor event corresponding to name
  */
 monitor_event_t
-mon_name_to_event (char *event_name)
+mon_name_to_event (const char *event_name)
 {
     monitor_event_t event;
 
@@ -797,15 +811,16 @@ monitor_launch_script(MONITOR* mon, MONITOR_SERVERS* ptr, char* script)
  * found.
  */
 int
-mon_parse_event_string(bool* events, size_t count, char* string)
+mon_parse_event_string(bool* events, size_t count, char* given_string)
 {
-    char *tok, *saved;
+    char *tok, *saved, *string = strdup(given_string);
     monitor_event_t event;
 
     tok = strtok_r(string, ",| ", &saved);
 
     if (tok == NULL)
     {
+        free(string);
         return -1;
     }
 
@@ -815,12 +830,17 @@ mon_parse_event_string(bool* events, size_t count, char* string)
         if (event == UNDEFINED_MONITOR_EVENT)
         {
             MXS_ERROR("Invalid event name %s", tok);
+            free(string);
             return -1;
         }
+        if (event < count)
+        {
             events[event] = true;
             tok = strtok_r(NULL, ",| ", &saved);
         }
+    }
 
+    free(string);
     return 0;
 }
 

server/core/server.c

@@ -674,6 +674,28 @@ server_set_status(SERVER *server, int bit)
 	}
 }
 
+/**
+ * Set one or more status bit(s) from a specified set, clearing any others
+ * in the specified set
+ *
+ * @param server	The server to update
+ * @param bit		The bit to set for the server
+ */
+void
+server_clear_set_status(SERVER *server, int specified_bits, int bits_to_set)
+{
+	/** clear error logged flag before the next failure */
+	if ((bits_to_set & SERVER_MASTER) && ((server->status & SERVER_MASTER) == 0))
+	{
+		server->master_err_is_logged = false;
+	}
+
+    if ((server->status & specified_bits) != bits_to_set)
+    {
+        server->status = (server->status & ~specified_bits) | bits_to_set;
+    }
+}
+
 /**
  * Clear a status bit in the server
  *

server/include/def_monitor_event.h

@@ -1,14 +1,29 @@
 /*
- * To change this license header, choose License Headers in Project Properties.
+ * This file is distributed as part of the MariaDB Corporation MaxScale.  It is free
- * To change this template file, choose Tools | Templates
+ * software: you can redistribute it and/or modify it under the terms of the
- * and open the template in the editor.
+ * GNU General Public License as published by the Free Software Foundation,
+ * version 2.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 51
+ * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Copyright MariaDB Corporation Ab 2013-2014
  */
 
-/* 
+/**
- * File:   def_event.h
+ * @file def_monitor_event.h
- * Author: mbrampton
  *
- * Created on 18 November 2015, 15:21
+ * @verbatim
+ * Revision History
+ *
+ * Date		Who			Description
+ * 01-06-2013	Martin Brampton		Initial implementation
  */
 
 ADDITEM( UNDEFINED_MONITOR_EVENT, undefined ),

server/include/monitor.h

@@ -153,13 +153,7 @@ typedef struct monitor_def_s
     char name[30];
 } monitor_def_t;
 
-#undef ADDITEM
+extern const monitor_def_t monitor_event_definitions[];
-#define ADDITEM( _event_type, _event_name ) { #_event_name }
-static const monitor_def_t monitor_event_definitions[MAX_MONITOR_EVENT] =
-{
-#include "def_monitor_event.h"
-};
-#undef ADDITEM
 
 /**
  * The linked list of servers that are being monitored by the monitor module.
@@ -217,7 +211,7 @@ extern void     monitorSetNetworkTimeout(MONITOR *, int, int);
 extern RESULTSET *monitorGetList();
 bool check_monitor_permissions(MONITOR* monitor);
 
-monitor_event_t mon_name_to_event(char* tok);
+monitor_event_t mon_name_to_event(const char* tok);
 void mon_append_node_names(MONITOR_SERVERS* start, char* str, int len);
 monitor_event_t mon_get_event_type(MONITOR_SERVERS* node);
 const char* mon_get_event_name(MONITOR_SERVERS* node);

server/include/server.h

@@ -200,6 +200,7 @@ extern void	dprintServer(DCB *, SERVER *);
 extern void     dprintPersistentDCBs(DCB *, SERVER *);
 extern void	dListServers(DCB *);
 extern char	*server_status(SERVER *);
+extern void     server_clear_set_status(SERVER *server, int specified_bits, int bits_to_set);
 extern void	server_set_status(SERVER *, int);
 extern void	server_clear_status(SERVER *, int);
 extern void     server_transfer_status(SERVER *dest_server, SERVER *source_server);

server/modules/monitor/galeramon.c

@@ -267,7 +267,9 @@ diagnostics(DCB *dcb, void *arg)
 }
 
 /**
- * Monitor an individual server
+ * Monitor an individual server. Does not deal with the setting of master or
+ * slave bits, except for clearing them when a server is not joined to the
+ * cluster.
  *
  * @param handle        The MySQL Monitor object
  * @param database      The database to probe
@@ -292,12 +294,8 @@ monitorDatabase(MONITOR *mon, MONITOR_SERVERS *database)
 
     server_transfer_status(&temp_server, database->server);
     server_clear_status(&temp_server, SERVER_RUNNING);
-
+    /* Also clear Joined */
-    /* Also clear Joined, M/S and Stickiness bits */
     server_clear_status(&temp_server, SERVER_JOINED);
-    server_clear_status(&temp_server, SERVER_SLAVE);
-    server_clear_status(&temp_server, SERVER_MASTER);
-    server_clear_status(&temp_server, SERVER_MASTER_STICKINESS);
 
     connect_result_t rval = mon_connect_to_db(mon, database);
     if (rval != MONITOR_CONN_OK)
@@ -413,6 +411,19 @@ monitorDatabase(MONITOR *mon, MONITOR_SERVERS *database)
         server_clear_status(&temp_server, SERVER_JOINED);
     }
 
+    /* clear bits for non member nodes */
+    if (!SERVER_IN_MAINT(database->server) && (!SERVER_IS_JOINED(&temp_server)))
+    {
+        database->server->depth = -1;
+
+        /* clear M/S status */
+        server_clear_status(&temp_server, SERVER_SLAVE);
+        server_clear_status(&temp_server, SERVER_MASTER);
+
+        /* clear master sticky status */
+        server_clear_status(&temp_server, SERVER_MASTER_STICKINESS);
+    }
+
     server_transfer_status(database->server, &temp_server);
 }
 
@@ -482,19 +493,6 @@ monitorMain(void *arg)
 
             monitorDatabase(mon, ptr);
 
-            /* clear bits for non member nodes */
-            if (!SERVER_IN_MAINT(ptr->server) && (!SERVER_IS_JOINED(ptr->server)))
-            {
-                ptr->server->depth = -1;
-
-                /* clear M/S status */
-                server_clear_status(ptr->server, SERVER_SLAVE);
-                server_clear_status(ptr->server, SERVER_MASTER);
-
-                /* clear master sticky status */
-                server_clear_status(ptr->server, SERVER_MASTER_STICKINESS);
-            }
-
             /* Log server status change */
             if (mon_status_changed(ptr))
             {
@@ -560,28 +558,24 @@ monitorMain(void *arg)
             {
                 if (ptr != handle->master)
                 {
-                    /* set the Slave role */
+                    /* set the Slave role and clear master stickiness */
-                    server_set_status(ptr->server, SERVER_SLAVE);
+                    server_clear_set_status(ptr->server, (SERVER_SLAVE|SERVER_MASTER|SERVER_MASTER_STICKINESS), SERVER_SLAVE);
-                    server_clear_status(ptr->server, SERVER_MASTER);
-
-                    /* clear master stickiness */
-                    server_clear_status(ptr->server, SERVER_MASTER_STICKINESS);
                 }
                 else
                 {
-                    /* set the Master role */
-                    server_set_status(handle->master->server, SERVER_MASTER);
-                    server_clear_status(handle->master->server, SERVER_SLAVE);
-
                     if (candidate_master && handle->master->server->node_id != candidate_master->server->node_id)
                     {
-                        /* set master stickiness */
+                        /* set master role and master stickiness */
-                        server_set_status(handle->master->server, SERVER_MASTER_STICKINESS);
+                        server_clear_set_status(ptr->server,
+                            (SERVER_SLAVE|SERVER_MASTER|SERVER_MASTER_STICKINESS),
+                            (SERVER_MASTER|SERVER_MASTER_STICKINESS));
                     }
                     else
                     {
-                        /* clear master stickiness */
+                        /* set master role and clear master stickiness */
-                        server_clear_status(ptr->server, SERVER_MASTER_STICKINESS);
+                        server_clear_set_status(ptr->server,
+                            (SERVER_SLAVE|SERVER_MASTER|SERVER_MASTER_STICKINESS),
+                            SERVER_MASTER);
                     }
                 }
             }