From 7ac31b26743e580364c3e25a357b22f2ddeeca22 Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Wed, 30 Nov 2016 10:37:07 +0200
Subject: [PATCH 01/25] Cache: Now implements destroyInstance

---
 server/modules/filter/cache/cachefilter.cc | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/server/modules/filter/cache/cachefilter.cc b/server/modules/filter/cache/cachefilter.cc
index 984be83be..4fb85c1d5 100644
--- a/server/modules/filter/cache/cachefilter.cc
+++ b/server/modules/filter/cache/cachefilter.cc
@@ -74,6 +74,7 @@ static int      routeQuery(FILTER* pInstance, void* pSessionData, GWBUF* pPacket
 static int      clientReply(FILTER* pInstance, void* pSessionData, GWBUF* pPacket);
 static void     diagnostics(FILTER* pInstance, void* pSessionData, DCB* pDcb);
 static uint64_t getCapabilities(void);
+static void     destroyInstance(FILTER* pInstance);
 
 static bool process_params(char **pzOptions, FILTER_PARAMETER **ppParams, CACHE_CONFIG& config);
 
@@ -121,7 +122,7 @@ extern "C" FILTER_OBJECT *GetModuleObject()
             clientReply,
             diagnostics,
             getCapabilities,
-            NULL, // destroyInstance
+            destroyInstance,
         };
 
     return &object;
@@ -301,7 +302,6 @@ static void diagnostics(FILTER* pInstance, void* pSessionData, DCB* pDcb)
     CPP_GUARD(pSessionCache->diagnostics(pDcb));
 }
 
-
 /**
  * Capability routine.
  *
@@ -312,6 +312,19 @@ static uint64_t getCapabilities(void)
     return RCAP_TYPE_TRANSACTION_TRACKING;
 }
 
+/**
+ * Destroy the filter instance.
+ *
+ * @param pInstance  The filter instance.
+ */
+static void destroyInstance(FILTER* pInstance)
+{
+    MXS_NOTICE("Deleting Cache filter instance.");
+    CACHE_FILTER* pFilter = reinterpret_cast<CACHE_FILTER*>(pInstance);
+
+    delete pFilter;
+}
+
 //
 // API Implementation END
 //

From 24af4b3225753c3dfef3aa584ad5ea46350a8dd7 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Wed, 30 Nov 2016 11:08:07 +0200
Subject: [PATCH 02/25] Clean up hintfilter

The hintfilter no longer needs to process the queries into one buffer as
the client protocol will handle that.
---
 server/modules/filter/hintfilter/hintfilter.c | 51 ++++---------------
 1 file changed, 9 insertions(+), 42 deletions(-)

diff --git a/server/modules/filter/hintfilter/hintfilter.c b/server/modules/filter/hintfilter/hintfilter.c
index fe35db089..9fb5017c0 100644
--- a/server/modules/filter/hintfilter/hintfilter.c
+++ b/server/modules/filter/hintfilter/hintfilter.c
@@ -70,7 +70,7 @@ version()
 }
 
 /**
- * The module initialisation routine, called when the module
+ * The module initialization routine, called when the module
  * is first loaded.
  * @see function load_module in load_utils.c for explanation of lint
  */
@@ -205,7 +205,7 @@ setDownstream(FILTER *instance, void *session, DOWNSTREAM *downstream)
 /**
  * The routeQuery entry point. This is passed the query buffer
  * to which the filter should be applied. Once applied the
- * query shoudl normally be passed to the downstream component
+ * query should normally be passed to the downstream component
  * (filter or router) in the filter chain.
  *
  * @param instance  The filter instance data
@@ -216,51 +216,18 @@ static int
 routeQuery(FILTER *instance, void *session, GWBUF *queue)
 {
     HINT_SESSION *my_session = (HINT_SESSION *)session;
-    char *ptr;
-    int rval, len, residual;
-    HINT *hint;
 
-    if (my_session->request == NULL)
+    if (modutil_is_SQL(queue))
     {
-        /*
-         * No stored buffer, so this must be the first
-         * buffer of a new request.
-         */
-        if (modutil_MySQL_Query(queue, &ptr, &len, &residual) == 0)
-        {
-            return my_session->down.routeQuery(
-                       my_session->down.instance,
-                       my_session->down.session, queue);
-        }
-        my_session->request = queue;
-        my_session->query_len = len;
+        my_session->request = NULL;
+        my_session->query_len = 0;
+        HINT *hint = hint_parser(my_session, queue);
+        queue->hint = hint;
     }
-    else
-    {
-        gwbuf_append(my_session->request, queue);
-    }
-
-    if (gwbuf_length(my_session->request) < my_session->query_len)
-    {
-        /*
-         * We have not got the entire SQL text, buffer and wait for
-         * the remainder.
-         */
-        return 1;
-    }
-    /* We have the entire SQL text, parse for hints and attach to the
-     * buffer at the head of the queue.
-     */
-    queue = my_session->request;
-    my_session->request = NULL;
-    my_session->query_len = 0;
-    hint = hint_parser(my_session, queue);
-    queue->hint = hint;
 
     /* Now process the request */
-    rval = my_session->down.routeQuery(my_session->down.instance,
+    return my_session->down.routeQuery(my_session->down.instance,
                                        my_session->down.session, queue);
-    return rval;
 }
 
 /**
@@ -289,5 +256,5 @@ diagnostic(FILTER *instance, void *fsession, DCB *dcb)
  */
 static uint64_t getCapabilities(void)
 {
-    return RCAP_TYPE_STMT_INPUT;
+    return RCAP_TYPE_CONTIGUOUS_INPUT;
 }

From 4c4bd24a406539b6f58f3beb4bc810b2103f2e92 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Tue, 29 Nov 2016 16:25:45 +0200
Subject: [PATCH 03/25] Allow module specific monitor parameters to be altered

Module specific parameters can now be altered at runtime. This allows both
the removal and addition of arbitrary monitor parameters.
---
 include/maxscale/config.h                  |  2 +-
 include/maxscale/monitor.h                 |  2 ++
 server/core/config.c                       |  4 +--
 server/core/config_runtime.c               | 31 +++++++++++++++++++---
 server/core/monitor.c                      | 29 +++++++++++++++++++-
 server/core/service.c                      |  2 +-
 server/modules/routing/debugcli/debugcmd.c | 12 ++++++---
 7 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/include/maxscale/config.h b/include/maxscale/config.h
index 6f02ec613..afb1a0029 100644
--- a/include/maxscale/config.h
+++ b/include/maxscale/config.h
@@ -238,7 +238,7 @@ bool                config_set_qualified_param(CONFIG_PARAMETER* param,
                                                config_param_type_t type);
 int                 config_threadcount();
 int                 config_truth_value(char *);
-void                free_config_parameter(CONFIG_PARAMETER* p1);
+void                config_parameter_free(CONFIG_PARAMETER* p1);
 bool                is_internal_service(const char *router);
 
 MXS_END_DECLS
diff --git a/include/maxscale/monitor.h b/include/maxscale/monitor.h
index 70dcd7b63..cbaf45917 100644
--- a/include/maxscale/monitor.h
+++ b/include/maxscale/monitor.h
@@ -199,6 +199,7 @@ struct monitor
     char *module_name;            /**< Name of the monitor module */
     void *handle;                 /**< Handle returned from startMonitor */
     size_t interval;              /**< The monitor interval */
+    bool created_online;          /**< Whether this monitor was created at runtime */
     struct monitor *next;         /**< Next monitor in the linked list */
 };
 
@@ -209,6 +210,7 @@ extern void monitorAddServer(MONITOR *mon, SERVER *server);
 extern void monitorRemoveServer(MONITOR *mon, SERVER *server);
 extern void monitorAddUser(MONITOR *, char *, char *);
 extern void monitorAddParameters(MONITOR *monitor, CONFIG_PARAMETER *params);
+extern bool monitorRemoveParameter(MONITOR *monitor, const char *key);
 extern void monitorStop(MONITOR *);
 extern void monitorStart(MONITOR *, void*);
 extern void monitorStopAll();
diff --git a/server/core/config.c b/server/core/config.c
index 51e1a82ec..70ae9c77e 100644
--- a/server/core/config.c
+++ b/server/core/config.c
@@ -1070,7 +1070,7 @@ return_p2:
  * Free a configuration parameter
  * @param p1 Parameter to free
  */
-void free_config_parameter(CONFIG_PARAMETER* p1)
+void config_parameter_free(CONFIG_PARAMETER* p1)
 {
     while (p1)
     {
@@ -1089,7 +1089,7 @@ void config_context_free(CONFIG_CONTEXT *context)
     while (context)
     {
         obj = context->next;
-        free_config_parameter(context->parameters);
+        config_parameter_free(context->parameters);
         MXS_FREE(context->object);
         MXS_FREE(context);
         context = obj;
diff --git a/server/core/config_runtime.c b/server/core/config_runtime.c
index a5f5dcb8f..c5900990a 100644
--- a/server/core/config_runtime.c
+++ b/server/core/config_runtime.c
@@ -361,8 +361,27 @@ bool runtime_alter_monitor(MONITOR *monitor, char *key, char *value)
             monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, ival);
         }
     }
+    else
+    {
+        /** We're modifying module specific parameters and we need to stop the monitor */
+        monitorStop(monitor);
 
-    if (valid)
+        if (monitorRemoveParameter(monitor, key) || value[0])
+        {
+            /** Either we're removing an existing parameter or adding a new one */
+            valid = true;
+
+            if (value[0])
+            {
+                CONFIG_PARAMETER p = {.name = key, .value = value};
+                monitorAddParameters(monitor, &p);
+            }
+        }
+
+        monitorStart(monitor, monitor->parameters);
+    }
+
+    if (valid && monitor->created_online)
     {
         monitor_serialize(monitor);
     }
@@ -497,9 +516,15 @@ bool runtime_create_monitor(const char *name, const char *module)
     bool rval = false;
     MONITOR *monitor = monitor_alloc((char*)name, (char*)module);
 
-    if (monitor && monitor_serialize(monitor))
+    if (monitor)
     {
-        rval = true;
+        /** Mark that this monitor was created after MaxScale was started */
+        monitor->created_online = true;
+
+        if (monitor_serialize(monitor))
+        {
+            rval = true;
+        }
     }
 
     spinlock_release(&crt_lock);
diff --git a/server/core/monitor.c b/server/core/monitor.c
index 9542b912b..cb02f5deb 100644
--- a/server/core/monitor.c
+++ b/server/core/monitor.c
@@ -104,6 +104,7 @@ monitor_alloc(char *name, char *module)
     mon->connect_timeout = DEFAULT_CONNECT_TIMEOUT;
     mon->interval = MONITOR_INTERVAL;
     mon->parameters = NULL;
+    mon->created_online = false;
     spinlock_init(&mon->lock);
     spinlock_acquire(&monLock);
     mon->next = allMonitors;
@@ -144,7 +145,7 @@ monitor_free(MONITOR *mon)
         }
     }
     spinlock_release(&monLock);
-    free_config_parameter(mon->parameters);
+    config_parameter_free(mon->parameters);
     monitor_server_free_all(mon->databases);
     MXS_FREE(mon->name);
     MXS_FREE(mon->module_name);
@@ -767,6 +768,32 @@ void monitorAddParameters(MONITOR *monitor, CONFIG_PARAMETER *params)
     }
 }
 
+bool monitorRemoveParameter(MONITOR *monitor, const char *key)
+{
+    CONFIG_PARAMETER *prev = NULL;
+
+    for (CONFIG_PARAMETER *p = monitor->parameters; p; p = p->next)
+    {
+        if (strcmp(p->name, key) == 0)
+        {
+            if (p == monitor->parameters)
+            {
+                monitor->parameters = monitor->parameters->next;
+                p->next = NULL;
+            }
+            else
+            {
+                prev->next = p->next;
+                p->next = NULL;
+            }
+            config_parameter_free(p);
+            return true;
+        }
+        prev = p;
+    }
+    return false;
+}
+
 /**
  * Set a pending status bit in the monitor server
  *
diff --git a/server/core/service.c b/server/core/service.c
index 0c7165e64..310fd712f 100644
--- a/server/core/service.c
+++ b/server/core/service.c
@@ -665,7 +665,7 @@ void service_free(SERVICE *service)
     MXS_FREE(service->credentials.name);
     MXS_FREE(service->credentials.authdata);
 
-    free_config_parameter(service->svc_config_param);
+    config_parameter_free(service->svc_config_param);
     serviceClearRouterOptions(service);
 
     MXS_FREE(service);
diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c
index cad322c67..818756553 100644
--- a/server/modules/routing/debugcli/debugcmd.c
+++ b/server/modules/routing/debugcli/debugcmd.c
@@ -1290,6 +1290,13 @@ static void alterMonitor(DCB *dcb, MONITOR *monitor, char *v1, char *v2, char *v
             {
                 dcb_printf(dcb, "Error: Bad key-value parameter: %s=%s\n", key, value);
             }
+            else if (!monitor->created_online)
+            {
+                dcb_printf(dcb, "Warning: Altered monitor '%s' which is in the "
+                           "main\nconfiguration file. These changes will not be "
+                           "persisted and need\nto be manually added or set again"
+                           "after a restart.\n", monitor->name);
+            }
         }
         else
         {
@@ -1316,9 +1323,8 @@ struct subcommand alteroptions[] =
         "monitor", 2, 12, alterMonitor,
         "Alter monitor parameters",
         "Usage: alter monitor NAME KEY=VALUE ...\n"
-        "This will alter an existing parameter of a monitor. The accepted values\n"
-        "for KEY are: 'user', 'password', 'monitor_interval',\n"
-        "'backend_connect_timeout', 'backend_write_timeout', 'backend_read_timeout'\n"
+        "This will alter an existing parameter of a monitor. To remove parameters,\n"
+        "pass an empty value for a key e.g. 'maxadmin alter monitor my-monitor my-key='\n"
         "A maximum of 11 parameters can be changed at one time",
         {ARG_TYPE_MONITOR, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,
             ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,

From 55f1bbfce6263e6ff37f287d141716eba9544c5f Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Tue, 29 Nov 2016 16:48:16 +0200
Subject: [PATCH 04/25] Fix monitor alteration and serialization

When the monitor credentials were being written with snprintf, the source
and destination overlapped.

The serialization didn't add a 'type=monitor' line into the configuration.
---
 server/core/monitor.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/server/core/monitor.c b/server/core/monitor.c
index cb02f5deb..0c04268d6 100644
--- a/server/core/monitor.c
+++ b/server/core/monitor.c
@@ -397,8 +397,16 @@ void monitorRemoveServer(MONITOR *mon, SERVER *server)
 void
 monitorAddUser(MONITOR *mon, char *user, char *passwd)
 {
-    snprintf(mon->user, sizeof(mon->user), "%s", user);
-    snprintf(mon->password, sizeof(mon->password), "%s", passwd);
+    /** If a pointer to mon->password or mon->user is passed as one of the
+     * parameters the destination and source would overlap. Copy the values to
+     * a local buffer to avoid this. */
+    char pwd[strlen(passwd) + 1];
+    char usr[strlen(user) + 1];
+    strcpy(usr, user);
+    strcpy(pwd, passwd);
+
+    snprintf(mon->user, sizeof(mon->user), "%s", usr);
+    snprintf(mon->password, sizeof(mon->password), "%s", pwd);
 }
 
 /**
@@ -1350,6 +1358,7 @@ static bool create_monitor_config(const MONITOR *monitor, const char *filename)
      * TODO: Check for return values on all of the dprintf calls
      */
     dprintf(file, "[%s]\n", monitor->name);
+    dprintf(file, "type=monitor\n");
     dprintf(file, "module=%s\n", monitor->module_name);
     dprintf(file, "user=%s\n", monitor->user);
     dprintf(file, "password=%s\n", monitor->password);

From b3e31a3da2890707445c9f7281d49fa676abb066 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Tue, 29 Nov 2016 16:49:45 +0200
Subject: [PATCH 05/25] Add creation and destruction of monitors to maxadmin

Maxadmin can now create and destroy monitors. The created monitors are not
started as they would be useless without added servers and configuration
parameters.
---
 server/core/monitor.c                      | 17 ++++-----
 server/modules/routing/debugcli/debugcmd.c | 44 ++++++++++++++++++++++
 2 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/server/core/monitor.c b/server/core/monitor.c
index 0c04268d6..e451b5cd3 100644
--- a/server/core/monitor.c
+++ b/server/core/monitor.c
@@ -397,16 +397,15 @@ void monitorRemoveServer(MONITOR *mon, SERVER *server)
 void
 monitorAddUser(MONITOR *mon, char *user, char *passwd)
 {
-    /** If a pointer to mon->password or mon->user is passed as one of the
-     * parameters the destination and source would overlap. Copy the values to
-     * a local buffer to avoid this. */
-    char pwd[strlen(passwd) + 1];
-    char usr[strlen(user) + 1];
-    strcpy(usr, user);
-    strcpy(pwd, passwd);
+    if (user != mon->user)
+    {
+        snprintf(mon->user, sizeof(mon->user), "%s", user);
+    }
 
-    snprintf(mon->user, sizeof(mon->user), "%s", usr);
-    snprintf(mon->password, sizeof(mon->password), "%s", pwd);
+    if (passwd != mon->password)
+    {
+        snprintf(mon->password, sizeof(mon->password), "%s", passwd);
+    }
 }
 
 /**
diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c
index 818756553..e95636e88 100644
--- a/server/modules/routing/debugcli/debugcmd.c
+++ b/server/modules/routing/debugcli/debugcmd.c
@@ -1081,6 +1081,18 @@ static void createListener(DCB *dcb, SERVICE *service, char *name, char *address
     }
 }
 
+static void createMonitor(DCB *dcb, const char *name, const char *module)
+{
+    if (runtime_create_monitor(name, module))
+    {
+        dcb_printf(dcb, "Created monitor '%s'\n", name);
+    }
+    else
+    {
+        dcb_printf(dcb, "Failed to create monitor '%s', see log for more details\n", name);
+    }
+}
+
 struct subcommand createoptions[] =
 {
     {
@@ -1127,6 +1139,16 @@ struct subcommand createoptions[] =
             ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,
         }
     },
+    {
+        "monitor", 2, 2, createMonitor,
+        "Create a new monitor",
+        "Usage: create monitor NAME MODULE\n"
+        "NAME    Monitor name\n"
+        "MODULE  Monitor module\n",
+        {
+            ARG_TYPE_STRING, ARG_TYPE_STRING
+        }
+    },
     {
         EMPTY_OPTION
     }
@@ -1162,6 +1184,22 @@ static void destroyListener(DCB *dcb, SERVICE *service, const char *name)
     }
 }
 
+
+static void destroyMonitor(DCB *dcb, MONITOR *monitor)
+{
+    char name[strlen(monitor->name) + 1];
+    strcpy(name, monitor->name);
+
+    if (runtime_destroy_monitor(monitor))
+    {
+        dcb_printf(dcb, "Destroyed monitor '%s'\n", name);
+    }
+    else
+    {
+        dcb_printf(dcb, "Failed to destroy monitor '%s', see log file for more details\n", name);
+    }
+}
+
 struct subcommand destroyoptions[] =
 {
     {
@@ -1176,6 +1214,12 @@ struct subcommand destroyoptions[] =
         "Usage: destroy listener SERVICE NAME",
         {ARG_TYPE_SERVICE, ARG_TYPE_STRING}
     },
+    {
+        "monitor", 1, 1, destroyMonitor,
+        "Destroy a monitor",
+        "Usage: destroy monitor NAME",
+        {ARG_TYPE_MONITOR}
+    },
     {
         EMPTY_OPTION
     }

From a6df8754952cec61b55be50a6fd5c2987d0d5dde Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Wed, 30 Nov 2016 15:19:14 +0200
Subject: [PATCH 06/25] Make filter const correct and use snake case

CamelCase is still in use with the printing functions.
---
 include/maxscale/filter.h       | 16 ++++++-------
 server/core/config.c            |  4 ++--
 server/core/filter.c            | 40 ++++++++++++++++-----------------
 server/core/session.c           | 12 +++++-----
 server/core/test/testfilter.c   | 12 +++++-----
 server/modules/filter/tee/tee.c |  3 +--
 6 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/include/maxscale/filter.h b/include/maxscale/filter.h
index 27a1ef8f6..3b25b4483 100644
--- a/include/maxscale/filter.h
+++ b/include/maxscale/filter.h
@@ -109,17 +109,17 @@ typedef struct filter_def
     struct filter_def *next;       /**< Next filter in the chain of all filters */
 } FILTER_DEF;
 
-FILTER_DEF *filter_alloc(char *, char *);
+FILTER_DEF *filter_alloc(const char *, const char *);
 void filter_free(FILTER_DEF *);
 bool filter_load(FILTER_DEF* filter);
-FILTER_DEF *filter_find(char *);
-void filterAddOption(FILTER_DEF *, char *);
-void filterAddParameter(FILTER_DEF *, char *, char *);
-DOWNSTREAM *filterApply(FILTER_DEF *, SESSION *, DOWNSTREAM *);
-UPSTREAM *filterUpstream(FILTER_DEF *, void *, UPSTREAM *);
-int filter_standard_parameter(char *);
+FILTER_DEF *filter_find(const char *);
+void filter_add_option(FILTER_DEF *, const char *);
+void filter_add_parameter(FILTER_DEF *, const char *, const char *);
+DOWNSTREAM *filter_apply(FILTER_DEF *, SESSION *, DOWNSTREAM *);
+UPSTREAM *filter_upstream(FILTER_DEF *, void *, UPSTREAM *);
+int filter_standard_parameter(const char *);
 void dprintAllFilters(DCB *);
-void dprintFilter(DCB *, FILTER_DEF *);
+void dprintFilter(DCB *, const FILTER_DEF *);
 void dListFilters(DCB *);
 
 /**
diff --git a/server/core/config.c b/server/core/config.c
index 70ae9c77e..2e9009889 100644
--- a/server/core/config.c
+++ b/server/core/config.c
@@ -3183,7 +3183,7 @@ int create_new_filter(CONFIG_CONTEXT *obj)
                 char *s = strtok_r(options, ",", &lasts);
                 while (s)
                 {
-                    filterAddOption(obj->element, s);
+                    filter_add_option(obj->element, s);
                     s = strtok_r(NULL, ",", &lasts);
                 }
             }
@@ -3193,7 +3193,7 @@ int create_new_filter(CONFIG_CONTEXT *obj)
             {
                 if (strcmp(params->name, "module") && strcmp(params->name, "options"))
                 {
-                    filterAddParameter(obj->element, params->name, params->value);
+                    filter_add_parameter(obj->element, params->name, params->value);
                 }
                 params = params->next;
             }
diff --git a/server/core/filter.c b/server/core/filter.c
index c3f8812b1..9536d4c2c 100644
--- a/server/core/filter.c
+++ b/server/core/filter.c
@@ -48,22 +48,22 @@ static void filter_free_parameters(FILTER_DEF *filter);
  * @return              The newly created filter or NULL if an error occured
  */
 FILTER_DEF *
