MXS-2271 All monitors inherit from MXS_MONITOR

Most of the API entrypoints are replaced with virtual functions.

include/maxscale/monitor.hh

@@ -24,13 +24,13 @@
 #include <maxbase/semaphore.hh>
 #include <maxbase/worker.hh>
 #include <maxbase/iterator.hh>
-#include <maxbase/jansson.h>
 #include <maxscale/config.hh>
-#include <maxscale/dcb.hh>
 #include <maxscale/server.hh>
 #include <maxscale/protocol/mysql.hh>
 
 class MXS_MONITOR;
+struct DCB;
+struct json_t;
 
 /**
  * An opaque type representing a monitor instance.
@@ -67,19 +67,13 @@ struct MXS_MONITOR_API
     /**
      * @brief Create the monitor.
      *
-     * This entry point is called once when MaxScale is started, for
+     * This entry point is called once when MaxScale is started, for creating the monitor.
-     * creating the monitor.
+     * If the function fails, MaxScale will not start. The returned object must inherit from
+     * the abstract base monitor class and implement the missing methods.
      *
-     * If the function fails, MaxScale will not start. That is, it
+     * @return Monitor object
-     * should fail only for fatal reasons such as not being able to
-     * create vital resources.
-     *
-     * @param monitor  The monitor object.
-     *
-     * @return Pointer to the monitor specific data. Will be stored
-     *         in @c monitor->handle.
      */
-    MXS_MONITOR_INSTANCE* (* createInstance)(MXS_MONITOR* monitor);
+    MXS_MONITOR* (* createInstance)();
 
     /**
      * @brief Destroy the monitor.
@@ -245,14 +239,17 @@ inline mxb::intrusive_slist_iterator<MXS_MONITORED_SERVER> end(MXS_MONITORED_SER
 class MXS_MONITOR
 {
 public:
-    MXS_MONITOR(const std::string& name, const std::string& module, MXS_MONITOR_API* api);
+    MXS_MONITOR();
-    ~MXS_MONITOR();
+    virtual ~MXS_MONITOR();
+    virtual bool configure(const MXS_CONFIG_PARAMETER* params) = 0;
+    virtual bool start(const MXS_CONFIG_PARAMETER* params) = 0;
+    virtual void stop() = 0;
+    virtual void diagnostics(DCB* dcb) const = 0;
+    virtual json_t* diagnostics_json() const = 0;
 
     char*             name;         /**< Monitor instance name */
-    const std::string module_name;  /**< Name of the monitor module */
+    std::string       module_name;  /**< Name of the monitor module */
 
-    MXS_MONITOR_API*      api;      /**< The monitor api */
-    MXS_MONITOR_INSTANCE* instance; /**< Instance returned from startMonitor */
     MXS_MONITOR*          next;     /**< Next monitor in the linked list */
     mutable std::mutex    lock;
 
@@ -289,6 +286,7 @@ public:
     MxsDiskSpaceThreshold* disk_space_threshold = NULL;     /**< Disk space thresholds */
     int64_t                disk_space_check_interval = -1;  /**< How often should a disk space check be made
                                                              *   at most. */
+
 private:
     friend class MonitorManager;
 
@@ -298,7 +296,7 @@ private:
      * @param params Config parameters
      * @return True on success
      */
