From 7ca8db14de8a5035fd4956a0392223dd097ab88f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Mon, 2 Oct 2017 14:14:54 +0300
Subject: [PATCH] MXS-1444: Add monitor parameter alteration

The parameter handling for monitors can now be done in a consistent manner
by establishing a rule that the monitor owns the parameter object as long
as it is running. This will allow parameters to be added and removed
safely both from outside and inside monitors.

Currently this functionality is only used by mysqlmon to disable failover
after an attempt to perform a failover has failed.
---
 include/maxscale/monitor.h                   | 15 +++++++
 server/core/monitor.cc                       | 41 ++++++++++++++++++--
 server/modules/monitor/mysqlmon/mysql_mon.cc |  2 +
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/include/maxscale/monitor.h b/include/maxscale/monitor.h
index a354185f8..051f8ce02 100644
--- a/include/maxscale/monitor.h
+++ b/include/maxscale/monitor.h
@@ -273,6 +273,21 @@ const char* mon_get_event_name(mxs_monitor_event_t event);
 void lock_monitor_servers(MXS_MONITOR *monitor);
 void release_monitor_servers(MXS_MONITOR *monitor);
 
+/**
+ * Alter monitor parameters
+ *
+ * The monitor parameters should not be altered while the monitor is
+ * running. To alter a parameter from outside a monitor module, stop the monitor,
+ * do the alteration and then restart the monitor. The monitor "owns" the parameters
+ * as long as it is running so if the monitor needs to change its own parameters,
+ * it can do it without stopping itself.
+ *
+ * @param monitor Monitor whose parameter is altered
+ * @param key     Parameter name to alter
+ * @param value   New value for the parameter
+ */
+void mon_alter_parameter(MXS_MONITOR* monitor, const char* key, const char* value);
+
 /**
  * @brief Handle state change events
  *
diff --git a/server/core/monitor.cc b/server/core/monitor.cc
index d47140c64..8aabaffe7 100644
--- a/server/core/monitor.cc
+++ b/server/core/monitor.cc
@@ -856,6 +856,8 @@ bool check_monitor_permissions(MXS_MONITOR* monitor, const char* query)
  */
 void monitorAddParameters(MXS_MONITOR *monitor, MXS_CONFIG_PARAMETER *params)
 {
+    spinlock_acquire(&monitor->lock);
+
     while (params)
     {
         MXS_CONFIG_PARAMETER* old = config_get_param(monitor->parameters, params->name);
@@ -874,11 +876,16 @@ void monitorAddParameters(MXS_MONITOR *monitor, MXS_CONFIG_PARAMETER *params)
 
         params = params->next;
     }
+
+    spinlock_release(&monitor->lock);
 }
 
 bool monitorRemoveParameter(MXS_MONITOR *monitor, const char *key)
 {
     MXS_CONFIG_PARAMETER *prev = NULL;
+    bool rval = false;
+
+    spinlock_acquire(&monitor->lock);
 
     for (MXS_CONFIG_PARAMETER *p = monitor->parameters; p; p = p->next)
     {
@@ -887,19 +894,41 @@ bool monitorRemoveParameter(MXS_MONITOR *monitor, const char *key)
             if (p == monitor->parameters)
             {
                 monitor->parameters = monitor->parameters->next;
-                p->next = NULL;
             }
             else
             {
                 prev->next = p->next;
-                p->next = NULL;
             }
+
+            p->next = NULL;
             config_parameter_free(p);
-            return true;
+            rval = true;
+            break;
         }
+
         prev = p;
     }
-    return false;
+
+    spinlock_release(&monitor->lock);
+
+    return rval;
+}
+
+void mon_alter_parameter(MXS_MONITOR* monitor, const char* key, const char* value)
+{
+    spinlock_acquire(&monitor->lock);
+
+    for (MXS_CONFIG_PARAMETER* p = monitor->parameters; p; p = p->next)
+    {
+        if (strcmp(p->name, key) == 0)
+        {
+            MXS_FREE(p->value);
+            p->value = MXS_STRDUP_A(value);
+            break;
+        }
+    }
+
+    spinlock_release(&monitor->lock);
 }
 
 /**
@@ -1531,6 +1560,8 @@ static bool create_monitor_config(const MXS_MONITOR *monitor, const char *filena
         return false;
     }
 
+    spinlock_acquire(&monitor->lock);
+
     dprintf(file, "[%s]\n", monitor->name);
     dprintf(file, "%s=monitor\n", CN_TYPE);
     dprintf(file, "%s=%s\n", CN_MODULE, monitor->module_name);
@@ -1585,6 +1616,8 @@ static bool create_monitor_config(const MXS_MONITOR *monitor, const char *filena
         }
     }
 
+    spinlock_release(&monitor->lock);
+
     close(file);
 
     return true;
diff --git a/server/modules/monitor/mysqlmon/mysql_mon.cc b/server/modules/monitor/mysqlmon/mysql_mon.cc
index 8bb697230..5de92b647 100644
--- a/server/modules/monitor/mysqlmon/mysql_mon.cc
+++ b/server/modules/monitor/mysqlmon/mysql_mon.cc
@@ -1878,6 +1878,8 @@ monitorMain(void *arg)
                 MXS_ALERT("Failed to perform failover, disabling failover functionality. "
                           "To enable failover functionality, manually set 'failover' to "
                           "'true' for monitor '%s' via MaxAdmin or the REST API.", mon->name);
+
+                mon_alter_parameter(handle->monitor, CN_FAILOVER, "false");
                 handle->failover = false;
             }
         }