-filter_alloc(char *name, char *module)
+filter_alloc(const char *name, const char *module)
 {
-    name = MXS_STRDUP(name);
-    module = MXS_STRDUP(module);
+    char* my_name = MXS_STRDUP(name);
+    char* my_module = MXS_STRDUP(module);
 
     FILTER_DEF *filter = (FILTER_DEF *)MXS_MALLOC(sizeof(FILTER_DEF));
 
-    if (!name || !module || !filter)
+    if (!my_name || !my_module || !filter)
     {
-        MXS_FREE(name);
-        MXS_FREE(module);
+        MXS_FREE(my_name);
+        MXS_FREE(my_module);
         MXS_FREE(filter);
         return NULL;
     }
-    filter->name = name;
-    filter->module = module;
+    filter->name = my_name;
+    filter->module = my_module;
     filter->filter = NULL;
     filter->options = NULL;
     filter->obj = NULL;
@@ -140,7 +140,7 @@ filter_free(FILTER_DEF *filter)
  * @return      The server or NULL if not found
  */
 FILTER_DEF *
-filter_find(char *name)
+filter_find(const char *name)
 {
     FILTER_DEF *filter;
 
@@ -164,7 +164,7 @@ filter_find(char *name)
  * @param name  Parameter name to check
  */
 int
-filter_standard_parameter(char *name)
+filter_standard_parameter(const char *name)
 {
     if (strcmp(name, "type") == 0 || strcmp(name, "module") == 0)
     {
@@ -220,7 +220,7 @@ dprintAllFilters(DCB *dcb)
  * to display all active filters in MaxScale
  */
 void
-dprintFilter(DCB *dcb, FILTER_DEF *filter)
+dprintFilter(DCB *dcb, const FILTER_DEF *filter)
 {
     int i;
 
@@ -287,7 +287,7 @@ dListFilters(DCB *dcb)
  * @param option        The option string
  */
 void
-filterAddOption(FILTER_DEF *filter, char *option)
+filter_add_option(FILTER_DEF *filter, const char *option)
 {
     int i;
 
@@ -323,7 +323,7 @@ filterAddOption(FILTER_DEF *filter, char *option)
  * @param value         The parameter value
  */
 void
-filterAddParameter(FILTER_DEF *filter, char *name, char *value)
+filter_add_parameter(FILTER_DEF *filter, const char *name, const char *value)
 {
     int i;
 
@@ -344,13 +344,13 @@ filterAddParameter(FILTER_DEF *filter, char *name, char *value)
                                                       (i + 2) * sizeof(FILTER_PARAMETER *));
     }
     FILTER_PARAMETER *parameter = MXS_CALLOC(1, sizeof(FILTER_PARAMETER));
-    name = MXS_STRDUP(name);
-    value = MXS_STRDUP(value);
+    char* my_name = MXS_STRDUP(name);
+    char* my_value = MXS_STRDUP(value);
 
-    MXS_ABORT_IF_TRUE(!parameters || !parameter || !name || !value);
+    MXS_ABORT_IF_TRUE(!parameters || !parameter || !my_name || !my_value);
 
-    parameter->name = name;
-    parameter->value = value;
+    parameter->name = my_name;
+    parameter->value = my_value;
     parameters[i] = parameter;
     parameters[i + 1] = NULL;
     filter->parameters = parameters;
@@ -433,7 +433,7 @@ bool filter_load(FILTER_DEF* filter)
  *                      if the filter could not be created
  */
 DOWNSTREAM *
-filterApply(FILTER_DEF *filter, SESSION *session, DOWNSTREAM *downstream)
+filter_apply(FILTER_DEF *filter, SESSION *session, DOWNSTREAM *downstream)
 {
     DOWNSTREAM *me;
 
@@ -468,7 +468,7 @@ filterApply(FILTER_DEF *filter, SESSION *session, DOWNSTREAM *downstream)
  * @return              The upstream component for the next filter
  */
 UPSTREAM *
-filterUpstream(FILTER_DEF *filter, void *fsession, UPSTREAM *upstream)
+filter_upstream(FILTER_DEF *filter, void *fsession, UPSTREAM *upstream)
 {
     UPSTREAM *me = NULL;
 
diff --git a/server/core/session.c b/server/core/session.c
index 7648fe45e..9b155c982 100644
--- a/server/core/session.c
+++ b/server/core/session.c
@@ -661,8 +661,8 @@ session_setup_filters(SESSION *session)
             MXS_ERROR("Service '%s' contians an unresolved filter.", service->name);
             return 0;
         }
-        if ((head = filterApply(service->filters[i], session,
-                                &session->head)) == NULL)
+        if ((head = filter_apply(service->filters[i], session,
+                                 &session->head)) == NULL)
         {
             MXS_ERROR("Failed to create filter '%s' for "
                       "service '%s'.\n",
@@ -679,9 +679,9 @@ session_setup_filters(SESSION *session)
 
     for (i = 0; i < service->n_filters; i++)
     {
-        if ((tail = filterUpstream(service->filters[i],
-                                   session->filters[i].session,
-                                   &session->tail)) == NULL)
+        if ((tail = filter_upstream(service->filters[i],
+                                    session->filters[i].session,
+                                    &session->tail)) == NULL)
         {
             MXS_ERROR("Failed to create filter '%s' for service '%s'.",
                       service->filters[i]->name,
@@ -690,7 +690,7 @@ session_setup_filters(SESSION *session)
         }
 
         /*
-         * filterUpstream may simply return the 3 parameter if
+         * filter_upstream may simply return the 3 parameter if
          * the filter has no upstream entry point. So no need
          * to copy the contents or free tail in this case.
          */
diff --git a/server/core/test/testfilter.c b/server/core/test/testfilter.c
index 377f5a7af..2fb88b68a 100644
--- a/server/core/test/testfilter.c
+++ b/server/core/test/testfilter.c
@@ -83,12 +83,12 @@ test2()
         fprintf(stderr, "filter_alloc: test 1 failed.\n");
         return 1;
     }
-    filterAddOption(f1, "option1");
-    filterAddOption(f1, "option2");
-    filterAddOption(f1, "option3");
-    filterAddParameter(f1, "name1", "value1");
-    filterAddParameter(f1, "name2", "value2");
-    filterAddParameter(f1, "name3", "value3");
+    filter_add_option(f1, "option1");
+    filter_add_option(f1, "option2");
+    filter_add_option(f1, "option3");
+    filter_add_parameter(f1, "name1", "value1");
+    filter_add_parameter(f1, "name2", "value2");
+    filter_add_parameter(f1, "name3", "value3");
     return 0;
 }
 
diff --git a/server/modules/filter/tee/tee.c b/server/modules/filter/tee/tee.c
index ab91bfd3a..26a15e130 100644
--- a/server/modules/filter/tee/tee.c
+++ b/server/modules/filter/tee/tee.c
@@ -596,8 +596,7 @@ newSession(FILTER *instance, SESSION *session)
             my_session->branch_dcb = dcb;
             my_session->dummy_filterdef = dummy;
 
-            if ((dummy_upstream = filterUpstream(
-                                      dummy, my_session, &ses->tail)) == NULL)
+            if ((dummy_upstream = filter_upstream(dummy, my_session, &ses->tail)) == NULL)
             {
                 filter_free(dummy);
                 closeSession(instance, (void*) my_session);

From aef6c7b09950a7b91074032fb2286a64e051239e Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Thu, 1 Dec 2016 15:23:42 +0200
Subject: [PATCH 07/25] Make directory name equal to the library name

* avro -> avrorouter
* binlog -> binlogrouter
---
 server/modules/routing/CMakeLists.txt                         | 4 ++--
 server/modules/routing/{avro => avrorouter}/CMakeLists.txt    | 0
 server/modules/routing/{avro => avrorouter}/avro.c            | 0
 server/modules/routing/{avro => avrorouter}/avro_client.c     | 0
 server/modules/routing/{avro => avrorouter}/avro_file.c       | 0
 server/modules/routing/{avro => avrorouter}/avro_index.c      | 0
 server/modules/routing/{avro => avrorouter}/avro_rbr.c        | 0
 server/modules/routing/{avro => avrorouter}/avro_schema.c     | 0
 server/modules/routing/{avro => avrorouter}/avrorouter.h      | 0
 .../modules/routing/{binlog => binlogrouter}/CMakeLists.txt   | 0
 server/modules/routing/{binlog => binlogrouter}/README        | 0
 server/modules/routing/{binlog => binlogrouter}/STATUS        | 0
 .../modules/routing/{binlog => binlogrouter}/binlog_common.c  | 0
 server/modules/routing/{binlog => binlogrouter}/blr.c         | 0
 server/modules/routing/{binlog => binlogrouter}/blr.h         | 0
 server/modules/routing/{binlog => binlogrouter}/blr_cache.c   | 0
 server/modules/routing/{binlog => binlogrouter}/blr_file.c    | 0
 server/modules/routing/{binlog => binlogrouter}/blr_master.c  | 0
 server/modules/routing/{binlog => binlogrouter}/blr_slave.c   | 0
 .../modules/routing/{binlog => binlogrouter}/maxbinlogcheck.c | 0
 .../routing/{binlog => binlogrouter}/test/CMakeLists.txt      | 0
 .../routing/{binlog => binlogrouter}/test/testbinlog.c        | 0
 22 files changed, 2 insertions(+), 2 deletions(-)
 rename server/modules/routing/{avro => avrorouter}/CMakeLists.txt (100%)
 rename server/modules/routing/{avro => avrorouter}/avro.c (100%)
 rename server/modules/routing/{avro => avrorouter}/avro_client.c (100%)
 rename server/modules/routing/{avro => avrorouter}/avro_file.c (100%)
 rename server/modules/routing/{avro => avrorouter}/avro_index.c (100%)
 rename server/modules/routing/{avro => avrorouter}/avro_rbr.c (100%)
 rename server/modules/routing/{avro => avrorouter}/avro_schema.c (100%)
 rename server/modules/routing/{avro => avrorouter}/avrorouter.h (100%)
 rename server/modules/routing/{binlog => binlogrouter}/CMakeLists.txt (100%)
 rename server/modules/routing/{binlog => binlogrouter}/README (100%)
 rename server/modules/routing/{binlog => binlogrouter}/STATUS (100%)
 rename server/modules/routing/{binlog => binlogrouter}/binlog_common.c (100%)
 rename server/modules/routing/{binlog => binlogrouter}/blr.c (100%)
 rename server/modules/routing/{binlog => binlogrouter}/blr.h (100%)
 rename server/modules/routing/{binlog => binlogrouter}/blr_cache.c (100%)
 rename server/modules/routing/{binlog => binlogrouter}/blr_file.c (100%)
 rename server/modules/routing/{binlog => binlogrouter}/blr_master.c (100%)
 rename server/modules/routing/{binlog => binlogrouter}/blr_slave.c (100%)
 rename server/modules/routing/{binlog => binlogrouter}/maxbinlogcheck.c (100%)
 rename server/modules/routing/{binlog => binlogrouter}/test/CMakeLists.txt (100%)
 rename server/modules/routing/{binlog => binlogrouter}/test/testbinlog.c (100%)

diff --git a/server/modules/routing/CMakeLists.txt b/server/modules/routing/CMakeLists.txt
index 6fee3cb20..5f989da22 100644
--- a/server/modules/routing/CMakeLists.txt
+++ b/server/modules/routing/CMakeLists.txt
@@ -1,8 +1,8 @@
 if(BUILD_AVRO)
-  add_subdirectory(avro)
+  add_subdirectory(avrorouter)
 endif()
 if(BUILD_BINLOG)
-  add_subdirectory(binlog)
+  add_subdirectory(binlogrouter)
 endif()
 
 add_subdirectory(cli)
diff --git a/server/modules/routing/avro/CMakeLists.txt b/server/modules/routing/avrorouter/CMakeLists.txt
similarity index 100%
rename from server/modules/routing/avro/CMakeLists.txt
rename to server/modules/routing/avrorouter/CMakeLists.txt
diff --git a/server/modules/routing/avro/avro.c b/server/modules/routing/avrorouter/avro.c
similarity index 100%
rename from server/modules/routing/avro/avro.c
rename to server/modules/routing/avrorouter/avro.c
diff --git a/server/modules/routing/avro/avro_client.c b/server/modules/routing/avrorouter/avro_client.c
similarity index 100%
rename from server/modules/routing/avro/avro_client.c
rename to server/modules/routing/avrorouter/avro_client.c
diff --git a/server/modules/routing/avro/avro_file.c b/server/modules/routing/avrorouter/avro_file.c
similarity index 100%
rename from server/modules/routing/avro/avro_file.c
rename to server/modules/routing/avrorouter/avro_file.c
diff --git a/server/modules/routing/avro/avro_index.c b/server/modules/routing/avrorouter/avro_index.c
similarity index 100%
rename from server/modules/routing/avro/avro_index.c
rename to server/modules/routing/avrorouter/avro_index.c
diff --git a/server/modules/routing/avro/avro_rbr.c b/server/modules/routing/avrorouter/avro_rbr.c
similarity index 100%
rename from server/modules/routing/avro/avro_rbr.c
rename to server/modules/routing/avrorouter/avro_rbr.c
diff --git a/server/modules/routing/avro/avro_schema.c b/server/modules/routing/avrorouter/avro_schema.c
similarity index 100%
rename from server/modules/routing/avro/avro_schema.c
rename to server/modules/routing/avrorouter/avro_schema.c
diff --git a/server/modules/routing/avro/avrorouter.h b/server/modules/routing/avrorouter/avrorouter.h
similarity index 100%
rename from server/modules/routing/avro/avrorouter.h
rename to server/modules/routing/avrorouter/avrorouter.h
diff --git a/server/modules/routing/binlog/CMakeLists.txt b/server/modules/routing/binlogrouter/CMakeLists.txt
similarity index 100%
rename from server/modules/routing/binlog/CMakeLists.txt
rename to server/modules/routing/binlogrouter/CMakeLists.txt
diff --git a/server/modules/routing/binlog/README b/server/modules/routing/binlogrouter/README
similarity index 100%
rename from server/modules/routing/binlog/README
rename to server/modules/routing/binlogrouter/README
diff --git a/server/modules/routing/binlog/STATUS b/server/modules/routing/binlogrouter/STATUS
similarity index 100%
rename from server/modules/routing/binlog/STATUS
rename to server/modules/routing/binlogrouter/STATUS
diff --git a/server/modules/routing/binlog/binlog_common.c b/server/modules/routing/binlogrouter/binlog_common.c
similarity index 100%
rename from server/modules/routing/binlog/binlog_common.c
rename to server/modules/routing/binlogrouter/binlog_common.c
diff --git a/server/modules/routing/binlog/blr.c b/server/modules/routing/binlogrouter/blr.c
similarity index 100%
rename from server/modules/routing/binlog/blr.c
rename to server/modules/routing/binlogrouter/blr.c
diff --git a/server/modules/routing/binlog/blr.h b/server/modules/routing/binlogrouter/blr.h
similarity index 100%
rename from server/modules/routing/binlog/blr.h
rename to server/modules/routing/binlogrouter/blr.h
diff --git a/server/modules/routing/binlog/blr_cache.c b/server/modules/routing/binlogrouter/blr_cache.c
similarity index 100%
rename from server/modules/routing/binlog/blr_cache.c
rename to server/modules/routing/binlogrouter/blr_cache.c
diff --git a/server/modules/routing/binlog/blr_file.c b/server/modules/routing/binlogrouter/blr_file.c
similarity index 100%
rename from server/modules/routing/binlog/blr_file.c
rename to server/modules/routing/binlogrouter/blr_file.c
diff --git a/server/modules/routing/binlog/blr_master.c b/server/modules/routing/binlogrouter/blr_master.c
similarity index 100%
rename from server/modules/routing/binlog/blr_master.c
rename to server/modules/routing/binlogrouter/blr_master.c
diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlogrouter/blr_slave.c
similarity index 100%
rename from server/modules/routing/binlog/blr_slave.c
rename to server/modules/routing/binlogrouter/blr_slave.c
diff --git a/server/modules/routing/binlog/maxbinlogcheck.c b/server/modules/routing/binlogrouter/maxbinlogcheck.c
similarity index 100%
rename from server/modules/routing/binlog/maxbinlogcheck.c
rename to server/modules/routing/binlogrouter/maxbinlogcheck.c
diff --git a/server/modules/routing/binlog/test/CMakeLists.txt b/server/modules/routing/binlogrouter/test/CMakeLists.txt
similarity index 100%
rename from server/modules/routing/binlog/test/CMakeLists.txt
rename to server/modules/routing/binlogrouter/test/CMakeLists.txt
diff --git a/server/modules/routing/binlog/test/testbinlog.c b/server/modules/routing/binlogrouter/test/testbinlog.c
similarity index 100%
rename from server/modules/routing/binlog/test/testbinlog.c
rename to server/modules/routing/binlogrouter/test/testbinlog.c

From 1707684992447ef7727d8bc54a6b2972f01ac584 Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Thu, 1 Dec 2016 15:41:15 +0200
Subject: [PATCH 08/25] Move all authenticators into separate subdirectories

---
 .../authenticator/CDCPlainAuth/CMakeLists.txt |  6 ++
 .../{ => CDCPlainAuth}/cdc_plain_auth.c       |  0
 server/modules/authenticator/CMakeLists.txt   | 58 +++----------------
 .../authenticator/GSSAPI/CMakeLists.txt       | 11 ++++
 .../GSSAPI/GSSAPIAuth/CMakeLists.txt          |  4 ++
 .../{ => GSSAPI/GSSAPIAuth}/gssapi_auth.c     |  2 +-
 .../GSSAPI/GSSAPIBackendAuth/CMakeLists.txt   |  4 ++
 .../GSSAPIBackendAuth}/gssapi_backend_auth.c  |  2 +-
 .../authenticator/{ => GSSAPI}/gssapi_auth.h  |  0
 .../{ => GSSAPI}/gssapi_auth_common.c         |  0
 .../authenticator/HTTPAuth/CMakeLists.txt     |  4 ++
 .../authenticator/{ => HTTPAuth}/http_auth.c  |  0
 .../authenticator/MaxAdminAuth/CMakeLists.txt |  4 ++
 .../{ => MaxAdminAuth}/max_admin_auth.c       |  0
 .../MySQLBackendAuth/CMakeLists.txt           |  4 ++
 .../mysql_backend_auth.c                      |  0
 .../NullAuthAllow/CMakeLists.txt              |  4 ++
 .../{ => NullAuthAllow}/null_auth_allow.c     |  0
 .../authenticator/NullAuthDeny/CMakeLists.txt |  4 ++
 .../{ => NullAuthDeny}/null_auth_deny.c       |  0
 20 files changed, 54 insertions(+), 53 deletions(-)
 create mode 100644 server/modules/authenticator/CDCPlainAuth/CMakeLists.txt
 rename server/modules/authenticator/{ => CDCPlainAuth}/cdc_plain_auth.c (100%)
 create mode 100644 server/modules/authenticator/GSSAPI/CMakeLists.txt
 create mode 100644 server/modules/authenticator/GSSAPI/GSSAPIAuth/CMakeLists.txt
 rename server/modules/authenticator/{ => GSSAPI/GSSAPIAuth}/gssapi_auth.c (99%)
 create mode 100644 server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/CMakeLists.txt
 rename server/modules/authenticator/{ => GSSAPI/GSSAPIBackendAuth}/gssapi_backend_auth.c (99%)
 rename server/modules/authenticator/{ => GSSAPI}/gssapi_auth.h (100%)
 rename server/modules/authenticator/{ => GSSAPI}/gssapi_auth_common.c (100%)
 create mode 100644 server/modules/authenticator/HTTPAuth/CMakeLists.txt
 rename server/modules/authenticator/{ => HTTPAuth}/http_auth.c (100%)
 create mode 100644 server/modules/authenticator/MaxAdminAuth/CMakeLists.txt
 rename server/modules/authenticator/{ => MaxAdminAuth}/max_admin_auth.c (100%)
 create mode 100644 server/modules/authenticator/MySQLBackendAuth/CMakeLists.txt
 rename server/modules/authenticator/{ => MySQLBackendAuth}/mysql_backend_auth.c (100%)
 create mode 100644 server/modules/authenticator/NullAuthAllow/CMakeLists.txt
 rename server/modules/authenticator/{ => NullAuthAllow}/null_auth_allow.c (100%)
 create mode 100644 server/modules/authenticator/NullAuthDeny/CMakeLists.txt
 rename server/modules/authenticator/{ => NullAuthDeny}/null_auth_deny.c (100%)

diff --git a/server/modules/authenticator/CDCPlainAuth/CMakeLists.txt b/server/modules/authenticator/CDCPlainAuth/CMakeLists.txt
new file mode 100644
index 000000000..af8e187cf
--- /dev/null
+++ b/server/modules/authenticator/CDCPlainAuth/CMakeLists.txt
@@ -0,0 +1,6 @@
+if(BUILD_CDC)
+  add_library(CDCPlainAuth SHARED cdc_plain_auth.c)
+  target_link_libraries(CDCPlainAuth maxscale-common)
+  set_target_properties(CDCPlainAuth PROPERTIES VERSION "1.0.0")
+  install_module(CDCPlainAuth core)
+endif()
diff --git a/server/modules/authenticator/cdc_plain_auth.c b/server/modules/authenticator/CDCPlainAuth/cdc_plain_auth.c
similarity index 100%
rename from server/modules/authenticator/cdc_plain_auth.c
rename to server/modules/authenticator/CDCPlainAuth/cdc_plain_auth.c
diff --git a/server/modules/authenticator/CMakeLists.txt b/server/modules/authenticator/CMakeLists.txt
index b8294d01a..f18e73a0d 100644
--- a/server/modules/authenticator/CMakeLists.txt
+++ b/server/modules/authenticator/CMakeLists.txt
@@ -1,49 +1,11 @@
+add_subdirectory(CDCPlainAuth)
+add_subdirectory(GSSAPI)
+add_subdirectory(HTTPAuth)
+add_subdirectory(MaxAdminAuth)
 add_subdirectory(MySQLAuth)
-
-add_library(MySQLBackendAuth SHARED mysql_backend_auth.c)
-target_link_libraries(MySQLBackendAuth maxscale-common MySQLCommon)
-set_target_properties(MySQLBackendAuth PROPERTIES VERSION "1.0.0")
-install_module(MySQLBackendAuth core)
-
-if (GSSAPI_FOUND AND SQLITE_FOUND)
-  if (NOT SQLITE_VERSION VERSION_LESS "3.7.7")
-    include_directories(${GSSAPI_INCS})
-    include_directories(${SQLITE_INCLUDE_DIR})
-
-    add_library(GSSAPIAuth SHARED gssapi_auth.c gssapi_auth_common.c)
-    target_link_libraries(GSSAPIAuth maxscale-common ${GSSAPI_LIBS} ${SQLITE_LIBRARIES} MySQLCommon)
-    set_target_properties(GSSAPIAuth PROPERTIES VERSION "1.0.0")
-    install_module(GSSAPIAuth core)
-
-    add_library(GSSAPIBackendAuth SHARED gssapi_backend_auth.c gssapi_auth_common.c)
-    target_link_libraries(GSSAPIBackendAuth maxscale-common  ${GSSAPI_LIBS} MySQLCommon)
-    set_target_properties(GSSAPIBackendAuth PROPERTIES VERSION "1.0.0")
-    install_module(GSSAPIBackendAuth core)
-
-  else()
-    message(STATUS "Minimum requires SQLite version for GSSAPIAuth is 3.7.7, current SQLite version is ${SQLITE_VERSION}")
-  endif()
-endif()
-
-add_library(NullAuthAllow SHARED null_auth_allow.c)
-target_link_libraries(NullAuthAllow maxscale-common)
-set_target_properties(NullAuthAllow PROPERTIES VERSION "1.0.0")
-install_module(NullAuthAllow core)
-
-add_library(NullAuthDeny SHARED null_auth_deny.c)
-target_link_libraries(NullAuthDeny maxscale-common)
-set_target_properties(NullAuthDeny PROPERTIES VERSION "1.0.0")
-install_module(NullAuthDeny core)
-
-add_library(MaxAdminAuth SHARED max_admin_auth.c)
-target_link_libraries(MaxAdminAuth maxscale-common)
-set_target_properties(MaxAdminAuth PROPERTIES VERSION "1.0.0")
-install_module(MaxAdminAuth core)
-
-add_library(HTTPAuth SHARED http_auth.c)
-target_link_libraries(HTTPAuth maxscale-common)
-set_target_properties(HTTPAuth PROPERTIES VERSION "1.0.0")
-install_module(HTTPAuth core)
+add_subdirectory(MySQLBackendAuth)
+add_subdirectory(NullAuthAllow)
+add_subdirectory(NullAuthDeny)
 
 # if(BUILD_TESTS)
 #   add_library(testprotocol SHARED testprotocol.c)
@@ -51,10 +13,4 @@ install_module(HTTPAuth core)
 #   target_link_libraries(testprotocol maxscale-common)
 #   install_module(testprotocol core)
 # endif()
-if(BUILD_CDC)
-  add_library(CDCPlainAuth SHARED cdc_plain_auth.c)
-  target_link_libraries(CDCPlainAuth maxscale-common)
-  set_target_properties(CDCPlainAuth PROPERTIES VERSION "1.0.0")
-  install_module(CDCPlainAuth core)
-endif()
 
diff --git a/server/modules/authenticator/GSSAPI/CMakeLists.txt b/server/modules/authenticator/GSSAPI/CMakeLists.txt
new file mode 100644
index 000000000..01588c2f7
--- /dev/null
+++ b/server/modules/authenticator/GSSAPI/CMakeLists.txt
@@ -0,0 +1,11 @@
+if (GSSAPI_FOUND AND SQLITE_FOUND)
+  if (NOT SQLITE_VERSION VERSION_LESS "3.7.7")
+    include_directories(${GSSAPI_INCS})
+    include_directories(${SQLITE_INCLUDE_DIR})
+
+    add_subdirectory(GSSAPIAuth)
+    add_subdirectory(GSSAPIBackendAuth)
+  else()
+    message(STATUS "Minimum requires SQLite version for GSSAPIAuth is 3.7.7, current SQLite version is ${SQLITE_VERSION}")
+  endif()
+endif()
diff --git a/server/modules/authenticator/GSSAPI/GSSAPIAuth/CMakeLists.txt b/server/modules/authenticator/GSSAPI/GSSAPIAuth/CMakeLists.txt
new file mode 100644
index 000000000..7dd3034a9
--- /dev/null
+++ b/server/modules/authenticator/GSSAPI/GSSAPIAuth/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_library(GSSAPIAuth SHARED gssapi_auth.c ../gssapi_auth_common.c)
+target_link_libraries(GSSAPIAuth maxscale-common ${GSSAPI_LIBS} ${SQLITE_LIBRARIES} MySQLCommon)
+set_target_properties(GSSAPIAuth PROPERTIES VERSION "1.0.0")
+install_module(GSSAPIAuth core)
diff --git a/server/modules/authenticator/gssapi_auth.c b/server/modules/authenticator/GSSAPI/GSSAPIAuth/gssapi_auth.c
similarity index 99%
rename from server/modules/authenticator/gssapi_auth.c
rename to server/modules/authenticator/GSSAPI/GSSAPIAuth/gssapi_auth.c
index 3d1f8b095..5909282f4 100644
--- a/server/modules/authenticator/gssapi_auth.c
+++ b/server/modules/authenticator/GSSAPI/GSSAPIAuth/gssapi_auth.c
@@ -19,7 +19,7 @@
 #include <maxscale/secrets.h>
 #include <maxscale/mysql_utils.h>
 #include <sqlite3.h>
-#include "gssapi_auth.h"
+#include "../gssapi_auth.h"
 
 /** Default timeout is one minute */
 #define MXS_SQLITE_BUSY_TIMEOUT 60000
diff --git a/server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/CMakeLists.txt b/server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/CMakeLists.txt
new file mode 100644
index 000000000..fc61e8e9d
--- /dev/null
+++ b/server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_library(GSSAPIBackendAuth SHARED gssapi_backend_auth.c ../gssapi_auth_common.c)
+target_link_libraries(GSSAPIBackendAuth maxscale-common  ${GSSAPI_LIBS} MySQLCommon)
+set_target_properties(GSSAPIBackendAuth PROPERTIES VERSION "1.0.0")
+install_module(GSSAPIBackendAuth core)
diff --git a/server/modules/authenticator/gssapi_backend_auth.c b/server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/gssapi_backend_auth.c
similarity index 99%
rename from server/modules/authenticator/gssapi_backend_auth.c
rename to server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/gssapi_backend_auth.c
index 44325881a..be4b27c33 100644
--- a/server/modules/authenticator/gssapi_backend_auth.c
+++ b/server/modules/authenticator/GSSAPI/GSSAPIBackendAuth/gssapi_backend_auth.c
@@ -16,7 +16,7 @@
 #include <maxscale/dcb.h>
 #include <maxscale/log_manager.h>
 #include <maxscale/protocol/mysql.h>
-#include "gssapi_auth.h"
+#include "../gssapi_auth.h"
 
 /**
  * @file gssapi_backend_auth.c - GSSAPI backend authenticator
diff --git a/server/modules/authenticator/gssapi_auth.h b/server/modules/authenticator/GSSAPI/gssapi_auth.h
similarity index 100%
rename from server/modules/authenticator/gssapi_auth.h
rename to server/modules/authenticator/GSSAPI/gssapi_auth.h
diff --git a/server/modules/authenticator/gssapi_auth_common.c b/server/modules/authenticator/GSSAPI/gssapi_auth_common.c
similarity index 100%
rename from server/modules/authenticator/gssapi_auth_common.c
rename to server/modules/authenticator/GSSAPI/gssapi_auth_common.c
diff --git a/server/modules/authenticator/HTTPAuth/CMakeLists.txt b/server/modules/authenticator/HTTPAuth/CMakeLists.txt
new file mode 100644
index 000000000..31e72afac
--- /dev/null
+++ b/server/modules/authenticator/HTTPAuth/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_library(HTTPAuth SHARED http_auth.c)
+target_link_libraries(HTTPAuth maxscale-common)
+set_target_properties(HTTPAuth PROPERTIES VERSION "1.0.0")
+install_module(HTTPAuth core)
diff --git a/server/modules/authenticator/http_auth.c b/server/modules/authenticator/HTTPAuth/http_auth.c
similarity index 100%
rename from server/modules/authenticator/http_auth.c
rename to server/modules/authenticator/HTTPAuth/http_auth.c
diff --git a/server/modules/authenticator/MaxAdminAuth/CMakeLists.txt b/server/modules/authenticator/MaxAdminAuth/CMakeLists.txt
new file mode 100644
index 000000000..7de926fb2
--- /dev/null
+++ b/server/modules/authenticator/MaxAdminAuth/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_library(MaxAdminAuth SHARED max_admin_auth.c)
+target_link_libraries(MaxAdminAuth maxscale-common)
+set_target_properties(MaxAdminAuth PROPERTIES VERSION "1.0.0")
+install_module(MaxAdminAuth core)
diff --git a/server/modules/authenticator/max_admin_auth.c b/server/modules/authenticator/MaxAdminAuth/max_admin_auth.c
similarity index 100%
rename from server/modules/authenticator/max_admin_auth.c
rename to server/modules/authenticator/MaxAdminAuth/max_admin_auth.c
diff --git a/server/modules/authenticator/MySQLBackendAuth/CMakeLists.txt b/server/modules/authenticator/MySQLBackendAuth/CMakeLists.txt
new file mode 100644
index 000000000..65582f6ca
--- /dev/null
+++ b/server/modules/authenticator/MySQLBackendAuth/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_library(MySQLBackendAuth SHARED mysql_backend_auth.c)
+target_link_libraries(MySQLBackendAuth maxscale-common MySQLCommon)
+set_target_properties(MySQLBackendAuth PROPERTIES VERSION "1.0.0")
+install_module(MySQLBackendAuth core)
diff --git a/server/modules/authenticator/mysql_backend_auth.c b/server/modules/authenticator/MySQLBackendAuth/mysql_backend_auth.c
similarity index 100%
rename from server/modules/authenticator/mysql_backend_auth.c
rename to server/modules/authenticator/MySQLBackendAuth/mysql_backend_auth.c
diff --git a/server/modules/authenticator/NullAuthAllow/CMakeLists.txt b/server/modules/authenticator/NullAuthAllow/CMakeLists.txt
new file mode 100644
index 000000000..2f11fb0f2
--- /dev/null
+++ b/server/modules/authenticator/NullAuthAllow/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_library(NullAuthAllow SHARED null_auth_allow.c)
+target_link_libraries(NullAuthAllow maxscale-common)
+set_target_properties(NullAuthAllow PROPERTIES VERSION "1.0.0")
+install_module(NullAuthAllow core)
diff --git a/server/modules/authenticator/null_auth_allow.c b/server/modules/authenticator/NullAuthAllow/null_auth_allow.c
similarity index 100%
rename from server/modules/authenticator/null_auth_allow.c
rename to server/modules/authenticator/NullAuthAllow/null_auth_allow.c
diff --git a/server/modules/authenticator/NullAuthDeny/CMakeLists.txt b/server/modules/authenticator/NullAuthDeny/CMakeLists.txt
new file mode 100644
index 000000000..bedb90cee
--- /dev/null
+++ b/server/modules/authenticator/NullAuthDeny/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_library(NullAuthDeny SHARED null_auth_deny.c)
+target_link_libraries(NullAuthDeny maxscale-common)
+set_target_properties(NullAuthDeny PROPERTIES VERSION "1.0.0")
+install_module(NullAuthDeny core)
diff --git a/server/modules/authenticator/null_auth_deny.c b/server/modules/authenticator/NullAuthDeny/null_auth_deny.c
similarity index 100%
rename from server/modules/authenticator/null_auth_deny.c
rename to server/modules/authenticator/NullAuthDeny/null_auth_deny.c

From f5280fecfe1956ed2ccfe43fdfc5c0883b932003 Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Thu, 1 Dec 2016 15:55:25 +0200
Subject: [PATCH 09/25] Fix avrouter/CMakeLists.txt

Needed to be updated when binlog was renamed to binlogrouter.
---
 server/modules/routing/avrorouter/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/modules/routing/avrorouter/CMakeLists.txt b/server/modules/routing/avrorouter/CMakeLists.txt
index 0fed40b78..d852862bb 100644
--- a/server/modules/routing/avrorouter/CMakeLists.txt
+++ b/server/modules/routing/avrorouter/CMakeLists.txt
@@ -1,7 +1,7 @@
 if(AVRO_FOUND AND JANSSON_FOUND)
   include_directories(${AVRO_INCLUDE_DIR})
   include_directories(${JANSSON_INCLUDE_DIR})
-  add_library(avrorouter SHARED avro.c ../binlog/binlog_common.c avro_client.c avro_schema.c avro_rbr.c avro_file.c avro_index.c)
+  add_library(avrorouter SHARED avro.c ../binlogrouter/binlog_common.c avro_client.c avro_schema.c avro_rbr.c avro_file.c avro_index.c)
   set_target_properties(avrorouter PROPERTIES VERSION "1.0.0")
   set_target_properties(avrorouter PROPERTIES LINK_FLAGS -Wl,-z,defs)
   target_link_libraries(avrorouter maxscale-common ${JANSSON_LIBRARIES} ${AVRO_LIBRARIES} maxavro sqlite3 lzma)

From 582948e690bd543524da2b63220853a2bc5ff1c7 Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Fri, 2 Dec 2016 09:32:14 +0200
Subject: [PATCH 10/25] Add UINT[32|64]_MAX defines

---
 server/modules/filter/cache/cachefilter.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/server/modules/filter/cache/cachefilter.h b/server/modules/filter/cache/cachefilter.h
index 2da6bcfaa..1accd1222 100644
--- a/server/modules/filter/cache/cachefilter.h
+++ b/server/modules/filter/cache/cachefilter.h
@@ -38,6 +38,14 @@ class StorageFactory;
 #define CACHE_DEBUG_MIN          CACHE_DEBUG_NONE
 #define CACHE_DEBUG_MAX          (CACHE_DEBUG_RULES | CACHE_DEBUG_USAGE | CACHE_DEBUG_DECISIONS)
 
+#if !defined(UINT32_MAX)
+#define UINT32_MAX		(4294967295U)
+#endif
+
+#if !defined(UINT64_MAX)
+#define UINT64_MAX		(18446744073709551615UL)
+#endif
+
 // Count
 #define CACHE_DEFAULT_MAX_RESULTSET_ROWS UINT32_MAX
 // Bytes

From 3ed4ef40ba54edfecfafe9116df2223a9e015510 Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Fri, 2 Dec 2016 10:04:57 +0200
Subject: [PATCH 11/25] Do not use PRIu[32|64] macros in C++ code

---
 .../cache/storage/storage_inmemory/storage_inmemory.cc    | 8 ++++----
 .../cache/storage/storage_rocksdb/storage_rocksdb.cc      | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/server/modules/filter/cache/storage/storage_inmemory/storage_inmemory.cc b/server/modules/filter/cache/storage/storage_inmemory/storage_inmemory.cc
index b9cb1cd69..34a0a10cc 100644
--- a/server/modules/filter/cache/storage/storage_inmemory/storage_inmemory.cc
+++ b/server/modules/filter/cache/storage/storage_inmemory/storage_inmemory.cc
@@ -41,14 +41,14 @@ CACHE_STORAGE* createInstance(cache_thread_model_t model,
 
     if (max_count != 0)
     {
-        MXS_WARNING("A maximum item count of %" PRIu32 " specified, although 'storage_inMemory' "
-                    "does not enforce such a limit.", max_count);
+        MXS_WARNING("A maximum item count of %u specified, although 'storage_inMemory' "
+                    "does not enforce such a limit.", (unsigned int)max_count);
     }
 
     if (max_size != 0)
     {
-        MXS_WARNING("A maximum size of %" PRIu64 " specified, although 'storage_inMemory' "
-                    "does not enforce such a limit.", max_size);
+        MXS_WARNING("A maximum size of %lu specified, although 'storage_inMemory' "
+                    "does not enforce such a limit.", (unsigned long)max_size);
     }
 
     try
diff --git a/server/modules/filter/cache/storage/storage_rocksdb/storage_rocksdb.cc b/server/modules/filter/cache/storage/storage_rocksdb/storage_rocksdb.cc
index 8f68de876..792fa04f6 100644
--- a/server/modules/filter/cache/storage/storage_rocksdb/storage_rocksdb.cc
+++ b/server/modules/filter/cache/storage/storage_rocksdb/storage_rocksdb.cc
@@ -40,14 +40,14 @@ CACHE_STORAGE* createInstance(cache_thread_model_t, // Ignored, RocksDB always M
 
     if (maxCount != 0)
     {
-        MXS_WARNING("A maximum item count of %" PRIu32 " specifed, although 'storage_rocksdb' "
-                    "does not enforce such a limit.", maxCount);
+        MXS_WARNING("A maximum item count of %u specifed, although 'storage_rocksdb' "
+                    "does not enforce such a limit.", (unsigned int)maxCount);
     }
 
     if (maxSize != 0)
     {
-        MXS_WARNING("A maximum size of %" PRIu64 " specified, although 'storage_rocksdb' "
-                    "does not enforce such a limit.", maxSize);
+        MXS_WARNING("A maximum size of %lu specified, although 'storage_rocksdb' "
+                    "does not enforce such a limit.", (unsigned long)maxSize);
     }
 
     try

From bc3d6f2ac70279c80bc181e2bf7ef002282bbefb Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Wed, 30 Nov 2016 15:01:25 +0200
Subject: [PATCH 12/25] Change `functions` to `commands`

The module commands operations are now listed as `commands` instead of
`functions`. The output was also formatted and an optional filtering was
added to the `list commands` call.
---
 server/modules/routing/debugcli/debugcmd.c | 38 ++++++++++++----------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c
index e95636e88..f7c8be6d6 100644
--- a/server/modules/routing/debugcli/debugcmd.c
+++ b/server/modules/routing/debugcli/debugcmd.c
@@ -290,8 +290,8 @@ bool listfuncs_cb(const MODULECMD *cmd, void *data)
 {
     DCB *dcb = (DCB*)data;
 
-    dcb_printf(dcb, "%s::%s(", cmd->domain, cmd->identifier);
-
+    dcb_printf(dcb, "Command: %s %s\n", cmd->domain, cmd->identifier);
+    dcb_printf(dcb, "Parameters: ");
 
     for (int i = 0; i < cmd->arg_count_max; i++)
     {
@@ -309,8 +309,7 @@ bool listfuncs_cb(const MODULECMD *cmd, void *data)
         }
     }
 
-    dcb_printf(dcb, ")\n");
-
+    dcb_printf(dcb, "\n\n");
 
     for (int i = 0; i < cmd->arg_count_max; i++)
     {
@@ -334,9 +333,9 @@ bool listfuncs_cb(const MODULECMD *cmd, void *data)
     return true;
 }
 
-void dListFunctions(DCB *dcb)
+void dListCommands(DCB *dcb, const char *domain, const char *ident)
 {
-    modulecmd_foreach(NULL, NULL, listfuncs_cb, dcb);
+    modulecmd_foreach(domain, ident, listfuncs_cb, dcb);
 }
 
 /**
@@ -405,10 +404,13 @@ struct subcommand listoptions[] =
         {0, 0, 0}
     },
     {
-        "functions", 0, 0, dListFunctions,
-        "List registered functions",
-        "List all registered functions",
-        {0}
+        "commands", 0, 2, dListCommands,
+        "List registered commands",
+        "Usage list commands [DOMAIN] [COMMAND]\n"
+        "Parameters:\n"
+        "DOMAIN  Regular expressions for filtering module domains\n"
+        "COMMAND Regular expressions for filtering module commands\n",
+        {ARG_TYPE_STRING, ARG_TYPE_STRING}
     },
     { EMPTY_OPTION}
 };
@@ -1385,9 +1387,9 @@ static inline bool requires_output_dcb(const MODULECMD *cmd)
     return cmd->arg_count_max > 0 && MODULECMD_GET_TYPE(type) == MODULECMD_ARG_OUTPUT;
 }
 
-static void callFunction(DCB *dcb, char *domain, char *id, char *v3,
-                         char *v4, char *v5, char *v6, char *v7, char *v8, char *v9,
-                         char *v10, char *v11, char *v12)
+static void callModuleCommand(DCB *dcb, char *domain, char *id, char *v3,
+                              char *v4, char *v5, char *v6, char *v7, char *v8, char *v9,
+                              char *v10, char *v11, char *v12)
 {
     const void *values[11] = {v3, v4, v5, v6, v7, v8, v9, v10, v11, v12};
     const int valuelen = sizeof(values) / sizeof(values[0]);
@@ -1404,7 +1406,7 @@ static void callFunction(DCB *dcb, char *domain, char *id, char *v3,
     {
         if (requires_output_dcb(cmd))
         {
-            /** The function requires a DCB for output, add the client DCB
+            /** The command requires a DCB for output, add the client DCB
              * as the first argument */
             for (int i = valuelen - 1; i > 0; i--)
             {
@@ -1438,10 +1440,10 @@ static void callFunction(DCB *dcb, char *domain, char *id, char *v3,
 struct subcommand calloptions[] =
 {
     {
-        "function", 2, 12, callFunction,
-        "Call module function",
-        "Usage: call function NAMESPACE FUNCTION ARGS...\n"
-        "To list all registered functions, run 'list functions'.\n",
+        "command", 2, 12, callModuleCommand,
+        "Call module command",
+        "Usage: call command NAMESPACE COMMAND ARGS...\n"
+        "To list all registered commands, run 'list commands'.\n",
         { ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,
             ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,
             ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING}

From 994299b4f111612bbae4346e478b885b51dc3fba Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Fri, 2 Dec 2016 10:58:21 +0200
Subject: [PATCH 13/25] MXS-1043: Handle @@identity like @@last_insert_id

The type of @@identity, @@last_insert_id and last_insert_id() is
now the same, that is, QUERY_TYPE_READ|QUERY_TYPE_MASTER_READ.
---
 .../qc_mysqlembedded/qc_mysqlembedded.cc      | 22 ++++++++++++++-----
 query_classifier/qc_sqlite/qc_sqlite.c        | 10 ++++++++-
 query_classifier/test/expected.sql            |  3 +++
 query_classifier/test/input.sql               |  3 +++
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc
index c1d336d3e..fa900b4ec 100644
--- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc
+++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc
@@ -928,11 +928,23 @@ static uint32_t resolve_query_type(parsing_info_t *pi, THD* thd)
 
                         /** System session variable */
                     case Item_func::GSYSVAR_FUNC:
-                        func_qtype |= QUERY_TYPE_SYSVAR_READ;
-                        MXS_DEBUG("%lu [resolve_query_type] "
-                                  "functype GSYSVAR_FUNC, system "
-                                  "variable read.",
-                                  pthread_self());
+                        {
+                            const char* name = item->name;
+                            if (name &&
+                                ((strcasecmp(name, "@@last_insert_id") == 0) ||
+                                 (strcasecmp(name, "@@identity") == 0)))
+                            {
+                                func_qtype |= QUERY_TYPE_MASTER_READ;
+                            }
+                            else
+                            {
+                                func_qtype |= QUERY_TYPE_SYSVAR_READ;
+                            }
+                            MXS_DEBUG("%lu [resolve_query_type] "
+                                      "functype GSYSVAR_FUNC, system "
+                                      "variable read.",
+                                      pthread_self());
+                        }
                         break;
 
                         /** User-defined variable read */
diff --git a/query_classifier/qc_sqlite/qc_sqlite.c b/query_classifier/qc_sqlite/qc_sqlite.c
index a6f351c0d..910bd323c 100644
--- a/query_classifier/qc_sqlite/qc_sqlite.c
+++ b/query_classifier/qc_sqlite/qc_sqlite.c
@@ -833,7 +833,15 @@ static void update_field_infos(QC_SQLITE_INFO* info,
                     }
                     else
                     {
-                        info->types |= QUERY_TYPE_SYSVAR_READ;
+                        if ((strcasecmp(&zToken[2], "identity") == 0) ||
+                            (strcasecmp(&zToken[2], "last_insert_id") == 0))
+                        {
+                            info->types |= QUERY_TYPE_MASTER_READ;
+                        }
+                        else
+                        {
+                            info->types |= QUERY_TYPE_SYSVAR_READ;
+                        }
                     }
                 }
                 else
diff --git a/query_classifier/test/expected.sql b/query_classifier/test/expected.sql
index fd67bcc91..0aa0a9739 100644
--- a/query_classifier/test/expected.sql
+++ b/query_classifier/test/expected.sql
@@ -12,3 +12,6 @@ QUERY_TYPE_BEGIN_TRX
 QUERY_TYPE_ROLLBACK
 QUERY_TYPE_COMMIT
 QUERY_TYPE_SESSION_WRITE
+QUERY_TYPE_READ|QUERY_TYPE_MASTER_READ
+QUERY_TYPE_READ|QUERY_TYPE_MASTER_READ
+QUERY_TYPE_READ|QUERY_TYPE_MASTER_READ
diff --git a/query_classifier/test/input.sql b/query_classifier/test/input.sql
index 450bf81c8..bfa445297 100644
--- a/query_classifier/test/input.sql
+++ b/query_classifier/test/input.sql
@@ -12,3 +12,6 @@ BEGIN;
 ROLLBACK;
 COMMIT;
 use X;
+select last_insert_id();
+select @@last_insert_id;
+select @@identity;

From adbd66699191cdf59f91e2acef1b2fa93dde9af0 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Wed, 30 Nov 2016 19:32:12 +0200
Subject: [PATCH 14/25] Change module command parameter types

This commit introduces safe session references that can be handled without
holding locks. This allows the safe searching of sessions with the unique
ID of the session.

Remove the use of raw pointers passed as strings. Change the comments of
the argument types and add more details to the parsing function
documentation.
---
 include/maxscale/modulecmd.h     | 28 +++++++++++++++++-------
 include/maxscale/session.h       | 20 +++++++++++++++++
 server/core/modulecmd.c          | 30 +++++++-------------------
 server/core/session.c            | 37 ++++++++++++++++++++++++++++++++
 server/core/test/testmodulecmd.c | 22 +++++--------------
 5 files changed, 90 insertions(+), 47 deletions(-)

diff --git a/include/maxscale/modulecmd.h b/include/maxscale/modulecmd.h
index 10f772bb8..8d9a11aef 100644
--- a/include/maxscale/modulecmd.h
+++ b/include/maxscale/modulecmd.h
@@ -55,14 +55,12 @@ typedef struct
 #define MODULECMD_ARG_NONE        0 /**< Empty argument */
 #define MODULECMD_ARG_STRING      1 /**< String */
 #define MODULECMD_ARG_BOOLEAN     2 /**< Boolean value */
-#define MODULECMD_ARG_SERVICE     3 /**< Service name */
-#define MODULECMD_ARG_SERVER      4 /**< Server name */
-#define MODULECMD_ARG_SESSION     5 /**< SESSION pointer in string format */
-#define MODULECMD_ARG_SESSION_PTR 6 /**< Raw SESSION pointer */
-#define MODULECMD_ARG_DCB         7 /**< DCB pointer in string format*/
-#define MODULECMD_ARG_DCB_PTR     8 /**< Raw DCB pointer*/
-#define MODULECMD_ARG_MONITOR     9 /**< Monitor name */
-#define MODULECMD_ARG_FILTER      10 /**< Filter name */
+#define MODULECMD_ARG_SERVICE     3 /**< Service */
+#define MODULECMD_ARG_SERVER      4 /**< Server */
+#define MODULECMD_ARG_SESSION     6 /**< Session */
+#define MODULECMD_ARG_DCB         8 /**< DCB */
+#define MODULECMD_ARG_MONITOR     9 /**< Monitor */
+#define MODULECMD_ARG_FILTER      10 /**< Filter */
 #define MODULECMD_ARG_OUTPUT      11 /**< DCB suitable for writing results to.
                                           This should always be the first argument
                                           if the function requires an output DCB. */
@@ -157,6 +155,20 @@ const MODULECMD* modulecmd_find_command(const char *domain, const char *identifi
 /**
  * @brief Parse arguments for a command
  *
+ * The argument types expect different forms of input.
+ *
+ * | Argument type         | Expected input    |
+ * |-----------------------|-------------------|
+ * | MODULECMD_ARG_SERVICE | Service name      |
+ * | MODULECMD_ARG_SERVER  | Server name       |
+ * | MODULECMD_ARG_SESSION | Session unique ID |
+ * | MODULECMD_ARG_MONITOR | Monitor name      |
+ * | MODULECMD_ARG_FILTER  | Filter name       |
+ * | MODULECMD_ARG_STRING  | String            |
+ * | MODULECMD_ARG_BOOLEAN | Boolean value     |
+ * | MODULECMD_ARG_DCB     | Raw DCB pointer   |
+ * | MODULECMD_ARG_OUTPUT  | DCB for output    |
+ *
  * @param cmd Command for which the parameters are parsed
  * @param argc Number of arguments
  * @param argv Argument list in string format of size @c argc
diff --git a/include/maxscale/session.h b/include/maxscale/session.h
index 9dda114cf..6b5f401c7 100644
--- a/include/maxscale/session.h
+++ b/include/maxscale/session.h
@@ -339,4 +339,24 @@ static inline bool session_set_autocommit(SESSION* ses, bool autocommit)
     return prev_autocommit;
 }
 
+/**
+ * @brief Get a session reference by ID
+ *
+ * This creates an additional reference to a session whose unique ID matches @c id.
+ *
+ * @param id Unique session ID
+ * @return Reference to a SESSION or NULL if the session was not found
+ *
+ * @note The caller must free the session reference by passing it to the
+ * @c session_put_ref function
+ */
+SESSION* session_get_ref(int id);
+
+/**
+ * @brief Release a session reference
+ *
+ * @param session Session reference to release
+ */
+void session_put_ref(SESSION *session);
+
 MXS_END_DECLS
diff --git a/server/core/modulecmd.c b/server/core/modulecmd.c
index 018f7bb97..d9146563a 100644
--- a/server/core/modulecmd.c
+++ b/server/core/modulecmd.c
@@ -277,25 +277,15 @@ static bool process_argument(modulecmd_arg_type_t *type, const void* value,
                 break;
 
             case MODULECMD_ARG_SESSION:
-                arg->type.type = MODULECMD_ARG_SESSION;
-                arg->value.session = (SESSION*)strtol((char*)value, NULL, 0);
-                rval = true;
-                break;
-
-            case MODULECMD_ARG_SESSION_PTR:
-                arg->type.type = MODULECMD_ARG_SESSION_PTR;
-                arg->value.session = (SESSION*)value;
+                if ((arg->value.session = session_get_ref(atoi(value))))
+                {
+                    arg->type.type = MODULECMD_ARG_SESSION;
+                }
                 rval = true;
                 break;
 
             case MODULECMD_ARG_DCB:
                 arg->type.type = MODULECMD_ARG_DCB;
-                arg->value.dcb = (DCB*)strtol((char*)value, NULL, 0);
-                rval = true;
-                break;
-
-            case MODULECMD_ARG_DCB_PTR:
-                arg->type.type = MODULECMD_ARG_DCB_PTR;
                 arg->value.dcb = (DCB*)value;
                 rval = true;
                 break;
@@ -373,6 +363,10 @@ static void free_argument(struct arg_node *arg)
             MXS_FREE(arg->value.string);
             break;
 
+        case MODULECMD_ARG_SESSION:
+            session_put_ref(arg->value.session);
+            break;
+
         default:
             break;
     }
@@ -625,18 +619,10 @@ char* modulecmd_argtype_to_str(modulecmd_arg_type_t *type)
             strtype = "SESSION";
             break;
 
-        case MODULECMD_ARG_SESSION_PTR:
-            strtype = "SESSION_PTR";
-            break;
-
         case MODULECMD_ARG_DCB:
             strtype = "DCB";
             break;
 
-        case MODULECMD_ARG_DCB_PTR:
-            strtype = "DCB_PTR";
-            break;
-
         case MODULECMD_ARG_MONITOR:
             strtype = "MONITOR";
             break;
diff --git a/server/core/session.c b/server/core/session.c
index 9b155c982..15fc6207b 100644
--- a/server/core/session.c
+++ b/server/core/session.c
@@ -915,3 +915,40 @@ const char* session_trx_state_to_string(session_trx_state_t state)
     MXS_ERROR("Unknown session_trx_state_t value: %d", (int)state);
     return "UNKNOWN";
 }
+
+static bool ses_find_id(DCB *dcb, void *data)
+{
+    void **params = (void**)data;
+    SESSION **ses = (SESSION**)params[0];
+    int *id = (int*)params[1];
+    bool rval = true;
+
+    if (dcb->session->ses_id == *id)
+    {
+        /** We need to increment the reference count of the session to prevent it
+         * from being freed while it's in use. */
+        atomic_add(&dcb->session->refcount, 1);
+        *ses = dcb->session;
+        rval = false;
+    }
+
+    return rval;
+}
+
+SESSION* session_get_ref(int id)
+{
+    SESSION *session = NULL;
+    void *params[] = {&session, &id};
+
+    dcb_foreach(ses_find_id, params);
+
+    return session;
+}
+
+void session_put_ref(SESSION *session)
+{
+    if (session)
+    {
+        session_free(session);
+    }
+}
diff --git a/server/core/test/testmodulecmd.c b/server/core/test/testmodulecmd.c
index 6b6628e2f..5b341faa8 100644
--- a/server/core/test/testmodulecmd.c
+++ b/server/core/test/testmodulecmd.c
@@ -305,16 +305,12 @@ int test_map()
 }
 
 static DCB my_dcb;
-static SESSION my_session;
 
 bool ptrfn(const MODULECMD_ARG *argv)
 {
     bool rval = false;
 
-    if (argv->argc == 4 && argv->argv[0].value.dcb == &my_dcb &&
-        argv->argv[1].value.dcb == &my_dcb &&
-        argv->argv[2].value.session == &my_session &&
-        argv->argv[3].value.session == &my_session)
+    if (argv->argc == 1 && argv->argv[0].value.dcb == &my_dcb)
     {
         rval = true;
     }
@@ -329,26 +325,18 @@ int test_pointers()
 
     modulecmd_arg_type_t args[] =
     {
-        {MODULECMD_ARG_DCB, ""},
-        {MODULECMD_ARG_DCB_PTR, ""},
-        {MODULECMD_ARG_SESSION, ""},
-        {MODULECMD_ARG_SESSION_PTR, ""}
+        {MODULECMD_ARG_DCB, ""}
     };
 
-    TEST(modulecmd_register_command(ns, id, ptrfn, 4, args), "Registering a command should succeed");
+    TEST(modulecmd_register_command(ns, id, ptrfn, 1, args), "Registering a command should succeed");
     TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty");
 
     const MODULECMD *cmd = modulecmd_find_command(ns, id);
     TEST(cmd, "The registered command should be found");
-    char dcb_str[200];
-    char session_str[200];
 
-    sprintf(dcb_str, "%p", &my_dcb);
-    sprintf(session_str, "%p", &my_session);
+    const void* params[] = {&my_dcb};
 
-    const void* params[] = {dcb_str, &my_dcb, session_str, &my_session};
-
-    MODULECMD_ARG *arg = modulecmd_arg_parse(cmd, 4, params);
+    MODULECMD_ARG *arg = modulecmd_arg_parse(cmd, 1, params);
     TEST(arg, "Parsing arguments should succeed");
     TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty");
 

From b2e11d41d59c530ae1d53a7df5c8bcf46cf7c38b Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Fri, 2 Dec 2016 13:19:07 +0200
Subject: [PATCH 15/25] MXS-536: Add option to MySQLAuth that skips
 authentication

Disabling authentication in MaxScale allows creation of users which act
like wildcard users but require that the connection is made through
MaxScale.
---
 .../Authenticators/MySQL-Authenticator.md     | 19 +++++++++++++++++++
 .../MaxScale-2.1.0-Release-Notes.md           | 11 ++++++++---
 .../authenticator/MySQLAuth/mysql_auth.c      | 14 ++++++++++++--
 3 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/Documentation/Authenticators/MySQL-Authenticator.md b/Documentation/Authenticators/MySQL-Authenticator.md
index 82db5d52d..f2397d018 100644
--- a/Documentation/Authenticators/MySQL-Authenticator.md
+++ b/Documentation/Authenticators/MySQL-Authenticator.md
@@ -14,6 +14,25 @@ options. The `authenticator_options` parameter is supported by listeners
 and servers and expects a comma-separated list of key-value pairs. The
 following options contain examples on how to define it.
 
+### `skip_authentication`
+
+This option takes a boolean value which controls whether MaxScale will fully
+authenticate users. This option is disabled by default.
+
+Disabling authentication in MaxScale will allow MaxScale to act as a security
+gateway to the server. The authentication of users is offloaded to the backend
+server.
+
+For example, creating the user `jdoe@%` will allow the user _jdoe_ to connect
+from any IP address. This can be a problem if all traffic needs to go through
+MaxScale. By enabling this option and replacing the user with
+`jdoe@maxscale-IP`, the users can still connect from any client IP but will be
+forced to go though MaxScale.
+
+```
+authenticator_options=skip_authentication=true
+```
+
 ### `cache_dir`
 
 The location where the user credential cache is stored. The default value
diff --git a/Documentation/Release-Notes/MaxScale-2.1.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.1.0-Release-Notes.md
index 00db1d6cd..c539b49d7 100644
--- a/Documentation/Release-Notes/MaxScale-2.1.0-Release-Notes.md
+++ b/Documentation/Release-Notes/MaxScale-2.1.0-Release-Notes.md
@@ -104,9 +104,7 @@ following new commands were added to maxadmin, see output of `maxadmin help
 With these new features, you can start MaxScale without the servers and define
 them later.
 
-# Module commands
-
-## Module commands
+### Module commands
 
 Introduced in MaxScale 2.1, the module commands are special, module-specific
 commands. They allow the modules to expand beyound the capabilities of the
@@ -145,6 +143,13 @@ aimed for two node master-slave clusters where the slave can act as a
 master in case the original master fails. For more details, please read
 the [MySQL Monitor Documentation](../Monitors/MySQL-Monitor.md).
 
+### Permissive authentication mode for MySQLAuth
+
+The MySQL authentication module supports the `skip_authentication` option which
+allows authentication to always succedd in MaxScale. This option offloads the
+actual authentication to the backend server and it can be used to implement a
+secure version of a wildcard user.
+
 ## Bug fixes
 
 [Here is a list of bugs fixed since the release of MaxScale 2.0.X.](https://jira.mariadb.org/browse/MXS-739?jql=project%20%3D%20MXS%20AND%20issuetype%20%3D%20Bug%20AND%20resolution%20in%20(Fixed%2C%20Done)%20AND%20fixVersion%20%3D%202.0.0)
diff --git a/server/modules/authenticator/MySQLAuth/mysql_auth.c b/server/modules/authenticator/MySQLAuth/mysql_auth.c
index 17ade6b30..ee011fd7c 100644
--- a/server/modules/authenticator/MySQLAuth/mysql_auth.c
+++ b/server/modules/authenticator/MySQLAuth/mysql_auth.c
@@ -39,6 +39,7 @@ typedef struct mysql_auth
 {
     char *cache_dir;          /**< Custom cache directory location */
     bool inject_service_user; /**< Inject the service user into the list of users */
+    bool skip_auth;           /**< Authentication will always be successful */
 } MYSQL_AUTH;
 
 
@@ -144,6 +145,7 @@ static void* mysql_auth_init(char **options)
         bool error = false;
         instance->cache_dir = NULL;
         instance->inject_service_user = true;
+        instance->skip_auth = false;
 
         for (int i = 0; options[i]; i++)
         {
@@ -165,6 +167,10 @@ static void* mysql_auth_init(char **options)
                 {
                     instance->inject_service_user = config_truth_value(value);
                 }
+                else if (strcmp(options[i], "skip_authentication") == 0)
+                {
+                    instance->skip_auth = config_truth_value(value);
+                }
                 else
                 {
                     MXS_ERROR("Unknown authenticator option: %s", options[i]);
@@ -248,17 +254,21 @@ mysql_auth_authenticate(DCB *dcb)
         auth_ret = combined_auth_check(dcb, client_data->auth_token, client_data->auth_token_len,
                                        protocol, client_data->user, client_data->client_sha1, client_data->db);
 
+        MYSQL_AUTH *instance = (MYSQL_AUTH*)dcb->listener->auth_instance;
+
         /* On failed authentication try to load user table from backend database */
         /* Success for service_refresh_users returns 0 */
-        if (MXS_AUTH_SUCCEEDED != auth_ret && 0 == service_refresh_users(dcb->service))
+        if (MXS_AUTH_SUCCEEDED != auth_ret && !instance->skip_auth &&
+            0 == service_refresh_users(dcb->service))
         {
             auth_ret = combined_auth_check(dcb, client_data->auth_token, client_data->auth_token_len, protocol,
                                            client_data->user, client_data->client_sha1, client_data->db);
         }
 
         /* on successful authentication, set user into dcb field */
-        if (MXS_AUTH_SUCCEEDED == auth_ret)
+        if (MXS_AUTH_SUCCEEDED == auth_ret || instance->skip_auth)
         {
+            auth_ret = MXS_AUTH_SUCCEEDED;
             dcb->user = MXS_STRDUP_A(client_data->user);
             /** Send an OK packet to the client */
         }

From a4bc5753538515416a6e0ec186757b2191cf3e12 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Fri, 2 Dec 2016 14:39:32 +0200
Subject: [PATCH 16/25] Remove direct freeing of sessions

Sessions are now always freed by releasing the last reference to it.
---
 include/maxscale/session.h      | 19 ++++++++++---
 server/core/dcb.c               |  4 +--
 server/core/modulecmd.c         |  2 +-
 server/core/session.c           | 47 ++++++++++++---------------------
 server/modules/filter/tee/tee.c |  2 +-
 5 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/include/maxscale/session.h b/include/maxscale/session.h
index 6b5f401c7..3553cc0ab 100644
--- a/include/maxscale/session.h
+++ b/include/maxscale/session.h
@@ -204,7 +204,6 @@ typedef struct session
 
 SESSION *session_alloc(struct service *, struct dcb *);
 SESSION *session_set_dummy(struct dcb *);
-bool session_free(SESSION *);
 int session_isvalid(SESSION *);
 int session_reply(void *inst, void *session, GWBUF *data);
 char *session_get_remote(SESSION *);
@@ -347,10 +346,22 @@ static inline bool session_set_autocommit(SESSION* ses, bool autocommit)
  * @param id Unique session ID
  * @return Reference to a SESSION or NULL if the session was not found
  *
- * @note The caller must free the session reference by passing it to the
- * @c session_put_ref function
+ * @note The caller must free the session reference by calling session_put_ref
  */
-SESSION* session_get_ref(int id);
+SESSION* session_get_by_id(int id);
+
+/**
+ * @brief Get a session reference
+ *
+ * This creates an additional reference to a session which allows it to live
+ * as long as it is needed.
+ *
+ * @param session Session reference to get
+ * @return Reference to a SESSION
+ *
+ * @note The caller must free the session reference by calling session_put_ref
+ */
+SESSION* session_get_ref(SESSION *sessoin);
 
 /**
  * @brief Release a session reference
diff --git a/server/core/dcb.c b/server/core/dcb.c
index 8589315fb..8ac2fd1f7 100644
--- a/server/core/dcb.c
+++ b/server/core/dcb.c
@@ -348,7 +348,7 @@ dcb_final_free(DCB *dcb)
             bool is_client_dcb = (DCB_ROLE_CLIENT_HANDLER == dcb->dcb_role ||
                                   DCB_ROLE_INTERNAL == dcb->dcb_role);
 
-            session_free(local_session);
+            session_put_ref(local_session);
 
             if (is_client_dcb)
             {
@@ -1714,7 +1714,7 @@ dcb_maybe_add_persistent(DCB *dcb)
             CHK_SESSION(local_session);
             if (SESSION_STATE_DUMMY != local_session->state)
             {
-                session_free(local_session);
+                session_put_ref(local_session);
             }
         }
         spinlock_acquire(&dcb->cb_lock);
diff --git a/server/core/modulecmd.c b/server/core/modulecmd.c
index d9146563a..d3cdd38b3 100644
--- a/server/core/modulecmd.c
+++ b/server/core/modulecmd.c
@@ -277,7 +277,7 @@ static bool process_argument(modulecmd_arg_type_t *type, const void* value,
                 break;
 
             case MODULECMD_ARG_SESSION:
-                if ((arg->value.session = session_get_ref(atoi(value))))
+                if ((arg->value.session = session_get_by_id(atoi(value))))
                 {
                     arg->type.type = MODULECMD_ARG_SESSION;
                 }
diff --git a/server/core/session.c b/server/core/session.c
index 15fc6207b..54ab0fe81 100644
--- a/server/core/session.c
+++ b/server/core/session.c
@@ -336,31 +336,14 @@ session_simple_free(SESSION *session, DCB *dcb)
  *
  * @param session       The session to deallocate
  */
-bool
-session_free(SESSION *session)
+static void session_free(SESSION *session)
 {
-    if (NULL == session || SESSION_STATE_DUMMY == session->state)
-    {
-        return true;
-    }
     CHK_SESSION(session);
+    ss_dassert(session->refcount == 0);
 
-    /*
-     * Remove one reference. If there are no references left,
-     * free session.
-     */
-    if (atomic_add(&session->refcount, -1) > 1)
-    {
-        /* Must be one or more references left */
-        return false;
-    }
     session->state = SESSION_STATE_TO_BE_FREED;
-
     atomic_add(&session->service->stats.n_current, -1);
 
-    /***
-     *
-     */
     if (session->client_dcb)
     {
         dcb_free_all_memory(session->client_dcb);
@@ -396,9 +379,7 @@ session_free(SESSION *session)
         MXS_FREE(session->filters);
     }
 
-    MXS_INFO("Stopped %s client session [%lu]",
-             session->service->name,
-             session->ses_id);
+    MXS_INFO("Stopped %s client session [%lu]", session->service->name, session->ses_id);
 
     /** Disable trace and decrease trace logger counter */
     session_disable_log_priority(session, LOG_INFO);
@@ -409,7 +390,6 @@ session_free(SESSION *session)
         session->state = SESSION_STATE_FREE;
         session_final_free(session);
     }
-    return true;
 }
 
 static void
@@ -925,17 +905,14 @@ static bool ses_find_id(DCB *dcb, void *data)
 
     if (dcb->session->ses_id == *id)
     {
-        /** We need to increment the reference count of the session to prevent it
-         * from being freed while it's in use. */
-        atomic_add(&dcb->session->refcount, 1);
-        *ses = dcb->session;
+        *ses = session_get_ref(dcb->session);
         rval = false;
     }
 
     return rval;
 }
 
-SESSION* session_get_ref(int id)
+SESSION* session_get_by_id(int id)
 {
     SESSION *session = NULL;
     void *params[] = {&session, &id};
@@ -945,10 +922,20 @@ SESSION* session_get_ref(int id)
     return session;
 }
 
+SESSION* session_get_ref(SESSION *session)
+{
+    atomic_add(&session->refcount, 1);
+    return session;
+}
+
 void session_put_ref(SESSION *session)
 {
-    if (session)
+    if (session && session->state != SESSION_STATE_DUMMY)
     {
-        session_free(session);
+        /** Remove one reference. If there are no references left, free session */
+        if (atomic_add(&session->refcount, -1) == 1)
+        {
+            session_free(session);
+        }
     }
 }
diff --git a/server/modules/filter/tee/tee.c b/server/modules/filter/tee/tee.c
index 26a15e130..0e8707b77 100644
--- a/server/modules/filter/tee/tee.c
+++ b/server/modules/filter/tee/tee.c
@@ -701,7 +701,7 @@ freeSession(FILTER *instance, void *session)
 
         if (state == SESSION_STATE_ROUTER_READY)
         {
-            session_free(ses);
+            session_put_ref(ses);
         }
         else if (state == SESSION_STATE_TO_BE_FREED)
         {

From be7a31561471722bd581613cf845d8640dd0a412 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Fri, 2 Dec 2016 19:08:54 +0200
Subject: [PATCH 17/25] Exit on bad maxadmin arguments

The errors were detected but the code proceeded to call various functions
with bad pointers. This led to a crash if a bad server name was given to
'show server'.
---
 server/modules/routing/debugcli/debugcmd.c | 128 +++++++++++----------
 1 file changed, 66 insertions(+), 62 deletions(-)

diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c
index f7c8be6d6..bdddbd68e 100644
--- a/server/modules/routing/debugcli/debugcmd.c
+++ b/server/modules/routing/debugcli/debugcmd.c
@@ -1755,6 +1755,7 @@ execute_cmd(CLI_SESSION *cli)
                         else
                         {
                             unsigned long arg_list[MAXARGS] = {};
+                            bool ok = true;
 
                             for (int k = 0; k < cmds[i].options[j].argc_max && k < argc; k++)
                             {
@@ -1762,72 +1763,75 @@ execute_cmd(CLI_SESSION *cli)
                                 if (arg_list[k] == 0)
                                 {
                                     dcb_printf(dcb, "Invalid argument: %s\n", args[k + 2]);
-                                    break;
+                                    ok = false;
                                 }
                             }
 
-                            switch (cmds[i].options[j].argc_max)
+                            if (ok)
                             {
-                                case 0:
-                                    cmds[i].options[j].fn(dcb);
-                                    break;
-                                case 1:
-                                    cmds[i].options[j].fn(dcb, arg_list[0]);
-                                    break;
-                                case 2:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1]);
-                                    break;
-                                case 3:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2]);
-                                    break;
-                                case 4:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
-                                                          arg_list[3]);
-                                    break;
-                                case 5:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
-                                                          arg_list[3], arg_list[4]);
-                                    break;
-                                case 6:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
-                                                          arg_list[3], arg_list[4], arg_list[5]);
-                                    break;
-                                case 7:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
-                                                          arg_list[3], arg_list[4], arg_list[5],
-                                                          arg_list[6]);
-                                    break;
-                                case 8:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
-                                                          arg_list[3], arg_list[4], arg_list[5],
-                                                          arg_list[6], arg_list[7]);
-                                    break;
-                                case 9:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
-                                                          arg_list[3], arg_list[4], arg_list[5],
-                                                          arg_list[6], arg_list[7], arg_list[8]);
-                                    break;
-                                case 10:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
-                                                          arg_list[3], arg_list[4], arg_list[5],
-                                                          arg_list[6], arg_list[7], arg_list[8],
-                                                          arg_list[9]);
-                                    break;
-                                case 11:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
-                                                          arg_list[3], arg_list[4], arg_list[5],
-                                                          arg_list[6], arg_list[7], arg_list[8],
-                                                          arg_list[9], arg_list[10]);
-                                    break;
-                                case 12:
-                                    cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
-                                                          arg_list[3], arg_list[4], arg_list[5],
-                                                          arg_list[6], arg_list[7], arg_list[8],
-                                                          arg_list[9], arg_list[10], arg_list[11]);
-                                    break;
-                                default:
-                                    dcb_printf(dcb, "Error: Maximum argument count is %d.\n", MAXARGS);
-                                    break;
+                                switch (cmds[i].options[j].argc_max)
+                                {
+                                    case 0:
+                                        cmds[i].options[j].fn(dcb);
+                                        break;
+                                    case 1:
+                                        cmds[i].options[j].fn(dcb, arg_list[0]);
+                                        break;
+                                    case 2:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1]);
+                                        break;
+                                    case 3:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2]);
+                                        break;
+                                    case 4:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
+                                                              arg_list[3]);
+                                        break;
+                                    case 5:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
+                                                              arg_list[3], arg_list[4]);
+                                        break;
+                                    case 6:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
+                                                              arg_list[3], arg_list[4], arg_list[5]);
+                                        break;
+                                    case 7:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
+                                                              arg_list[3], arg_list[4], arg_list[5],
+                                                              arg_list[6]);
+                                        break;
+                                    case 8:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
+                                                              arg_list[3], arg_list[4], arg_list[5],
+                                                              arg_list[6], arg_list[7]);
+                                        break;
+                                    case 9:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
+                                                              arg_list[3], arg_list[4], arg_list[5],
+                                                              arg_list[6], arg_list[7], arg_list[8]);
+                                        break;
+                                    case 10:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
+                                                              arg_list[3], arg_list[4], arg_list[5],
+                                                              arg_list[6], arg_list[7], arg_list[8],
+                                                              arg_list[9]);
+                                        break;
+                                    case 11:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
+                                                              arg_list[3], arg_list[4], arg_list[5],
+                                                              arg_list[6], arg_list[7], arg_list[8],
+                                                              arg_list[9], arg_list[10]);
+                                        break;
+                                    case 12:
+                                        cmds[i].options[j].fn(dcb, arg_list[0], arg_list[1], arg_list[2],
+                                                              arg_list[3], arg_list[4], arg_list[5],
+                                                              arg_list[6], arg_list[7], arg_list[8],
+                                                              arg_list[9], arg_list[10], arg_list[11]);
+                                        break;
+                                    default:
+                                        dcb_printf(dcb, "Error: Maximum argument count is %d.\n", MAXARGS);
+                                        break;
+                                }
                             }
                         }
                     }

From 9df3f154cf95dc2fec5ea2b1471908e8234b6e8e Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Sat, 3 Dec 2016 11:31:18 +0200
Subject: [PATCH 18/25] Detect duplicate objects

Creating a monitor or a listener twice is now detected and the proper
error is printed.
---
 include/maxscale/service.h                 | 20 +++++++
 server/core/config.c                       |  4 +-
 server/core/config_runtime.c               | 70 ++++++++++++----------
 server/core/maxscale/service.h             |  4 +-
 server/core/service.c                      |  4 +-
 server/core/test/testservice.c             |  2 +-
 server/modules/routing/debugcli/debugcmd.c |  6 +-
 7 files changed, 71 insertions(+), 39 deletions(-)

diff --git a/include/maxscale/service.h b/include/maxscale/service.h
index fb0d29c78..37670a5ad 100644
--- a/include/maxscale/service.h
+++ b/include/maxscale/service.h
@@ -242,6 +242,26 @@ SERVICE* service_find(const char *name);
 // TODO: Change binlogrouter to use the functions in config_runtime.h
 void serviceAddBackend(SERVICE *service, SERVER *server);
 
+/**
+ * @brief Check if a service uses a server
+ * @param service Service to check
+ * @param server Server being used
+ * @return True if service uses the server
+ */
+bool serviceHasBackend(SERVICE *service, SERVER *server);
+
+/**
+ * @brief Check if a service has a listener
+ *
+ * @param service Service to check
+ * @param protocol Listener protocol
+ * @param address Listener address
+ * @param port Listener port
+ * @return True if service has the listener
+ */
+bool serviceHasListener(SERVICE *service, const char *protocol,
+                        const char* address, unsigned short port);
+
 int   serviceGetUser(SERVICE *service, char **user, char **auth);
 int   serviceSetUser(SERVICE *service, char *user, char *auth);
 bool  serviceSetFilters(SERVICE *service, char *filters);
diff --git a/server/core/config.c b/server/core/config.c
index 2e9009889..5ef1b9ca9 100644
--- a/server/core/config.c
+++ b/server/core/config.c
@@ -3110,7 +3110,7 @@ int create_new_listener(CONFIG_CONTEXT *obj)
             SSL_LISTENER *ssl_info = make_ssl_structure(obj, true, &error_count);
             if (socket)
             {
-                if (serviceHasProtocol(service, protocol, address, 0))
+                if (serviceHasListener(service, protocol, address, 0))
                 {
                     MXS_ERROR("Listener '%s' for service '%s' already has a socket at '%s.",
                               obj->object, service_name, socket);
@@ -3125,7 +3125,7 @@ int create_new_listener(CONFIG_CONTEXT *obj)
 
             if (port)
             {
-                if (serviceHasProtocol(service, protocol, address, atoi(port)))
+                if (serviceHasListener(service, protocol, address, atoi(port)))
                 {
                     MXS_ERROR("Listener '%s', for service '%s', already have port %s.",
                               obj->object,
diff --git a/server/core/config_runtime.c b/server/core/config_runtime.c
index c5900990a..bc02b1dac 100644
--- a/server/core/config_runtime.c
+++ b/server/core/config_runtime.c
@@ -396,8 +396,6 @@ bool runtime_create_listener(SERVICE *service, const char *name, const char *add
                              const char *ssl_cert, const char *ssl_ca,
                              const char *ssl_version, const char *ssl_depth)
 {
-    SSL_LISTENER *ssl = NULL;
-    bool rval = true;
 
     if (addr == NULL || strcasecmp(addr, "default") == 0)
     {
@@ -426,34 +424,42 @@ bool runtime_create_listener(SERVICE *service, const char *name, const char *add
 
     unsigned short u_port = atoi(port);
 
-    if (ssl_key && ssl_cert && ssl_ca)
-    {
-        ssl = create_ssl(name, ssl_key, ssl_cert, ssl_ca, ssl_version, ssl_depth);
-
-        if (ssl == NULL)
-        {
-            MXS_ERROR("SSL initialization for listener '%s' failed.", name);
-            rval = false;
-        }
-    }
-
     spinlock_acquire(&crt_lock);
 
-    if (rval)
-    {
-        const char *print_addr = addr ? addr : "0.0.0.0";
-        SERV_LISTENER *listener = serviceCreateListener(service, name, proto, addr,
-                                                        u_port, auth, auth_opt, ssl);
+    SSL_LISTENER *ssl = NULL;
+    bool rval = false;
 
-        if (listener && listener_serialize(listener) && serviceLaunchListener(service, listener))
+    if (!serviceHasListener(service, proto, addr, u_port))
+    {
+        rval = true;
+
+        if (ssl_key && ssl_cert && ssl_ca)
         {
-            MXS_NOTICE("Listener '%s' at %s:%s for service '%s' created",
-                       name, print_addr, port, service->name);
+            ssl = create_ssl(name, ssl_key, ssl_cert, ssl_ca, ssl_version, ssl_depth);
+
+            if (ssl == NULL)
+            {
+                MXS_ERROR("SSL initialization for listener '%s' failed.", name);
+                rval = false;
+            }
         }
-        else
+
+        if (rval)
         {
-            MXS_ERROR("Failed to start listener '%s' at %s:%s.", name, print_addr, port);
-            rval = false;
+            const char *print_addr = addr ? addr : "0.0.0.0";
+            SERV_LISTENER *listener = serviceCreateListener(service, name, proto, addr,
+                                                            u_port, auth, auth_opt, ssl);
+
+            if (listener && listener_serialize(listener) && serviceLaunchListener(service, listener))
+            {
+                MXS_NOTICE("Listener '%s' at %s:%s for service '%s' created",
+                           name, print_addr, port, service->name);
+            }
+            else
+            {
+                MXS_ERROR("Failed to start listener '%s' at %s:%s.", name, print_addr, port);
+                rval = false;
+            }
         }
     }
 
@@ -514,16 +520,20 @@ bool runtime_create_monitor(const char *name, const char *module)
 {
     spinlock_acquire(&crt_lock);
     bool rval = false;
-    MONITOR *monitor = monitor_alloc((char*)name, (char*)module);
 
-    if (monitor)
+    if (monitor_find(name) == NULL)
     {
-        /** Mark that this monitor was created after MaxScale was started */
-        monitor->created_online = true;
+        MONITOR *monitor = monitor_alloc((char*)name, (char*)module);
 
-        if (monitor_serialize(monitor))
+        if (monitor)
         {
-            rval = true;
+            /** Mark that this monitor was created after MaxScale was started */
+            monitor->created_online = true;
+
+            if (monitor_serialize(monitor))
+            {
+                rval = true;
+            }
         }
     }
 
diff --git a/server/core/maxscale/service.h b/server/core/maxscale/service.h
index aa17a1e22..ada5c7c7c 100644
--- a/server/core/maxscale/service.h
+++ b/server/core/maxscale/service.h
@@ -68,10 +68,8 @@ SERV_LISTENER* serviceCreateListener(SERVICE *service, const char *name,
                                      const char *protocol, const char *address,
                                      unsigned short port, const char *authenticator,
                                      const char *options, SSL_LISTENER *ssl);
-int  serviceHasProtocol(SERVICE *service, const char *protocol,
-                        const char* address, unsigned short port);
+
 void serviceRemoveBackend(SERVICE *service, const SERVER *server);
-bool serviceHasBackend(SERVICE *service, SERVER *server);
 
 /**
  * @brief Serialize a service to a file
diff --git a/server/core/service.c b/server/core/service.c
index 310fd712f..e34efc7c0 100644
--- a/server/core/service.c
+++ b/server/core/service.c
@@ -708,9 +708,9 @@ SERV_LISTENER* serviceCreateListener(SERVICE *service, const char *name, const c
  * @param protocol      The name of the protocol module
  * @param address       The address to listen on
  * @param port          The port to listen on
- * @return      TRUE if the protocol/port is already part of the service
+ * @return      True if the protocol/port is already part of the service
  */
-int serviceHasProtocol(SERVICE *service, const char *protocol,
+bool serviceHasListener(SERVICE *service, const char *protocol,
                        const char* address, unsigned short port)
 {
     SERV_LISTENER *proto;
diff --git a/server/core/test/testservice.c b/server/core/test/testservice.c
index 22eac3876..c9aa88d9d 100644
--- a/server/core/test/testservice.c
+++ b/server/core/test/testservice.c
@@ -72,7 +72,7 @@ test1()
     ss_info_dassert(serviceCreateListener(service, "TestProtocol", "testprotocol",
                                           "localhost", 9876, "MySQLAuth", NULL, NULL),
                     "Add Protocol should succeed");
-    ss_info_dassert(0 != serviceHasProtocol(service, "testprotocol", "localhost", 9876),
+    ss_info_dassert(0 != serviceHasListener(service, "testprotocol", "localhost", 9876),
                     "Service should have new protocol as requested");
 
     return 0;
diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c
index bdddbd68e..d64573b6e 100644
--- a/server/modules/routing/debugcli/debugcmd.c
+++ b/server/modules/routing/debugcli/debugcmd.c
@@ -1085,7 +1085,11 @@ static void createListener(DCB *dcb, SERVICE *service, char *name, char *address
 
 static void createMonitor(DCB *dcb, const char *name, const char *module)
 {
-    if (runtime_create_monitor(name, module))
+    if (monitor_find(name))
+    {
+        dcb_printf(dcb, "Monitor '%s' already exists\n", name);
+    }
+    else if (runtime_create_monitor(name, module))
     {
         dcb_printf(dcb, "Created monitor '%s'\n", name);
     }

From a64825c866c5b23a01e1d21039d88785e38bfe97 Mon Sep 17 00:00:00 2001
From: ekorh475 <ekorh475@users.noreply.github.com>
Date: Tue, 29 Nov 2016 13:06:56 +0200
Subject: [PATCH 19/25] Additions to QLA filter.

MXS-848 (partially). The QLA-filter now has additional options
to control the printing.
1. "append"
This toggles append-mode, where the filter opens the log files in
update mode (if file already existed) and only adds text to the end.
2. "print_service"
This toggles writing the service name onto each row. Mostly useful
with the unified_file-setting.
3. "print_session"
This toggles writing the session number onto each row. Mostly useful
with the unified_file-setting.

Also, the filter now writes a header to the beginning of the file
when creating it.

The printing has been separated to its own helper-function, in case
more accurate control will be added in the future.
---
 Documentation/Filters/Query-Log-All-Filter.md |  20 +-
 server/modules/filter/qlafilter/qlafilter.c   | 377 ++++++++++++++++--
 2 files changed, 347 insertions(+), 50 deletions(-)

diff --git a/Documentation/Filters/Query-Log-All-Filter.md b/Documentation/Filters/Query-Log-All-Filter.md
index db987c84a..fdff8d093 100644
--- a/Documentation/Filters/Query-Log-All-Filter.md
+++ b/Documentation/Filters/Query-Log-All-Filter.md
@@ -25,14 +25,18 @@ filters=MyLogFilter
 
 The QLA filter accepts the following options.
 
-|Option |Description |
-|----------|--------------------------------------------|
-|ignorecase|Use case-insensitive matching |
-|case |Use case-sensitive matching |
-|extended |Use extended regular expression syntax (ERE)|
-|session_file| Use session-specific file (default)|
-|unified_file| Use one file for all sessions|
-|flush_writes| Flush after every write|
+ Option | Description
+ -------| -----------
+ ignorecase | Use case-insensitive matching
+ case | Use case-sensitive matching
+ extended | Use extended regular expression syntax (ERE)
+ session_file | Write to session-specific files (default)
+ unified_file | Use one file for all sessions
+ flush_writes | Flush after every write
+ append | Append log entries instead of overwriting files
+ print_service | Add service name to log entries
+ print_session | Add session id to log entries (ignored for session-files)
+
 To use multiple filter options, list them in a comma-separated list. If no file settings are given, default will be used. Multiple file settings can be enabled simultaneously.
 
 ```
diff --git a/server/modules/filter/qlafilter/qlafilter.c b/server/modules/filter/qlafilter/qlafilter.c
index 7997b6ed9..a3563fcc7 100644
--- a/server/modules/filter/qlafilter/qlafilter.c
+++ b/server/modules/filter/qlafilter/qlafilter.c
@@ -48,6 +48,7 @@
 #include <string.h>
 #include <maxscale/atomic.h>
 #include <maxscale/alloc.h>
+#include <maxscale/service.h>
 
 MODULE_INFO info =
 {
@@ -59,13 +60,23 @@ MODULE_INFO info =
 
 static char *version_str = "V1.1.1";
 
-/** Formatting buffer size */
-#define QLA_STRING_BUFFER_SIZE 1024
+/** Date string buffer size */
+#define QLA_DATE_BUFFER_SIZE 20
 
-/** Log file settings flags */
+/** Log file save mode flags */
 #define CONFIG_FILE_SESSION (1 << 0) // Default value, session specific files
 #define CONFIG_FILE_UNIFIED (1 << 1) // One file shared by all sessions
 
+/* Flags for controlling extra log entry contents. The default items are
+ * always on, for now.
+ */
+#define LOG_DATA_SERVICE    (1 << 0)
+#define LOG_DATA_SESSION    (1 << 1)
+#define LOG_DATA_DATE       (1 << 2)
+#define LOG_DATA_USER       (1 << 3)
+#define LOG_DATA_QUERY      (1 << 4)
+#define LOG_DATA_DEFAULT (LOG_DATA_DATE | LOG_DATA_USER | LOG_DATA_QUERY)
+
 /*
  * The filter entry points
  */
@@ -105,17 +116,21 @@ static FILTER_OBJECT MyObject =
 typedef struct
 {
     int sessions; /* The count of sessions */
+    char *name;     /* Filter definition name */
     char *filebase; /* The filename base */
-    char *source; /* The source of the client connection */
-    char *userName; /* The user name to filter on */
+    char *source; /* The source of the client connection to filter on */
+    char *user_name; /* The user name to filter on */
     char *match; /* Optional text to match against */
     regex_t re; /* Compiled regex text */
     char *nomatch; /* Optional text to match against for exclusion */
     regex_t nore; /* Compiled regex nomatch text */
     uint32_t log_mode_flags; /* Log file mode settings */
+    uint32_t log_file_data_flags; /* What data is saved to the files */
     FILE *unified_fp; /* Unified log file. The pointer needs to be shared here
                        * to avoid garbled printing. */
-    bool flush_writes; /* Flush log file after every write */
+    bool flush_writes; /* Flush log file after every write? */
+    bool append;    /* Open files in append-mode? */
+    bool write_warning_given; /* To make sure some warning are only given once */
 } QLA_INSTANCE;
 
 /**
@@ -128,15 +143,20 @@ typedef struct
  */
 typedef struct
 {
-    DOWNSTREAM down;
-    char *filename;
-    FILE *fp;
     int active;
-    char *user;
+    DOWNSTREAM down;
+    char *filename; /* The session-specific log file name */
+    FILE *fp;       /* The session-specific log file */
     char *remote;
-    size_t ses_id; /* The session this filter serves */
+    char *service;  /* The service name this filter is attached to. Not owned. */
+    size_t ses_id;  /* The session this filter serves */
+    char *user;     /* The client */
 } QLA_SESSION;
 
+static FILE* open_log_file(uint32_t, QLA_INSTANCE *, const char *);
+static int write_log_entry(uint32_t, FILE*, QLA_INSTANCE*, QLA_SESSION*, const char*,
+                           const char*, size_t);
+
 /**
  * Implementation of the mandatory version entry point
  *
@@ -191,14 +211,18 @@ createInstance(const char *name, char **options, FILTER_PARAMETER **params)
 
     if (my_instance)
     {
-        my_instance->source = NULL;
-        my_instance->userName = NULL;
-        my_instance->match = NULL;
-        my_instance->nomatch = NULL;
+        my_instance->append = false;
         my_instance->filebase = NULL;
-        my_instance->log_mode_flags = 0;
-        my_instance->unified_fp = NULL;
         my_instance->flush_writes = false;
+        my_instance->log_file_data_flags = LOG_DATA_DEFAULT;
+        my_instance->log_mode_flags = 0;
+        my_instance->match = NULL;
+        my_instance->name = MXS_STRDUP(name);
+        my_instance->nomatch = NULL;
+        my_instance->source = NULL;
+        my_instance->unified_fp = NULL;
+        my_instance->user_name = NULL;
+        my_instance->write_warning_given = false;
         bool error = false;
 
         if (params)
@@ -219,7 +243,7 @@ createInstance(const char *name, char **options, FILTER_PARAMETER **params)
                 }
                 else if (!strcmp(params[i]->name, "user"))
                 {
-                    my_instance->userName = MXS_STRDUP_A(params[i]->value);
+                    my_instance->user_name = MXS_STRDUP_A(params[i]->value);
                 }
                 else if (!strcmp(params[i]->name, "filebase"))
                 {
@@ -264,6 +288,18 @@ createInstance(const char *name, char **options, FILTER_PARAMETER **params)
                 {
                     my_instance->flush_writes = true;
                 }
+                else if (!strcasecmp(options[i], "append"))
+                {
+                    my_instance->append = true;
+                }
+                else if (!strcasecmp(options[i], "print_service"))
+                {
+                    my_instance->log_file_data_flags |= LOG_DATA_SERVICE;
+                }
+                else if (!strcasecmp(options[i], "print_session"))
+                {
+                    my_instance->log_file_data_flags |= LOG_DATA_SESSION;
+                }
                 else
                 {
                     MXS_ERROR("qlafilter: Unsupported option '%s'.",
@@ -305,8 +341,8 @@ createInstance(const char *name, char **options, FILTER_PARAMETER **params)
             error = true;
         }
         // Try to open the unified log file
-        if (my_instance->log_mode_flags & CONFIG_FILE_UNIFIED &&
-            my_instance->filebase != NULL)
+        if (!error && (my_instance->log_mode_flags & CONFIG_FILE_UNIFIED) &&
+            (my_instance->filebase != NULL))
         {
             // First calculate filename length
             const char UNIFIED[] = ".unified";
@@ -316,7 +352,9 @@ createInstance(const char *name, char **options, FILTER_PARAMETER **params)
             {
                 snprintf(filename, namelen, "%s.unified", my_instance->filebase);
                 // Open the file. It is only closed at program exit
-                my_instance->unified_fp = fopen(filename, "w");
+                my_instance->unified_fp = open_log_file(my_instance->log_file_data_flags,
+                                                        my_instance, filename);
+
                 if (my_instance->unified_fp == NULL)
                 {
                     char errbuf[MXS_STRERROR_BUFLEN];
@@ -353,7 +391,7 @@ createInstance(const char *name, char **options, FILTER_PARAMETER **params)
             }
             MXS_FREE(my_instance->filebase);
             MXS_FREE(my_instance->source);
-            MXS_FREE(my_instance->userName);
+            MXS_FREE(my_instance->user_name);
             MXS_FREE(my_instance);
             my_instance = NULL;
         }
@@ -392,8 +430,8 @@ newSession(FILTER *instance, SESSION *session)
 
         if ((my_instance->source && remote &&
              strcmp(remote, my_instance->source)) ||
-            (my_instance->userName && userName &&
-             strcmp(userName, my_instance->userName)))
+            (my_instance->user_name && userName &&
+             strcmp(userName, my_instance->user_name)))
         {
             my_session->active = 0;
         }
@@ -401,6 +439,7 @@ newSession(FILTER *instance, SESSION *session)
         my_session->user = userName;
         my_session->remote = remote;
         my_session->ses_id = session->ses_id;
+        my_session->service = session->service->name;
 
         sprintf(my_session->filename, "%s.%lu",
                 my_instance->filebase,
@@ -410,9 +449,11 @@ newSession(FILTER *instance, SESSION *session)
         atomic_add(&(my_instance->sessions), 1);
 
         // Only open the session file if the corresponding mode setting is used
-        if (my_session->active && (my_instance->log_mode_flags | CONFIG_FILE_SESSION))
+        if (my_session->active && (my_instance->log_mode_flags & CONFIG_FILE_SESSION))
         {
-            my_session->fp = fopen(my_session->filename, "w");
+            uint32_t data_flags = (my_instance->log_file_data_flags &
+                                   ~LOG_DATA_SESSION); // No point printing "Session"
+            my_session->fp = open_log_file(data_flags, my_instance, my_session->filename);
 
             if (my_session->fp == NULL)
             {
@@ -496,21 +537,21 @@ routeQuery(FILTER *instance, void *session, GWBUF *queue)
 {
     QLA_INSTANCE *my_instance = (QLA_INSTANCE *) instance;
     QLA_SESSION *my_session = (QLA_SESSION *) session;
-    char *ptr;
+    char *ptr = NULL;
     int length = 0;
     struct tm t;
     struct timeval tv;
 
     if (my_session->active)
     {
-        if ((ptr = modutil_get_SQL(queue)) != NULL)
+        if (modutil_extract_SQL(queue, &ptr, &length))
         {
             if ((my_instance->match == NULL ||
                  regexec(&my_instance->re, ptr, 0, NULL, 0) == 0) &&
                 (my_instance->nomatch == NULL ||
                  regexec(&my_instance->nore, ptr, 0, NULL, 0) != 0))
             {
-                char buffer[QLA_STRING_BUFFER_SIZE];
+                char buffer[QLA_DATE_BUFFER_SIZE];
                 gettimeofday(&tv, NULL);
                 localtime_r(&tv.tv_sec, &t);
                 strftime(buffer, sizeof(buffer), "%F %T", &t);
@@ -519,28 +560,39 @@ routeQuery(FILTER *instance, void *session, GWBUF *queue)
                  * Loop over all the possible log file modes and write to
                  * the enabled files.
                  */
-                char *sql_string = trim(squeeze_whitespace(ptr));
+
+                char *sql_string = ptr;
+                bool write_error = false;
                 if (my_instance->log_mode_flags & CONFIG_FILE_SESSION)
                 {
-                    fprintf(my_session->fp, "%s,%s@%s,%s\n", buffer, my_session->user,
-                            my_session->remote, sql_string);
-                    if (my_instance->flush_writes)
+                    // In this case there is no need to write the session
+                    // number into the files.
+                    uint32_t data_flags = (my_instance->log_file_data_flags &
+                                           ~LOG_DATA_SESSION);
+
+                    if (write_log_entry(data_flags, my_session->fp,
+                                        my_instance, my_session, buffer, sql_string, length) < 0)
                     {
-                        fflush(my_session->fp);
+                        write_error = true;
                     }
                 }
                 if (my_instance->log_mode_flags & CONFIG_FILE_UNIFIED)
                 {
-                    fprintf(my_instance->unified_fp, "S%zd,%s,%s@%s,%s\n",
-                            my_session->ses_id, buffer, my_session->user,
-                            my_session->remote, sql_string);
-                    if (my_instance->flush_writes)
+                    uint32_t data_flags = my_instance->log_file_data_flags;
+                    if (write_log_entry(data_flags, my_instance->unified_fp,
+                                        my_instance, my_session, buffer, sql_string, length) < 0)
                     {
-                        fflush(my_instance->unified_fp);
+                        write_error = true;
                     }
                 }
+                if (write_error && !my_instance->write_warning_given)
+                {
+                    MXS_ERROR("qla-filter '%s': Log file write failed. "
+                              "Suppressing further similar warnings.",
+                              my_instance->name);
+                    my_instance->write_warning_given = true;
+                }
             }
-            MXS_FREE(ptr);
         }
     }
     /* Pass the query downstream */
@@ -575,10 +627,10 @@ diagnostic(FILTER *instance, void *fsession, DCB *dcb)
         dcb_printf(dcb, "\t\tLimit logging to connections from  %s\n",
                    my_instance->source);
     }
-    if (my_instance->userName)
+    if (my_instance->user_name)
     {
         dcb_printf(dcb, "\t\tLimit logging to user      %s\n",
-                   my_instance->userName);
+                   my_instance->user_name);
     }
     if (my_instance->match)
     {
@@ -601,3 +653,244 @@ static uint64_t getCapabilities(void)
 {
     return RCAP_TYPE_CONTIGUOUS_INPUT;
 }
+/**
+ * Open the log file and print a header if appropriate.
+ * @param   data_flags  Data save settings flags
+ * @param   instance    The filter instance
+ * @param   filename    Target file path
+ * @return  A valid file on success, null otherwise.
+ */
+static FILE* open_log_file(uint32_t data_flags, QLA_INSTANCE *instance, const char *filename)
+{
+    bool file_existed = false;
+    FILE *fp = NULL;
+    if (instance->append == false)
+    {
+        // Just open the file (possibly overwriting) and then print header.
+        fp = fopen(filename, "w");
+    }
+    else
+    {
+        // Using fopen() with 'a+' means we will always write to the end but can read
+        // anywhere. Depending on the "append"-setting the file has been
+        // opened in different modes, which should be considered if file handling
+        // changes later (e.g. rewinding).
+        if ((fp = fopen(filename, "a+")) != NULL)
+        {
+            // Check to see if file already has contents
+            fseek(fp, 0, SEEK_END);
+            if (ftell(fp) > 0) // Any errors in ftell cause overwriting
+            {
+                file_existed = true;
+            }
+        }
+    }
+    if (fp && !file_existed)
+    {
+        // Print a header. Luckily, we know the header has limited length
+        const char SERVICE[] = "Service,";
+        const char SESSION[] = "Session,";
+        const char DATE[] = "Date,";
+        const char USERHOST[] = "User@Host,";
+        const char QUERY[] = "Query,";
+
+        const int headerlen = sizeof(SERVICE) + sizeof(SERVICE) + sizeof(DATE) +
+                              sizeof(USERHOST) + sizeof(QUERY);
+
+        char print_str[headerlen];
+        memset(print_str, '\0', headerlen);
+
+        char *current_pos = print_str;
+        if (instance->log_file_data_flags & LOG_DATA_SERVICE)
+        {
+            strcat(current_pos, SERVICE);
+            current_pos += sizeof(SERVICE) - 1;
+        }
+        if (instance->log_file_data_flags & LOG_DATA_SESSION)
+        {
+            strcat(current_pos, SESSION);
+            current_pos += sizeof(SERVICE) - 1;
+        }
+        if (instance->log_file_data_flags & LOG_DATA_DATE)
+        {
+            strcat(current_pos, DATE);
+            current_pos += sizeof(DATE) - 1;
+        }
+        if (instance->log_file_data_flags & LOG_DATA_USER)
+        {
+            strcat(current_pos, USERHOST);
+            current_pos += sizeof(USERHOST) - 1;
+        }
+        if (instance->log_file_data_flags & LOG_DATA_QUERY)
+        {
+            strcat(current_pos, QUERY);
+            current_pos += sizeof(QUERY) - 1;
+        }
+        if (current_pos > print_str)
+        {
+            // Overwrite the last ','.
+            *(current_pos - 1) = '\n';
+        }
+        else
+        {
+            // Nothing to print
+            return fp;
+        }
+
+        // Finally, write the log header.
+        int written = fprintf(fp, "%s", print_str);
+
+        if ((written <= 0) ||
+            ((instance->flush_writes) && (fflush(fp) < 0)))
+        {
+            // Weird error, file opened but a write failed. Best to stop.
+            fclose(fp);
+            MXS_ERROR("qlafilter: Failed to print header to file %s.", filename);
+            return NULL;
+        }
+    }
+    return fp;
+}
+
+/**
+ * Write an entry to the log file.
+ * @param   data_flags    Controls what to write
+ * @param   logfile    Target file
+ * @param   instance    Filter instance
+ * @param   session    Filter session
+ * @param   time_string Date entry
+ * @param   sql_string SQL-query, not NULL terminated!
+ * @param   sql_str_len Length of SQL-string
+ * @return  The number of characters written, or a negative value on failure
+ */
+static int write_log_entry(uint32_t data_flags, FILE *logfile, QLA_INSTANCE *instance,
+                           QLA_SESSION *session, const char *time_string, const char *sql_string,
+                           size_t sql_str_len)
+{
+    ss_dassert(logfile != NULL);
+    size_t print_len = 0;
+
+    // First calculate an upper limit for the total length. The strlen()-calls
+    // could be removed if the values would be saved into the instance or session
+    // or if we had some reasonable max lengths. (Apparently there are max lengths
+    // but they are much higher than what is typically needed)
+
+    // The numbers have some extra for delimiters.
+    if (data_flags & LOG_DATA_SERVICE)
+    {
+        print_len += strlen(session->service) + 1;
+    }
+    if (data_flags & LOG_DATA_SESSION)
+    {
+        print_len += 20; // To print a 64bit integer
+    }
+    if (data_flags & LOG_DATA_DATE)
+    {
+        print_len += QLA_DATE_BUFFER_SIZE + 1;
+    }
+    if (data_flags & LOG_DATA_USER)
+    {
+        print_len += strlen(session->user) + strlen(session->remote) + 2;
+    }
+    if (data_flags & LOG_DATA_QUERY)
+    {
+        print_len += sql_str_len + 1; // Can't use strlen, not null-terminated
+    }
+
+    if (print_len == 0)
+    {
+        return 0; // Nothing to print
+    }
+
+    // Allocate space for a buffer. Printing to the file in parts would likely
+    // cause garbled printing if several threads write simultaneously, so we
+    // have to first print to a string.
+    char *print_str = NULL;
+    if ((print_str = MXS_CALLOC(print_len, sizeof(char))) == NULL)
+    {
+        return -1;
+    }
+
+    bool error = false;
+    char *current_pos = print_str;
+    int rval = 0;
+    if (!error && (data_flags & LOG_DATA_SERVICE))
+    {
+        if ((rval = sprintf(current_pos, "%s,", session->service)) < 0)
+        {
+            error = true;
+        }
+        else
+        {
+            current_pos += rval;
+        }
+    }
+    if (!error && (data_flags & LOG_DATA_SESSION))
+    {
+        if ((rval = sprintf(current_pos, "%lu,", session->ses_id)) < 0)
+        {
+            error = true;
+        }
+        else
+        {
+            current_pos += rval;
+        }
+    }
+    if (!error && (data_flags & LOG_DATA_DATE))
+    {
+        if ((rval = sprintf(current_pos, "%s,", time_string)) < 0)
+        {
+            error = true;
+        }
+        else
+        {
+            current_pos += rval;
+        }
+    }
+    if (!error && (data_flags & LOG_DATA_USER))
+    {
+        if ((rval = sprintf(current_pos, "%s@%s,", session->user, session->remote)) < 0)
+        {
+            error = true;
+        }
+        else
+        {
+            current_pos += rval;
+        }
+    }
+    if (!error && (data_flags & LOG_DATA_QUERY))
+    {
+        strncat(current_pos, sql_string, sql_str_len); // non-null-terminated string
+        current_pos += sql_str_len + 1; // +1 to move to the next char after
+    }
+    if (error || current_pos <= print_str)
+    {
+        MXS_FREE(print_str);
+        MXS_ERROR("qlafilter ('%s'): Failed to format log event.", instance->name);
+        return -1;
+    }
+    else
+    {
+        // Overwrite the last ','. The rest is already filled with 0.
+        *(current_pos - 1) = '\n';
+    }
+
+    // Finally, write the log event.
+    int written = fprintf(logfile, "%s", print_str);
+    MXS_FREE(print_str);
+
+    if ((!instance->flush_writes) || (written <= 0))
+    {
+        return written;
+    }
+    else
+    {
+        // Try flushing. If successful, still return the characters written.
+        int rval = fflush(logfile);
+        if (rval >= 0)
+        {
+            return written;
+        }
+        return rval;
+    }
+}

From 6d7e419ed40aa07f93ef0bef9f9e921d05ba1111 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Sat, 3 Dec 2016 16:54:44 +0200
Subject: [PATCH 20/25] Improve `--config-check` mode

The configuration checking now detects bad router options. This allows for
better coverage of the configuration with only the --config-check flag.
---
 include/maxscale/config.h |  1 +
 server/core/config.c      |  1 +
 server/core/gateway.cc    | 44 ++++++++++++++++++++++-----------------
 server/core/service.c     | 10 ++++++++-
 4 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/include/maxscale/config.h b/include/maxscale/config.h
index afb1a0029..da440953a 100644
--- a/include/maxscale/config.h
+++ b/include/maxscale/config.h
@@ -110,6 +110,7 @@ typedef struct config_context
  */
 typedef struct
 {
+    bool          config_check;                        /**< Only check config */
     int           n_threads;                           /**< Number of polling threads */
     char          *version_string;                     /**< The version string of embedded db library */
     char          release_string[_RELEASE_STR_LENGTH]; /**< The release name string of the system */
diff --git a/server/core/config.c b/server/core/config.c
index 5ef1b9ca9..84434519b 100644
--- a/server/core/config.c
+++ b/server/core/config.c
@@ -1561,6 +1561,7 @@ global_defaults()
 {
     uint8_t mac_addr[6] = "";
     struct utsname uname_data;
+    gateway.config_check = false;
     gateway.n_threads = DEFAULT_NTHREADS;
     gateway.n_nbpoll = DEFAULT_NBPOLLS;
     gateway.pollsleep = DEFAULT_POLLSLEEP;
diff --git a/server/core/gateway.cc b/server/core/gateway.cc
index f2808c328..1d9f34d44 100644
--- a/server/core/gateway.cc
+++ b/server/core/gateway.cc
@@ -1865,12 +1865,7 @@ int main(int argc, char **argv)
         goto return_main;
     }
 
-    if (config_check)
-    {
-        MXS_NOTICE("Configuration was successfully verified.");
-        rc = MAXSCALE_SHUTDOWN;
-        goto return_main;
-    }
+    cnf->config_check = config_check;
 
     if (mysql_library_init(0, NULL, NULL))
     {
@@ -1918,21 +1913,24 @@ int main(int argc, char **argv)
     }
     libmysql_initialized = TRUE;
 
-    /** Check if a MaxScale process is already running */
-    if (pid_file_exists())
+    if (!config_check)
     {
-        /** There is a process with the PID of the maxscale.pid file running.
-         * Assuming that this is an already running MaxScale process, we
-         * should exit with an error code.  */
-        rc = MAXSCALE_ALREADYRUNNING;
-        goto return_main;
-    }
+        /** Check if a MaxScale process is already running */
+        if (pid_file_exists())
+        {
+            /** There is a process with the PID of the maxscale.pid file running.
+             * Assuming that this is an already running MaxScale process, we
+             * should exit with an error code.  */
+            rc = MAXSCALE_ALREADYRUNNING;
+            goto return_main;
+        }
 
-    /* Write process pid into MaxScale pidfile */
-    if (write_pid_file() != 0)
-    {
-        rc = MAXSCALE_ALREADYRUNNING;
-        goto return_main;
+        /* Write process pid into MaxScale pidfile */
+        if (write_pid_file() != 0)
+        {
+            rc = MAXSCALE_ALREADYRUNNING;
+            goto return_main;
+        }
     }
 
     /** Initialize statistics */
@@ -1961,6 +1959,14 @@ int main(int argc, char **argv)
         rc = MAXSCALE_NOSERVICES;
         goto return_main;
     }
+
+    if (config_check)
+    {
+        MXS_NOTICE("Configuration was successfully verified.");
+        rc = MAXSCALE_SHUTDOWN;
+        goto return_main;
+    }
+
     /*<
      * Start periodic log flusher thread.
      */
diff --git a/server/core/service.c b/server/core/service.c
index e34efc7c0..452b597ec 100644
--- a/server/core/service.c
+++ b/server/core/service.c
@@ -478,7 +478,15 @@ int serviceInitialize(SERVICE *service)
 
     if ((service->router_instance = service->router->createInstance(service, router_options)))
     {
-        listeners = serviceStartAllPorts(service);
+        if (!config_get_global_options()->config_check)
+        {
+            listeners = serviceStartAllPorts(service);
+        }
+        else
+        {
+            /** We're only checking that the configuration is valid */
+            listeners++;
+        }
     }
     else
     {

From 360d7d53b8a14909f7826772e4c826c13cfb32ff Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Sun, 4 Dec 2016 03:48:06 +0200
Subject: [PATCH 21/25] Fix `call command` error message

The error messages referred to functions and printed duplicate
information.
---
 server/modules/routing/debugcli/debugcmd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c
index d64573b6e..215658151 100644
--- a/server/modules/routing/debugcli/debugcmd.c
+++ b/server/modules/routing/debugcli/debugcmd.c
@@ -1426,18 +1426,18 @@ static void callModuleCommand(DCB *dcb, char *domain, char *id, char *v3,
         {
             if (!modulecmd_call_command(cmd, arg))
             {
-                dcb_printf(dcb, "Failed to call function: %s\n", modulecmd_get_error());
+                dcb_printf(dcb, "Error: %s\n", modulecmd_get_error());
             }
             modulecmd_arg_free(arg);
         }
         else
         {
-            dcb_printf(dcb, "Failed to parse arguments: %s\n", modulecmd_get_error());
+            dcb_printf(dcb, "Error: %s\n", modulecmd_get_error());
         }
     }
     else
     {
-        dcb_printf(dcb, "Function not found: %s\n", modulecmd_get_error());
+        dcb_printf(dcb, "Error: %s\n", modulecmd_get_error());
     }
 }
 

From d97e3587e0eacb0c759e84d22fcb88793de0838f Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Mon, 5 Dec 2016 12:50:37 +0200
Subject: [PATCH 22/25] MXS-878: Don't log an error for 10.1 slave connections

The error message was not needed as we know that the query is always
executed by 10.1 slaves.
---
 server/modules/routing/binlogrouter/blr_slave.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/server/modules/routing/binlogrouter/blr_slave.c b/server/modules/routing/binlogrouter/blr_slave.c
index c49b047a6..9de254e0c 100644
--- a/server/modules/routing/binlogrouter/blr_slave.c
+++ b/server/modules/routing/binlogrouter/blr_slave.c
@@ -344,6 +344,7 @@ blr_slave_query(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue)
     int query_len;
     char *ptr;
     extern char *strcasestr();
+    bool unexpected = true;
 
     qtext = (char*)GWBUF_DATA(queue);
     query_len = extract_field((uint8_t *)qtext, 24) - 1;
@@ -530,6 +531,10 @@ blr_slave_query(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue)
 
             return blr_slave_send_var_value(router, slave, heading, server_id, BLR_TYPE_INT);
         }
+        else if (strcasestr(word, "binlog_gtid_pos"))
+        {
+            unexpected = false;
+        }
     }
     else if (strcasecmp(word, "SHOW") == 0)
     {
@@ -1118,7 +1123,17 @@ blr_slave_query(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue)
     MXS_FREE(query_text);
 
     query_text = strndup(qtext, query_len);
-    MXS_ERROR("Unexpected query from '%s'@'%s': %s", slave->dcb->user, slave->dcb->remote, query_text);
+
+    if (unexpected)
+    {
+        MXS_ERROR("Unexpected query from '%s'@'%s': %s", slave->dcb->user, slave->dcb->remote, query_text);
+    }
+    else
+    {
+        MXS_INFO("Unexpected query from '%s'@'%s', possibly a 10.1 slave: %s",
+                 slave->dcb->user, slave->dcb->remote, query_text);
+    }
+
     MXS_FREE(query_text);
     blr_slave_send_error(router, slave,
                          "You have an error in your SQL syntax; Check the syntax "

From 83109a6e9e5df877cb833e4149668fd44953767b Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Sun, 4 Dec 2016 03:48:53 +0200
Subject: [PATCH 23/25] Use protocol module to process packets

The backend protocol module can be requested to provide complete and
contiguous packets to the router module. This removes the need to process
the packets in binlogrouter.
---
 server/modules/routing/binlogrouter/blr.c     |   2 +-
 .../modules/routing/binlogrouter/blr_master.c | 120 ++----------------
 2 files changed, 11 insertions(+), 111 deletions(-)

diff --git a/server/modules/routing/binlogrouter/blr.c b/server/modules/routing/binlogrouter/blr.c
index 25b4db681..2ad65d887 100644
--- a/server/modules/routing/binlogrouter/blr.c
+++ b/server/modules/routing/binlogrouter/blr.c
@@ -1771,7 +1771,7 @@ static void rses_end_locked_router_action(ROUTER_SLAVE *rses)
 
 static uint64_t getCapabilities(void)
 {
-    return RCAP_TYPE_NO_RSESSION;
+    return RCAP_TYPE_NO_RSESSION | RCAP_TYPE_CONTIGUOUS_OUTPUT;
 }
 
 /**
diff --git a/server/modules/routing/binlogrouter/blr_master.c b/server/modules/routing/binlogrouter/blr_master.c
index 7b0acc816..7599277ba 100644
--- a/server/modules/routing/binlogrouter/blr_master.c
+++ b/server/modules/routing/binlogrouter/blr_master.c
@@ -995,12 +995,10 @@ encode_value(unsigned char *data, unsigned int value, int len)
 void
 blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
 {
-    uint8_t *msg = NULL, *ptr, *pdata;
+    uint8_t *msg = NULL, *ptr;
     REP_HEADER hdr;
-    unsigned int len = 0, reslen;
+    unsigned int len = 0;
     unsigned int pkt_length;
-    int no_residual = 1;
-    int preslen = -1;
     int prev_length = -1;
     int n_bufs = -1, pn_bufs = -1;
     int check_packet_len;
@@ -1015,7 +1013,6 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
     {
         pkt = gwbuf_append(router->residual, pkt);
         router->residual = NULL;
-        no_residual = 0;
     }
 
     pkt_length = gwbuf_length(pkt);
@@ -1026,103 +1023,8 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
      */
     while (pkt && pkt_length > 24)
     {
-        reslen = GWBUF_LENGTH(pkt);
-        pdata = GWBUF_DATA(pkt);
-        if (reslen < 3) // Payload length straddles buffers
-        {
-            /* Get the length of the packet from the residual and new packet */
-            if (reslen >= 3)
-            {
-                len = EXTRACT24(pdata);
-            }
-            else if (reslen == 2)
-            {
-                len = EXTRACT16(pdata);
-                len |= (*(uint8_t *)GWBUF_DATA(pkt->next) << 16);
-            }
-            else if (reslen == 1)
-            {
-                len = *pdata;
-                len |= (EXTRACT16(GWBUF_DATA(pkt->next)) << 8);
-            }
-            len += 4;   // Allow space for the header
-        }
-        else
-        {
-            len = EXTRACT24(pdata) + 4;
-        }
-        /* len is now the payload length for the packet we are working on */
-
-        if (reslen < len && pkt_length >= len)
-        {
-            /*
-             * The message is contained in more than the current
-             * buffer, however we have the complete messasge in
-             * this buffer and the chain of remaining buffers.
-             *
-             * Allocate a contiguous buffer for the binlog message
-             * and copy the complete message into this buffer.
-             */
-            int msg_remainder = len;
-            GWBUF *p = pkt;
-
-            if ((msg = MXS_MALLOC(len)) == NULL)
-            {
-                break;
-            }
-
-            n_bufs = 0;
-            ptr = msg;
-            while (p && msg_remainder > 0)
-            {
-                int plen = GWBUF_LENGTH(p);
-                int n = (msg_remainder > plen ? plen : msg_remainder);
-                memcpy(ptr, GWBUF_DATA(p), n);
-                msg_remainder -= n;
-                ptr += n;
-                if (msg_remainder > 0)
-                {
-                    p = p->next;
-                }
-                n_bufs++;
-            }
-            if (msg_remainder)
-            {
-                MXS_ERROR("Expected entire message in buffer "
-                          "chain, but failed to create complete "
-                          "message as expected. %s @ %lu",
-                          router->binlog_name,
-                          router->current_pos);
-                MXS_FREE(msg);
-                /* msg = NULL; Not needed unless msg will be referred to again */
-                break;
-            }
-
-            ptr = msg;
-        }
-        else if (reslen < len)
-        {
-            /*
-             * The message is not fully contained in the current
-             * and we do not have the complete message in the
-             * buffer chain. Therefore we must stop processing
-             * until we receive the next buffer.
-             */
-            router->stats.n_residuals++;
-            MXS_DEBUG("Residual data left after %lu records. %s @ %lu",
-                      router->stats.n_binlogs,
-                      router->binlog_name, router->current_pos);
-            break;
-        }
-        else
-        {
-            /*
-             * The message is fully contained in the current buffer
-             */
-            ptr = pdata;
-            n_bufs = 1;
-        }
-
+        ptr = GWBUF_DATA(pkt);
+        len = gw_mysql_get_byte3(ptr);
         semisync_bytes = 0;
 
         /*
@@ -1133,6 +1035,8 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
 
         if (len < BINLOG_EVENT_HDR_LEN && router->master_event_state != BLR_EVENT_ONGOING)
         {
+            // Dead code?
+
             char *event_msg = "";
 
             /* Packet is too small to be a binlog event */
@@ -1189,16 +1093,12 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                         (hdr.event_size + (check_packet_len - MYSQL_HEADER_LEN)) < MYSQL_PACKET_LENGTH_MAX)
                     {
                         MXS_ERROR("Packet length is %d, but event size is %d, "
-                                  "binlog file %s position %lu "
-                                  "reslen is %d and preslen is %d, "
-                                  "length of previous event %d. %s",
+                                  "binlog file %s position %lu, "
+                                  "length of previous event %d.",
                                   len, hdr.event_size,
                                   router->binlog_name,
                                   router->current_pos,
-                                  reslen, preslen, prev_length,
-                                  (prev_length == -1 ?
-                                   (no_residual ? "No residual data from previous call" :
-                                    "Residual data from previous call") : ""));
+                                  prev_length);
 
                         blr_log_packet(LOG_ERR, "Packet:", ptr, len);
 
@@ -1774,7 +1674,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
             len -= n;
             pkt_length -= n;
         }
-        preslen = reslen;
+
         pn_bufs = n_bufs;
     }
 

From 2f082cb7fba4f9d64d67a6a4435e0a46c03e690a Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Sun, 4 Dec 2016 13:51:56 +0200
Subject: [PATCH 24/25] Store large events in memory

Storing the large events in memory allows checksum calculations to be done
in one step. This also makes the encryption of events easier as they
require the complete event in memory.
---
 server/modules/routing/binlogrouter/blr.c     |   8 -
 server/modules/routing/binlogrouter/blr.h     |  14 +-
 .../modules/routing/binlogrouter/blr_master.c | 389 +++++-------------
 .../modules/routing/binlogrouter/blr_slave.c  |   7 -
 4 files changed, 95 insertions(+), 323 deletions(-)

diff --git a/server/modules/routing/binlogrouter/blr.c b/server/modules/routing/binlogrouter/blr.c
index 2ad65d887..e15632af3 100644
--- a/server/modules/routing/binlogrouter/blr.c
+++ b/server/modules/routing/binlogrouter/blr.c
@@ -530,7 +530,6 @@ createInstance(SERVICE *service, char **options)
     inst->reconnect_pending = 0;
     inst->handling_threads = 0;
     inst->rotating = 0;
-    inst->residual = NULL;
     inst->slaves = NULL;
     inst->next = NULL;
     inst->lastEventTimestamp = 0;
@@ -2334,13 +2333,6 @@ destroyInstance(ROUTER *instance)
         }
     }
 
-    /* Discard the queued residual data */
-    while (inst->residual)
-    {
-        inst->residual = gwbuf_consume(inst->residual, GWBUF_LENGTH(inst->residual));
-    }
-    inst->residual = NULL;
-
     MXS_INFO("%s is being stopped by MaxScale shudown. Disconnecting from master %s:%d, "
                "read up to log %s, pos %lu, transaction safe pos %lu",
                inst->service->name,
diff --git a/server/modules/routing/binlogrouter/blr.h b/server/modules/routing/binlogrouter/blr.h
index 576542ff4..7e21c2c2c 100644
--- a/server/modules/routing/binlogrouter/blr.h
+++ b/server/modules/routing/binlogrouter/blr.h
@@ -231,12 +231,9 @@ static const char BLR_DBUSERS_FILE[] = "dbusers";
 /** Possible states of an event sent by the master */
 enum blr_event_state
 {
-    BLR_EVENT_DONE, /*< No event being processed  */
-    BLR_EVENT_STARTED, /*< The first packet of an event which spans multiple packets
-                        * has been received */
+    BLR_EVENT_STARTED, /*< The first packet of an event has been received */
     BLR_EVENT_ONGOING, /*< Other packets of a multi-packet event are being processed */
-    BLR_EVENT_COMPLETE /*< A multi-packet event has been successfully processed
-                        * but the router is not yet ready to process another one */
+    BLR_EVENT_DONE, /*< The complete event was received */
 };
 
 /* Master Server configuration struct */
@@ -490,19 +487,14 @@ typedef struct router_instance
     unsigned int            master_state;   /*< State of the master FSM */
     uint8_t                 lastEventReceived; /*< Last even received */
     uint32_t                lastEventTimestamp; /*< Timestamp from last event */
-    GWBUF                   *residual;      /*< Any residual binlog event */
     MASTER_RESPONSES        saved_master;   /*< Saved master responses */
     char                    *binlogdir;     /*< The directory with the binlog files */
     SPINLOCK                binlog_lock;    /*< Lock to control update of the binlog position */
     int                     trx_safe;       /*< Detect and handle partial transactions */
     int                     pending_transaction; /*< Pending transaction */
     enum blr_event_state    master_event_state; /*< Packet read state */
-    uint32_t                stored_checksum; /*< The current value of the checksum */
-    uint8_t                 partial_checksum[MYSQL_CHECKSUM_LEN]; /*< The partial value of the checksum
-                                                   * received from the master */
-    uint8_t                 partial_checksum_bytes; /*< How many bytes of the checksum we have read  */
-    uint64_t                checksum_size; /*< Data size for the checksum */
     REP_HEADER              stored_header; /*< Relication header of the event the master is sending */
+    GWBUF                  *stored_event; /*< Partial even buffer */
     uint64_t                last_safe_pos; /* last committed transaction */
     char                    binlog_name[BINLOG_FNAMELEN + 1];
     /*< Name of the current binlog file */
diff --git a/server/modules/routing/binlogrouter/blr_master.c b/server/modules/routing/binlogrouter/blr_master.c
index 7599277ba..c5464af80 100644
--- a/server/modules/routing/binlogrouter/blr_master.c
+++ b/server/modules/routing/binlogrouter/blr_master.c
@@ -102,8 +102,6 @@ static void blr_extract_header_semisync(uint8_t *pkt, REP_HEADER *hdr);
 static int blr_send_semisync_ack (ROUTER_INSTANCE *router, uint64_t pos);
 static int blr_get_master_semisync(GWBUF *buf);
 
-int blr_write_data_into_binlog(ROUTER_INSTANCE *router, uint32_t data_len, uint8_t *buf);
-void extract_checksum(ROUTER_INSTANCE* router, uint8_t *cksumptr, uint8_t len);
 static void blr_terminate_master_replication(ROUTER_INSTANCE *router, uint8_t* ptr, int len);
 void blr_notify_all_slaves(ROUTER_INSTANCE *router);
 extern bool blr_notify_waiting_slave(ROUTER_SLAVE *slave);
@@ -166,13 +164,6 @@ blr_start_master(void* data)
     }
     router->master_state = BLRM_CONNECTING;
 
-    /* Discard the queued residual data */
-    while (router->residual)
-    {
-        router->residual = gwbuf_consume(router->residual, GWBUF_LENGTH(router->residual));
-    }
-    router->residual = NULL;
-
     spinlock_release(&router->lock);
     if ((client = dcb_alloc(DCB_ROLE_INTERNAL, NULL)) == NULL)
     {
@@ -240,13 +231,6 @@ blr_restart_master(ROUTER_INSTANCE *router)
 {
     dcb_close(router->client);
 
-    /* Discard the queued residual data */
-    while (router->residual)
-    {
-        router->residual = gwbuf_consume(router->residual, GWBUF_LENGTH(router->residual));
-    }
-    router->residual = NULL;
-
     /* Now it is safe to unleash other threads on this router instance */
     spinlock_acquire(&router->lock);
     router->reconnect_pending = 0;
@@ -333,6 +317,8 @@ blr_master_close(ROUTER_INSTANCE *router)
     dcb_close(router->master);
     router->master_state = BLRM_UNCONNECTED;
     router->master_event_state = BLR_EVENT_DONE;
+    gwbuf_free(router->stored_event);
+    router->stored_event = NULL;
 }
 
 /**
@@ -985,6 +971,31 @@ encode_value(unsigned char *data, unsigned int value, int len)
     }
 }
 
+/**
+ * Check that the stored event checksum matches the calculated checksum
+ */
+static bool verify_checksum(ROUTER_INSTANCE *router, size_t len, uint8_t *ptr)
+{
+    bool rval = true;
+    uint32_t offset = MYSQL_HEADER_LEN + 1;
+    uint32_t size = len - (offset + MYSQL_CHECKSUM_LEN);
+
+    uint32_t checksum = crc32(0L, ptr + offset, size);
+    uint32_t pktsum = EXTRACT32(ptr + offset + size);
+
+    if (pktsum != checksum)
+    {
+        rval = false;
+        MXS_ERROR("%s: Checksum error in event from master, "
+                  "binlog %s @ %lu. Closing master connection.",
+                  router->service->name, router->binlog_name,
+                  router->current_pos);
+        router->stats.n_badcrc++;
+    }
+
+    return rval;
+}
+
 /**
  * blr_handle_binlog_record - we have received binlog records from
  * the master and we must now work out what to do with them.
@@ -998,30 +1009,18 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
     uint8_t *msg = NULL, *ptr;
     REP_HEADER hdr;
     unsigned int len = 0;
-    unsigned int pkt_length;
     int prev_length = -1;
     int n_bufs = -1, pn_bufs = -1;
     int check_packet_len;
     int semisync_bytes;
     int semi_sync_send_ack = 0;
 
-    /*
-     * Prepend any residual buffer to the buffer chain we have
-     * been called with.
-     */
-    if (router->residual)
-    {
-        pkt = gwbuf_append(router->residual, pkt);
-        router->residual = NULL;
-    }
-
-    pkt_length = gwbuf_length(pkt);
     /*
      * Loop over all the packets while we still have some data
      * and the packet length is enough to hold a replication event
      * header.
      */
-    while (pkt && pkt_length > 24)
+    while (pkt)
     {
         ptr = GWBUF_DATA(pkt);
         len = gw_mysql_get_byte3(ptr);
@@ -1035,9 +1034,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
 
         if (len < BINLOG_EVENT_HDR_LEN && router->master_event_state != BLR_EVENT_ONGOING)
         {
-            // Dead code?
-
-            char *event_msg = "";
+            char *event_msg = "unknown";
 
             /* Packet is too small to be a binlog event */
             if (ptr[4] == 0xfe) /* EOF Packet */
@@ -1049,6 +1046,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                 event_msg = "error";
             }
             MXS_NOTICE("Non-event message (%s) from master.", event_msg);
+            pkt = gwbuf_consume(pkt, len);
         }
         else
         {
@@ -1089,7 +1087,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                 /* Sanity check */
                 if (hdr.ok == 0)
                 {
-                    if (hdr.event_size != len - check_packet_len &&
+                    if (hdr.event_size != len - (check_packet_len - MYSQL_HEADER_LEN) &&
                         (hdr.event_size + (check_packet_len - MYSQL_HEADER_LEN)) < MYSQL_PACKET_LENGTH_MAX)
                     {
                         MXS_ERROR("Packet length is %d, but event size is %d, "
@@ -1114,18 +1112,10 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
 
                         break;
                     }
-                    else if (hdr.event_size + (check_packet_len - MYSQL_HEADER_LEN) >= MYSQL_PACKET_LENGTH_MAX)
-                    {
-                        router->master_event_state = BLR_EVENT_STARTED;
 
-                        /** Store the header for later use */
-                        memcpy(&router->stored_header, &hdr, sizeof(hdr));
-                    }
-
-                    /** Prepare the checksum variables for this event */
-                    router->stored_checksum = crc32(0L, NULL, 0);
-                    router->checksum_size = hdr.event_size - MYSQL_CHECKSUM_LEN;
-                    router->partial_checksum_bytes = 0;
+                    /** Store the header for later use */
+                    memcpy(&router->stored_header, &hdr, sizeof(hdr));
+                    router->master_event_state = BLR_EVENT_STARTED;
                 }
                 else
                 {
@@ -1134,7 +1124,6 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
 
                     gwbuf_free(pkt);
                     pkt = NULL;
-                    pkt_length = 0;
 
                     break;
                 }
@@ -1166,125 +1155,63 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                 }
             }
 
-            /* pending large event */
-            if (router->master_event_state != BLR_EVENT_DONE)
+            /** Gather the event into one big buffer */
+            GWBUF *part = gwbuf_split(&pkt, len + MYSQL_HEADER_LEN);
+
+            if (semisync_bytes)
             {
-                if (len - MYSQL_HEADER_LEN < MYSQL_PACKET_LENGTH_MAX)
+                /** Consume the two semi-sync bytes */
+                part = gwbuf_consume(part, semisync_bytes);
+            }
+
+            ss_dassert(router->master_event_state == BLR_EVENT_STARTED ||
+                       router->master_event_state == BLR_EVENT_ONGOING);
+
+            if (router->master_event_state == BLR_EVENT_ONGOING)
+            {
+                /** Only consume the network header */
+                part = gwbuf_consume(part, MYSQL_HEADER_LEN);
+            }
+
+            router->stored_event = gwbuf_append(router->stored_event, part);
+
+            if (len < MYSQL_PACKET_LENGTH_MAX)
+            {
+                /** This is the last packet, we can now write it to disk */
+                ss_dassert(router->master_event_state != BLR_EVENT_DONE);
+
+                if (router->master_event_state != BLR_EVENT_STARTED)
                 {
-                    /** This is the last packet, we can now proceed to distribute
-                     * the event afer it has been written to disk */
-                    ss_dassert(router->master_event_state != BLR_EVENT_COMPLETE);
-                    router->master_event_state = BLR_EVENT_COMPLETE;
+                    /** This is not the first packet of the event, copy the
+                     * stored header */
                     memcpy(&hdr, &router->stored_header, sizeof(hdr));
                 }
-                else
-                {
-                    /* current partial event is being written to disk file */
-                    uint32_t offset = MYSQL_HEADER_LEN;
-                    uint32_t extra_bytes = MYSQL_HEADER_LEN;
-
-                    /** Don't write the OK byte into the binlog */
-                    if (router->master_event_state == BLR_EVENT_STARTED)
-                    {
-                        offset = MYSQL_HEADER_LEN + 1;
-                        router->master_event_state = BLR_EVENT_ONGOING;
-                        extra_bytes = MYSQL_HEADER_LEN + 1;
-                    }
-
-                    ss_dassert(len - extra_bytes - semisync_bytes > 0);
-                    uint32_t bytes_available = len - extra_bytes - semisync_bytes;
-
-                    if (router->master_chksum)
-                    {
-                        uint32_t size = MXS_MIN(len - extra_bytes - semisync_bytes,
-                                            router->checksum_size);
-
-                        router->stored_checksum = crc32(router->stored_checksum,
-                                                        ptr + offset,
-                                                        size);
-                        router->checksum_size -= size;
-
-                        if (router->checksum_size == 0 && size < bytes_available)
-                        {
-                            extract_checksum(router, ptr + offset + size,
-                                             bytes_available - size);
-                        }
-                    }
-
-                    if (blr_write_data_into_binlog(router, bytes_available,
-                                                   ptr + offset) == 0)
-                    {
-                        /** Failed to write to the binlog file, destroy the buffer
-                         * chain and close the connection with the master */
-                        while (pkt)
-                        {
-                            pkt = GWBUF_CONSUME_ALL(pkt);
-                        }
-                        blr_master_close(router);
-                        blr_master_delayed_connect(router);
-                        return;
-                    }
-                    pkt = gwbuf_consume(pkt, len);
-                    pkt_length -= len;
-                    continue;
-                }
+                router->master_event_state = BLR_EVENT_DONE;
             }
+            else
+            {
+                router->master_event_state = BLR_EVENT_ONGOING;
+                continue;
+            }
+
+            /** We have the complete event in one contiguous buffer */
+            router->stored_event = gwbuf_make_contiguous(router->stored_event);
+            ptr = GWBUF_DATA(router->stored_event);
+
+            /** len is now the length of the complete event plus 4 bytes of network
+             * header and one OK byte. Semi-sync bytes are never stored. */
+            len = gwbuf_length(router->stored_event);
 
             /*
              * First check that the checksum we calculate matches the
              * checksum in the packet we received.
              */
-            if (router->master_chksum)
+            if (router->master_chksum && !verify_checksum(router, len, ptr))
             {
-                uint32_t offset = MYSQL_HEADER_LEN;
-                uint32_t size = len - MYSQL_HEADER_LEN - MYSQL_CHECKSUM_LEN;
-
-                if (router->master_event_state == BLR_EVENT_DONE)
-                {
-                    /** Set the pointer offset to the first byte after
-                     * the header and OK byte */
-                    offset = MYSQL_HEADER_LEN + 1;
-                    size = len - (check_packet_len + MYSQL_CHECKSUM_LEN);
-                }
-
-                size = MXS_MIN(size, router->checksum_size);
-
-                if (router->checksum_size > 0)
-                {
-                    router->stored_checksum = crc32(router->stored_checksum,
-                                                    ptr + offset, size);
-                    router->checksum_size -= size;
-                }
-
-                if(router->checksum_size == 0 && size < (len - offset - semisync_bytes))
-                {
-                    extract_checksum(router, ptr + offset + size,
-                                     len - offset - size - semisync_bytes);
-                }
-
-                if (router->partial_checksum_bytes == MYSQL_CHECKSUM_LEN)
-                {
-                    uint32_t pktsum = EXTRACT32(router->partial_checksum);
-                    if (pktsum != router->stored_checksum)
-                    {
-                        router->stats.n_badcrc++;
-                        MXS_FREE(msg);
-                        /* msg = NULL; Not needed unless msg will be referred to again */
-                        MXS_ERROR("%s: Checksum error in event from master, "
-                                  "binlog %s @ %lu. Closing master connection.",
-                                  router->service->name, router->binlog_name,
-                                  router->current_pos);
-                        blr_master_close(router);
-                        blr_master_delayed_connect(router);
-                        return;
-                    }
-                }
-                else
-                {
-                    pkt = gwbuf_consume(pkt, len);
-                    pkt_length -= len;
-                    continue;
-                }
+                MXS_FREE(msg);
+                blr_master_close(router);
+                blr_master_delayed_connect(router);
+                return;
             }
 
             if (hdr.ok == 0)
@@ -1479,15 +1406,8 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                     }
                     else if (hdr.flags != LOG_EVENT_ARTIFICIAL_F)
                     {
-                        ptr = ptr + 4; // Skip header
-                        uint32_t offset = 4;
-
-                        if (router->master_event_state == BLR_EVENT_STARTED ||
-                            router->master_event_state == BLR_EVENT_DONE)
-                        {
-                            ptr++;
-                            offset++;
-                        }
+                        ptr = ptr + MYSQL_HEADER_LEN + 1; // Skip header and OK byte
+                        uint32_t offset = MYSQL_HEADER_LEN + 1;
 
                         if (hdr.event_type == ROTATE_EVENT)
                         {
@@ -1496,23 +1416,10 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                             spinlock_release(&router->binlog_lock);
                         }
 
-                        /* Current event is being written to disk file.
-                         * It is possible for an empty packet to be sent if an
-                         * event is exactly 2^24 bytes long. In this case the
-                         * empty packet should be discarded. */
-                        if (len > MYSQL_HEADER_LEN &&
-                            blr_write_binlog_record(router, &hdr, len - offset - semisync_bytes, ptr) == 0)
+                        /* Write event to disk */
+                        if (blr_write_binlog_record(router, &hdr, len - offset, ptr) == 0)
                         {
-                            /*
-                             * Failed to write to the
-                             * binlog file, destroy the
-                             * buffer chain and close the
-                             * connection with the master
-                             */
-                            while ((pkt = gwbuf_consume(pkt, GWBUF_LENGTH(pkt))) != NULL)
-                            {
-                                ;
-                            }
+                            gwbuf_free(pkt);
                             blr_master_close(router);
                             blr_master_delayed_connect(router);
                             return;
@@ -1523,16 +1430,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                         {
                             if (!blr_rotate_event(router, ptr, &hdr))
                             {
-                                /*
-                                 * Failed to write to the
-                                 * binlog file, destroy the
-                                 * buffer chain and close the
-                                 * connection with the master
-                                 */
-                                while ((pkt = gwbuf_consume(pkt, GWBUF_LENGTH(pkt))) != NULL)
-                                {
-                                    ;
-                                }
+                                gwbuf_free(pkt);
                                 blr_master_close(router);
                                 blr_master_delayed_connect(router);
                                 return;
@@ -1542,8 +1440,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                         /* Handle semi-sync request fom master */
                         if (router->master_semi_sync != MASTER_SEMISYNC_NOT_AVAILABLE &&
                             semi_sync_send_ack == BLR_MASTER_SEMI_SYNC_ACK_REQ &&
-                            (router->master_event_state == BLR_EVENT_COMPLETE ||
-                            router->master_event_state == BLR_EVENT_DONE))
+                            (router->master_event_state == BLR_EVENT_DONE))
                         {
 
                             MXS_DEBUG("%s: binlog record in file %s, pos %lu has "
@@ -1629,16 +1526,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                             spinlock_release(&router->binlog_lock);
                             if (!blr_rotate_event(router, ptr, &hdr))
                             {
-                                /*
-                                 * Failed to write to the
-                                 * binlog file, destroy the
-                                 * buffer chain and close the
-                                 * connection with the master
-                                 */
-                                while ((pkt = gwbuf_consume(pkt, GWBUF_LENGTH(pkt))) != NULL)
-                                {
-                                    ;
-                                }
+                                gwbuf_free(pkt);
                                 blr_master_close(router);
                                 blr_master_delayed_connect(router);
                                 return;
@@ -1646,17 +1534,15 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                         }
                     }
                 }
-
-                /** A large event is now fully received and processed */
-                if (router->master_event_state == BLR_EVENT_COMPLETE)
-                {
-                    router->master_event_state = BLR_EVENT_DONE;
-                }
             }
             else
             {
                 blr_terminate_master_replication(router, ptr, len);
             }
+
+            /** Finished processing the event */
+            gwbuf_free(router->stored_event);
+            router->stored_event = NULL;
         }
 
         if (msg)
@@ -1664,33 +1550,8 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
             MXS_FREE(msg);
             msg = NULL;
         }
-        prev_length = len;
-        while (len > 0)
-        {
-            unsigned int n, plen;
-            plen = GWBUF_LENGTH(pkt);
-            n = (plen < len ? plen : len);
-            pkt = gwbuf_consume(pkt, n);
-            len -= n;
-            pkt_length -= n;
-        }
-
-        pn_bufs = n_bufs;
     }
 
-    /*
-     * Check if we have a residual, part binlog message to deal with.
-     * Just simply store the GWBUF for next time
-     */
-    if (pkt)
-    {
-        router->residual = pkt;
-        ss_dassert(pkt_length != 0);
-    }
-    else
-    {
-        ss_dassert(pkt_length == 0);
-    }
     blr_file_flush(router);
 }
 
@@ -2129,13 +1990,6 @@ blr_stop_start_master(ROUTER_INSTANCE *router)
         }
     }
 
-    /* Discard the queued residual data */
-    while (router->residual)
-    {
-        router->residual = gwbuf_consume(router->residual, GWBUF_LENGTH(router->residual));
-    }
-    router->residual = NULL;
-
     router->master_state = BLRM_UNCONNECTED;
     spinlock_release(&router->lock);
 
@@ -2298,45 +2152,6 @@ static void blr_log_identity(ROUTER_INSTANCE *router)
     }
 }
 