-    bool configure(const MXS_CONFIG_PARAMETER* params);
+    bool configure_base(const MXS_CONFIG_PARAMETER* params);
 };
 
 /**
@@ -481,6 +479,7 @@ namespace maxscale
 {
 
 class MonitorInstance : public MXS_MONITOR_INSTANCE
+                      , public MXS_MONITOR
                       , protected maxbase::Worker
 {
 public:
@@ -576,7 +575,7 @@ public:
     static int64_t get_time_ms();
 
 protected:
-    MonitorInstance(MXS_MONITOR* pMonitor);
+    MonitorInstance();
 
     /**
      * @brief Should the monitor shut down?
@@ -700,8 +699,7 @@ public:
     MonitorInstanceSimple& operator=(const MonitorInstanceSimple&) = delete;
 
 protected:
-    MonitorInstanceSimple(MXS_MONITOR* pMonitor)
+    MonitorInstanceSimple()
-        : MonitorInstance(pMonitor)
     {
     }
 
@@ -759,10 +757,10 @@ public:
     MonitorApi(const MonitorApi&) = delete;
     MonitorApi& operator=(const MonitorApi&) = delete;
 
-    static MXS_MONITOR_INSTANCE* createInstance(MXS_MONITOR* pMonitor)
+    static MXS_MONITOR* createInstance()
     {
         MonitorInstance* pInstance = NULL;
-        MXS_EXCEPTION_GUARD(pInstance = MonitorInstance::create(pMonitor));
+        MXS_EXCEPTION_GUARD(pInstance = MonitorInstance::create());
         return pInstance;
     }
 

server/core/monitor.cc

@@ -106,24 +106,19 @@ MXS_MONITOR* MonitorManager::create_monitor(const string& name, const string& mo
         return NULL;
     }
 
-    MXS_MONITOR* mon = new(std::nothrow) MXS_MONITOR(name, module, api);
+    MXS_MONITOR* mon = api->createInstance();
     if (!mon)
-    {
-        return NULL;
-    }
-
-    bool error = !mon->configure(params);
-    if (!error)
-    {
-        if ((mon->instance = mon->api->createInstance(mon)) == NULL)
     {
         MXS_ERROR("Unable to create monitor instance for '%s', using module '%s'.",
                   name.c_str(), module.c_str());
-            error = true;
+        return NULL;
-        }
     }
 
-    if (!error)
+    // These initializations are kept outside the constructor to keep it simple.
+    mon->name = MXS_STRDUP_A(name.c_str());
+    mon->module_name = module;
+
+    if (mon->configure_base(params)) // TODO: Move derived class configure() here
     {
         std::lock_guard<std::mutex> guard(monLock);
         mon->next = allMonitors;
@@ -137,16 +132,12 @@ MXS_MONITOR* MonitorManager::create_monitor(const string& name, const string& mo
     return mon;
 }
 
-MXS_MONITOR::MXS_MONITOR(const string& name, const string& module, MXS_MONITOR_API* api)
+MXS_MONITOR::MXS_MONITOR()
-    : module_name(module)
-    , api(api)
 {
-    // The strdup is required until name is an std::string.
-    this->name = MXS_STRDUP_A(name.c_str());
     memset(journal_hash, 0, sizeof(journal_hash));
 }
 
-bool MXS_MONITOR::configure(const MXS_CONFIG_PARAMETER* params)
+bool MXS_MONITOR::configure_base(const MXS_CONFIG_PARAMETER* params)
 {
     read_timeout = config_get_integer(params, CN_BACKEND_READ_TIMEOUT);
     write_timeout = config_get_integer(params, CN_BACKEND_WRITE_TIMEOUT);
@@ -211,8 +202,6 @@ void MonitorManager::destroy_monitor(MXS_MONITOR* mon)
     }
 
     guard.unlock();
-
-    mon->api->destroyInstance(mon->instance);
     delete mon;
 }
 
@@ -257,7 +246,7 @@ void monitor_start(MXS_MONITOR* monitor, const MXS_CONFIG_PARAMETER* params)
                 remove_server_journal(monitor);
             }
 
-            if ((*monitor->api->startMonitor)(monitor->instance, params))
+            if (monitor->start(params))
             {
                 monitor->state = MONITOR_STATE_RUNNING;
             }
@@ -303,7 +292,7 @@ void monitor_stop(MXS_MONITOR* monitor)
         if (monitor->state == MONITOR_STATE_RUNNING)
         {
             monitor->state = MONITOR_STATE_STOPPING;
-            monitor->api->stopMonitor(monitor->instance);
+            monitor->stop();
             monitor->state = MONITOR_STATE_STOPPED;
 
             MXS_MONITORED_SERVER* db = monitor->monitored_servers;
@@ -538,21 +527,14 @@ void monitor_show(DCB* dcb, MXS_MONITOR* monitor)
 
     dcb_printf(dcb, "\n");
 
-    if (monitor->instance)
+    if (monitor->state == MONITOR_STATE_RUNNING)
     {
-        if (monitor->api->diagnostics && monitor->state == MONITOR_STATE_RUNNING)
+        monitor->diagnostics(dcb);
-        {
-            monitor->api->diagnostics(monitor->instance, dcb);
     }
     else
     {
         dcb_printf(dcb, " (no diagnostics)\n");
     }
-    }
-    else
-    {
-        dcb_printf(dcb, " Monitor failed\n");
-    }
     dcb_printf(dcb, "\n");
 }
 
@@ -1760,11 +1742,9 @@ json_t* monitor_json_data(const MXS_MONITOR* monitor, const char* host)
         /** Monitor parameters */
         json_object_set_new(attr, CN_PARAMETERS, monitor_parameters_to_json(monitor));
 