-/**
- * @brief Write data into binlogs (incomplete event)
- *
- * Writes @c data_len bytes of data from @c buf into the current binlog being processed.
- *
- * @param router Router instance
- * @param data_len Number of bytes to write
- * @param buf Pointer where the data is read
- * @return Number of bytes written or 0 on error
- */
-int
-blr_write_data_into_binlog(ROUTER_INSTANCE *router, uint32_t data_len, uint8_t *buf)
-{
-    int n;
-
-    if ((n = pwrite(router->binlog_fd, buf, data_len,
-                    router->last_written)) != data_len)
-    {
-        char err_msg[MXS_STRERROR_BUFLEN];
-        MXS_ERROR("%s: Failed to write binlog record at %lu of %s, %s. "
-                  "Truncating to previous record.",
-                  router->service->name, router->last_written,
-                  router->binlog_name,
-                  strerror_r(errno, err_msg, sizeof(err_msg)));
-
-        /* Remove any partial event that was written */
-        if (ftruncate(router->binlog_fd, router->last_written))
-        {
-            MXS_ERROR("%s: Failed to truncate binlog record at %lu of %s, %s. ",
-                      router->service->name, router->last_written,
-                      router->binlog_name,
-                      strerror_r(errno, err_msg, sizeof(err_msg)));
-        }
-        return 0;
-    }
-    router->last_written += data_len;
-    return n;
-}
-
 /**
  * Send a replication event packet to a slave
  *
@@ -2485,26 +2300,6 @@ bool blr_send_event(blr_thread_role_t role,
     return rval;
 }
 
-/**
- * Extract the checksum from the binlogs
- *
- * This updates the internal state of the router and will allow us to detect
- * if the checksum is split across two packets.
- * @param router Router instance
- * @param cksumptr Pointer to the checksum
- * @param len How much of the data is readable
- */
-void extract_checksum(ROUTER_INSTANCE* router, uint8_t *cksumptr, uint8_t len)
-{
-    uint8_t *ptr = cksumptr;
-    while (ptr - cksumptr < len && router->partial_checksum_bytes < MYSQL_CHECKSUM_LEN)
-    {
-        router->partial_checksum[router->partial_checksum_bytes] = *ptr;
-        ptr++;
-        router->partial_checksum_bytes++;
-    }
-}
-
 /**
  * Stop the slave connection and log errors
  *
diff --git a/server/modules/routing/binlogrouter/blr_slave.c b/server/modules/routing/binlogrouter/blr_slave.c
index 9de254e0c..bf4763fd9 100644
--- a/server/modules/routing/binlogrouter/blr_slave.c
+++ b/server/modules/routing/binlogrouter/blr_slave.c
@@ -3332,13 +3332,6 @@ blr_stop_slave(ROUTER_INSTANCE* router, ROUTER_SLAVE* slave)
         }
     }
 
-    /* Discard the queued residual data */
-    while (router->residual)
-    {
-        router->residual = gwbuf_consume(router->residual, GWBUF_LENGTH(router->residual));
-    }
-    router->residual = NULL;
-
     /* Now it is safe to unleash other threads on this router instance */
     router->reconnect_pending = 0;
     router->active_logs = 0;

From f5eb4e21dc8a9c9bacc3228a13e13d332bbf5465 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Mon, 5 Dec 2016 15:35:29 +0200
Subject: [PATCH 25/25] Add more comments and clean up code

The binlog event processing code is now better commented and is slightly
easier to read.
---
 server/modules/routing/binlogrouter/blr.h     |   2 +-
 .../modules/routing/binlogrouter/blr_master.c | 126 ++++++++++++------
 2 files changed, 84 insertions(+), 44 deletions(-)

diff --git a/server/modules/routing/binlogrouter/blr.h b/server/modules/routing/binlogrouter/blr.h
index 7e21c2c2c..4b0f294b2 100644
--- a/server/modules/routing/binlogrouter/blr.h
+++ b/server/modules/routing/binlogrouter/blr.h
@@ -494,7 +494,7 @@ typedef struct router_instance
     int                     pending_transaction; /*< Pending transaction */
     enum blr_event_state    master_event_state; /*< Packet read state */
     REP_HEADER              stored_header; /*< Relication header of the event the master is sending */
-    GWBUF                  *stored_event; /*< Partial even buffer */
+    GWBUF                  *stored_event; /*< Buffer where partial events are stored */
     uint64_t                last_safe_pos; /* last committed transaction */
     char                    binlog_name[BINLOG_FNAMELEN + 1];
     /*< Name of the current binlog file */