-        if (monitor->instance && monitor->api->diagnostics_json
+        if (monitor->state == MONITOR_STATE_RUNNING)
-            && monitor->state == MONITOR_STATE_RUNNING)
         {
-            json_t* diag = monitor->api->diagnostics_json(monitor->instance);
+            json_t* diag = monitor->diagnostics_json();
-
             if (diag)
             {
                 json_object_set_new(attr, CN_MONITOR_DIAGNOSTICS, diag);
@@ -2449,8 +2429,8 @@ void monitor_debug_wait()
 namespace maxscale
 {
 
-MonitorInstance::MonitorInstance(MXS_MONITOR* pMonitor)
+MonitorInstance::MonitorInstance()
-    : m_monitor(pMonitor)
+    : m_monitor(this)
     , m_master(NULL)
     , m_thread_running(false)
     , m_shutdown(0)

server/modules/monitor/auroramon/auroramon.cc

@@ -25,8 +25,7 @@
 #include <maxscale/mysql_utils.hh>
 
 
-AuroraMonitor::AuroraMonitor(MXS_MONITOR* monitor)
+AuroraMonitor::AuroraMonitor()
-    : maxscale::MonitorInstanceSimple(monitor)
 {
 }
 
@@ -35,9 +34,9 @@ AuroraMonitor::~AuroraMonitor()
 }
 
 // static
-AuroraMonitor* AuroraMonitor::create(MXS_MONITOR* monitor)
+AuroraMonitor* AuroraMonitor::create()
 {
-    return new AuroraMonitor(monitor);
+    return new AuroraMonitor();
 }
 
 bool AuroraMonitor::has_sufficient_permissions() const

server/modules/monitor/auroramon/auroramon.hh

@@ -26,12 +26,12 @@ public:
     AuroraMonitor& operator=(const AuroraMonitor&) = delete;
 
     ~AuroraMonitor();
-    static AuroraMonitor* create(MXS_MONITOR* monitor);
+    static AuroraMonitor* create();
 
 protected:
     bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
 private:
-    AuroraMonitor(MXS_MONITOR* monitor);
+    AuroraMonitor();
 };

server/modules/monitor/clustrixmon/clustrixmonitor.cc

@@ -27,8 +27,7 @@ const int DEFAULT_HEALTH_PORT = 3581;
 
 }
 
-ClustrixMonitor::ClustrixMonitor(MXS_MONITOR* pMonitor)
+ClustrixMonitor::ClustrixMonitor()
-    : maxscale::MonitorInstance(pMonitor)
 {
 }
 
@@ -37,9 +36,9 @@ ClustrixMonitor::~ClustrixMonitor()
 }
 
 //static
-ClustrixMonitor* ClustrixMonitor::create(MXS_MONITOR* pMonitor)
+ClustrixMonitor* ClustrixMonitor::create()
 {
-    return new ClustrixMonitor(pMonitor);
+    return new ClustrixMonitor();
 }
 
 bool ClustrixMonitor::configure(const MXS_CONFIG_PARAMETER* pParams)

server/modules/monitor/clustrixmon/clustrixmonitor.hh

@@ -60,12 +60,12 @@ public:
 
     ~ClustrixMonitor();
 