diff --git a/server/modules/routing/binlogrouter/blr_master.c b/server/modules/routing/binlogrouter/blr_master.c
index c5464af80..f6f3e52a4 100644
--- a/server/modules/routing/binlogrouter/blr_master.c
+++ b/server/modules/routing/binlogrouter/blr_master.c
@@ -996,6 +996,38 @@ static bool verify_checksum(ROUTER_INSTANCE *router, size_t len, uint8_t *ptr)
     return rval;
 }
 
+/**
+ * @brief Reset router errors
+ *
+ * @param router Router instance
+ * @param hdr Replication header
+ */
+static void reset_errors(ROUTER_INSTANCE *router, REP_HEADER *hdr)
+{
+    spinlock_acquire(&router->lock);
+
+    /* set mysql errno to 0 */
+    router->m_errno = 0;
+
+    /* Remove error message */
+    if (router->m_errmsg)
+    {
+        MXS_FREE(router->m_errmsg);
+    }
+    router->m_errmsg = NULL;
+
+    spinlock_release(&router->lock);
+#ifdef SHOW_EVENTS
+    printf("blr: len %lu, event type 0x%02x, flags 0x%04x, "
+           "event size %d, event timestamp %lu\n",
+           (unsigned long)len - 4,
+           hdr->event_type,
+           hdr->flags,
+           hdr->event_size,
+           (unsigned long)hdr->timestamp);
+#endif
+}
+
 /**
  * blr_handle_binlog_record - we have received binlog records from
  * the master and we must now work out what to do with them.
@@ -1052,6 +1084,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
         {
             if (router->master_event_state == BLR_EVENT_DONE)
             {
+                /** This is the start of a new event */
                 spinlock_acquire(&router->lock);
                 router->stats.n_binlogs++;
                 router->stats.n_binlogs_ses++;