-    static ClustrixMonitor* create(MXS_MONITOR* pMonitor);
+    static ClustrixMonitor* create();
 
     bool configure(const MXS_CONFIG_PARAMETER* pParams) override;
 
 private:
-    ClustrixMonitor(MXS_MONITOR* pMonitor);
+    ClustrixMonitor();
 
     void pre_loop() override;
     void post_loop() override;

server/modules/monitor/csmon/csmon.cc

@@ -71,9 +71,7 @@ int get_cs_version(MXS_MONITORED_SERVER* srv)
 }
 }
 
-CsMonitor::CsMonitor(MXS_MONITOR* monitor)
+CsMonitor::CsMonitor()
-    : maxscale::MonitorInstanceSimple(monitor)
-    , m_primary(config_get_server(monitor->parameters, "primary"))
 {
 }
 
@@ -82,9 +80,9 @@ CsMonitor::~CsMonitor()
 }
 
 // static
-CsMonitor* CsMonitor::create(MXS_MONITOR* monitor)
+CsMonitor* CsMonitor::create()
 {
-    return new CsMonitor(monitor);
+    return new CsMonitor();
 }
 
 bool CsMonitor::has_sufficient_permissions() const
@@ -116,6 +114,12 @@ void CsMonitor::update_server_status(MXS_MONITORED_SERVER* srv)
     monitor_set_pending_status(srv, status);
 }
 
+bool CsMonitor::configure(const MXS_CONFIG_PARAMETER* pParams)
+{
+    m_primary = config_get_server(pParams, "primary");
+    return true;
+}
+
 extern "C" MXS_MODULE* MXS_CREATE_MODULE()
 {
     static MXS_MODULE info =

server/modules/monitor/csmon/csmon.hh

@@ -22,14 +22,15 @@ public:
     CsMonitor& operator=(const CsMonitor&) = delete;
 
     ~CsMonitor();
-    static CsMonitor* create(MXS_MONITOR* monitor);
+    static CsMonitor* create();
 
 protected:
     bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
 private:
-    CsMonitor(MXS_MONITOR* monitor);
+    CsMonitor();
+    bool configure(const MXS_CONFIG_PARAMETER* pParams) override;
 
     SERVER* m_primary;
 };

server/modules/monitor/galeramon/galeramon.cc

@@ -42,9 +42,8 @@ static int                   compare_node_index(const void*, const void*);
 static int                   compare_node_priority(const void*, const void*);
 static bool                  using_xtrabackup(MXS_MONITORED_SERVER* database, const char* server_string);
 
-GaleraMonitor::GaleraMonitor(MXS_MONITOR* mon)
+GaleraMonitor::GaleraMonitor()
-    : maxscale::MonitorInstanceSimple(mon)
+    : m_disableMasterFailback(0)
-    , m_disableMasterFailback(0)
     , m_availableWhenDonor(0)
     , m_disableMasterRoleSetting(0)
     , m_root_node_as_master(false)
@@ -60,9 +59,9 @@ GaleraMonitor::~GaleraMonitor()
 }
 
 // static
-GaleraMonitor* GaleraMonitor::create(MXS_MONITOR* monitor)
+GaleraMonitor* GaleraMonitor::create()
 {
-    return new GaleraMonitor(monitor);
+    return new GaleraMonitor();
 }
 
 void GaleraMonitor::diagnostics(DCB* dcb) const

server/modules/monitor/galeramon/galeramon.hh

@@ -43,7 +43,7 @@ public:
     GaleraMonitor& operator=(const GaleraMonitor&) = delete;
 
     ~GaleraMonitor();
-    static GaleraMonitor* create(MXS_MONITOR* monitor);
+    static GaleraMonitor* create();
     void                  diagnostics(DCB* dcb) const;
     json_t*               diagnostics_json() const;
 
@@ -68,7 +68,7 @@ private:
     NodeMap     m_info;                         /**< Contains Galera Cluster variables of all nodes */
     int         m_cluster_size;                 /**< How many nodes in the cluster */
 