@@ -1113,9 +1146,12 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                         break;
                     }
 
-                    /** Store the header for later use */
-                    memcpy(&router->stored_header, &hdr, sizeof(hdr));
+                    /** This is the first (and possibly last) packet of a replication
+                     * event. We store the header in case the event is large and
+                     * it is transmitted over multiple network packets.  */
                     router->master_event_state = BLR_EVENT_STARTED;
+                    memcpy(&router->stored_header, &hdr, sizeof(hdr));
+                    reset_errors(router, &hdr);
                 }
                 else
                 {
@@ -1127,32 +1163,11 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
 
                     break;
                 }
-
-                if (hdr.ok == 0)
-                {
-                    spinlock_acquire(&router->lock);
-
-                    /* set mysql errno to 0 */
-                    router->m_errno = 0;
-
-                    /* Remove error message */
-                    if (router->m_errmsg)
-                    {
-                        MXS_FREE(router->m_errmsg);
-                    }
-                    router->m_errmsg = NULL;
-
-                    spinlock_release(&router->lock);
-#ifdef SHOW_EVENTS
-                    printf("blr: len %lu, event type 0x%02x, flags 0x%04x, "
-                           "event size %d, event timestamp %lu\n",
-                           (unsigned long)len - 4,
-                           hdr.event_type,
-                           hdr.flags,
-                           hdr.event_size,
-                           (unsigned long)hdr.timestamp);
-#endif
-                }
+            }
+            else
+            {
+                /** We're processing a multi-packet replication event */
+                ss_dassert(router->master_event_state == BLR_EVENT_ONGOING);
             }
 
             /** Gather the event into one big buffer */