-    GaleraMonitor(MXS_MONITOR* monitor);
+    GaleraMonitor();
 
     bool detect_cluster_size(const int n_nodes,
                              const char* candidate_uuid,

server/modules/monitor/grmon/grmon.cc

@@ -26,8 +26,7 @@
 #include <mysqld_error.h>
 
 
-GRMon::GRMon(MXS_MONITOR* monitor)
+GRMon::GRMon()
-    : MonitorInstanceSimple(monitor)
 {
 }
 
@@ -35,9 +34,9 @@ GRMon::~GRMon()
 {
 }
 
-GRMon* GRMon::create(MXS_MONITOR* monitor)
+GRMon* GRMon::create()
 {
-    return new GRMon(monitor);
+    return new GRMon();
 }
 
 bool GRMon::has_sufficient_permissions() const

server/modules/monitor/grmon/grmon.hh

@@ -26,12 +26,12 @@ public:
     GRMon& operator&(const GRMon&) = delete;
 
     ~GRMon();
-    static GRMon* create(MXS_MONITOR* monitor);
+    static GRMon* create();
 
 protected:
     bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
 private:
-    GRMon(MXS_MONITOR* monitor);
+    GRMon();
 };

server/modules/monitor/mariadbmon/mariadbmon.cc

@@ -59,8 +59,7 @@ static const char CN_REPLICATION_PASSWORD[] = "replication_password";
 static const char DIAG_ERROR[] = "Internal error, could not print diagnostics. "
                                  "Check log for more information.";
 
-MariaDBMonitor::MariaDBMonitor(MXS_MONITOR* monitor)
+MariaDBMonitor::MariaDBMonitor()
-    : maxscale::MonitorInstance(monitor)
 {
 }
 
@@ -173,9 +172,9 @@ bool MariaDBMonitor::set_replication_credentials(const MXS_CONFIG_PARAMETER* par
     return rval;
 }
 
-MariaDBMonitor* MariaDBMonitor::create(MXS_MONITOR* monitor)
+MariaDBMonitor* MariaDBMonitor::create()
 {
-    return new MariaDBMonitor(monitor);
+    return new MariaDBMonitor();
 }
 
 /**
@@ -845,7 +844,7 @@ bool handle_manual_switchover(const MODULECMD_ARG* args, json_t** error_out)
     else
     {
         MXS_MONITOR* mon = args->argv[0].value.monitor;
-        auto handle = static_cast<MariaDBMonitor*>(mon->instance);
+        auto handle = static_cast<MariaDBMonitor*>(mon);
         SERVER* promotion_server = (args->argc >= 2) ? args->argv[1].value.server : NULL;
         SERVER* demotion_server = (args->argc == 3) ? args->argv[2].value.server : NULL;
         rval = handle->run_manual_switchover(promotion_server, demotion_server, error_out);
@@ -873,7 +872,7 @@ bool handle_manual_failover(const MODULECMD_ARG* args, json_t** output)
     else
     {
         MXS_MONITOR* mon = args->argv[0].value.monitor;
-        auto handle = static_cast<MariaDBMonitor*>(mon->instance);
+        auto handle = static_cast<MariaDBMonitor*>(mon);
         rv = handle->run_manual_failover(output);
     }
     return rv;
@@ -901,7 +900,7 @@ bool handle_manual_rejoin(const MODULECMD_ARG* args, json_t** output)
     {
         MXS_MONITOR* mon = args->argv[0].value.monitor;
         SERVER* server = args->argv[1].value.server;
-        auto handle = static_cast<MariaDBMonitor*>(mon->instance);
+        auto handle = static_cast<MariaDBMonitor*>(mon);
         rv = handle->run_manual_rejoin(server, output);
     }
     return rv;
@@ -923,7 +922,7 @@ bool handle_manual_reset_replication(const MODULECMD_ARG* args, json_t** output)
     {
         MXS_MONITOR* mon = args->argv[0].value.monitor;
         SERVER* server = args->argv[1].value.server;
-        auto handle = static_cast<MariaDBMonitor*>(mon->instance);
+        auto handle = static_cast<MariaDBMonitor*>(mon);
         rv = handle->run_manual_reset_replication(server, output);
     }
     return rv;

server/modules/monitor/mariadbmon/mariadbmon.hh

@@ -51,7 +51,7 @@ public:
      * @param monitor Generic monitor data
      * @return MariaDBMonitor instance
      */