@@ -1169,7 +1184,11 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
 
             if (router->master_event_state == BLR_EVENT_ONGOING)
             {
-                /** Only consume the network header */
+                /**
+                 * Consume the network header so that we can append the raw
+                 * event data to the original buffer. This allows both checksum
+                 * calculations and encryption to process it as a contiguous event
+                 */
                 part = gwbuf_consume(part, MYSQL_HEADER_LEN);
             }
 
@@ -1177,24 +1196,43 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
 
             if (len < MYSQL_PACKET_LENGTH_MAX)
             {
-                /** This is the last packet, we can now write it to disk */
+                /**
+                 * This is either the only packet for the event or the last
+                 * packet in a series for this event. The buffer now contains
+                 * the network header of the first packet (4 bytes) and one OK byte.
+                 * The semi-sync bytes are always consumed at an earlier stage.
+                 */
                 ss_dassert(router->master_event_state != BLR_EVENT_DONE);
 
                 if (router->master_event_state != BLR_EVENT_STARTED)
                 {
-                    /** This is not the first packet of the event, copy the
-                     * stored header */
+                    /**
+                     * This is not the first packet for this event. We must use
+                     * the stored header.
+                     */
                     memcpy(&hdr, &router->stored_header, sizeof(hdr));
                 }
+
+                /** The event is now complete */
                 router->master_event_state = BLR_EVENT_DONE;
             }
             else
             {
+                /**
+                 * This packet is a part of a series of packets that contain an
+                 * event larger than MYSQL_PACKET_LENGTH_MAX bytes.
+                 *
+                 * For each partial event chunk, we remove the network header and
+                 * append it to router->stored_event. The first event is an
+                 * exception to this and it is appended as-is with the network
+                 * header and the extra OK byte.
+                 */
+                ss_dassert(len == MYSQL_PACKET_LENGTH_MAX);
                 router->master_event_state = BLR_EVENT_ONGOING;
                 continue;
             }
 
-            /** We have the complete event in one contiguous buffer */
+            /** We now have the complete event in one contiguous buffer */
             router->stored_event = gwbuf_make_contiguous(router->stored_event);
             ptr = GWBUF_DATA(router->stored_event);
 
@@ -1202,9 +1240,9 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
              * header and one OK byte. Semi-sync bytes are never stored. */
             len = gwbuf_length(router->stored_event);
 
-            /*
-             * First check that the checksum we calculate matches the
-             * checksum in the packet we received.
+            /**
+             * If checksums are enabled, verify that the stored checksum
+             * matches the one we calculated
              */
             if (router->master_chksum && !verify_checksum(router, len, ptr))
             {
@@ -1406,9 +1444,6 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                     }
                     else if (hdr.flags != LOG_EVENT_ARTIFICIAL_F)
                     {
-                        ptr = ptr + MYSQL_HEADER_LEN + 1; // Skip header and OK byte
-                        uint32_t offset = MYSQL_HEADER_LEN + 1;
-
                         if (hdr.event_type == ROTATE_EVENT)
                         {
                             spinlock_acquire(&router->binlog_lock);
@@ -1416,8 +1451,13 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                             spinlock_release(&router->binlog_lock);
                         }
 
-                        /* Write event to disk */
-                        if (blr_write_binlog_record(router, &hdr, len - offset, ptr) == 0)
+                        uint32_t offset = MYSQL_HEADER_LEN + 1; // Skip header and OK byte
+
+                        /**
+                         * Write the raw event data to disk without the network
+                         * header or the OK byte
+                         */
+                        if (blr_write_binlog_record(router, &hdr, len - offset, ptr + offset) == 0)
                         {
                             gwbuf_free(pkt);
                             blr_master_close(router);
@@ -1425,7 +1465,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                             return;
                         }
 
-                        /* Check for rotete event */
+                        /* Check for rotate event */
                         if (hdr.event_type == ROTATE_EVENT)
                         {
                             if (!blr_rotate_event(router, ptr, &hdr))
@@ -1437,7 +1477,7 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt)
                             }
                         }
 
-                        /* Handle semi-sync request fom master */
+                        /* Handle semi-sync request from master */
                         if (router->master_semi_sync != MASTER_SEMISYNC_NOT_AVAILABLE &&
                             semi_sync_send_ack == BLR_MASTER_SEMI_SYNC_ACK_REQ &&
                             (router->master_event_state == BLR_EVENT_DONE))