-    static MariaDBMonitor* create(MXS_MONITOR* monitor);
+    static MariaDBMonitor* create();
 
     ~MariaDBMonitor();
 
@@ -236,7 +236,7 @@ private:
                                                  * gtid:s? */
 
     // Base methods
-    MariaDBMonitor(MXS_MONITOR* monitor_base);
+    MariaDBMonitor();
     bool configure(const MXS_CONFIG_PARAMETER* params);
     bool set_replication_credentials(const MXS_CONFIG_PARAMETER* params);
     void reset_server_info();

server/modules/monitor/mariadbmon/test/test_cycle_find.cc

@@ -90,7 +90,7 @@ int main()
 }
 
 MariaDBMonitor::Test::Test(bool use_hostnames)
-    : m_monitor(new MariaDBMonitor(NULL))
+    : m_monitor(new MariaDBMonitor())
     , m_use_hostnames(use_hostnames)
 {
 }

server/modules/monitor/mmmon/mmmon.cc

@@ -34,9 +34,8 @@
 static void detectStaleMaster(void*, int);
 static bool isMySQLEvent(mxs_monitor_event_t event);
 
-MMMonitor::MMMonitor(MXS_MONITOR* monitor)
+MMMonitor::MMMonitor()
-    : maxscale::MonitorInstanceSimple(monitor)
+    : m_detectStaleMaster(false)
-    , m_detectStaleMaster(false)
 {
 }
 
@@ -45,9 +44,9 @@ MMMonitor::~MMMonitor()
 }
 
 // static
-MMMonitor* MMMonitor::create(MXS_MONITOR* monitor)
+MMMonitor* MMMonitor::create()
 {
-    return new MMMonitor(monitor);
+    return new MMMonitor();
 }
 
 void MMMonitor::diagnostics(DCB* dcb) const

server/modules/monitor/mmmon/mmmon.hh

@@ -26,7 +26,7 @@ public:
     MMMonitor& operator=(const MMMonitor&) = delete;
 
     ~MMMonitor();
-    static MMMonitor* create(MXS_MONITOR* monitor);
+    static MMMonitor* create();
     void              diagnostics(DCB* dcb) const;
     json_t*           diagnostics_json() const;
 
@@ -39,7 +39,7 @@ protected:
 private:
     int           m_detectStaleMaster;  /**< Monitor flag for Stale Master detection */
 
-    MMMonitor(MXS_MONITOR* monitor);
+    MMMonitor();
 
     MXS_MONITORED_SERVER* get_current_master();
 };

server/modules/monitor/ndbclustermon/ndbclustermon.cc

@@ -22,8 +22,7 @@
 #include <maxscale/mysql_utils.hh>
 
 
-NDBCMonitor::NDBCMonitor(MXS_MONITOR* monitor)
+NDBCMonitor::NDBCMonitor()
-    : maxscale::MonitorInstanceSimple(monitor)
 {
 }
 
@@ -32,9 +31,9 @@ NDBCMonitor::~NDBCMonitor()
 }
 
 // static
-NDBCMonitor* NDBCMonitor::create(MXS_MONITOR* monitor)
+NDBCMonitor* NDBCMonitor::create()
 {
-    return new NDBCMonitor(monitor);
+    return new NDBCMonitor();
 }
 
 bool NDBCMonitor::has_sufficient_permissions() const

server/modules/monitor/ndbclustermon/ndbclustermon.hh

@@ -26,12 +26,12 @@ public:
     NDBCMonitor& operator=(const NDBCMonitor&) = delete;
 
     ~NDBCMonitor();
-    static NDBCMonitor* create(MXS_MONITOR* monitor);
+    static NDBCMonitor* create();
 
 protected:
     bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
 private:
-    NDBCMonitor(MXS_MONITOR* monitor);
+    NDBCMonitor();
 };