Merge branch '2.2' into 2.2-mrm

Documentation/Filters/Cache.md

@@ -10,7 +10,7 @@ The cache filter is a simple cache that is capable of caching the result of
 SELECTs, so that subsequent identical SELECTs are served directly by MaxScale,
 without the queries being routed to any server.
 
-The cache will be used and populated in the following circumstances:
+By _default_ the cache will be used and populated in the following circumstances:
 * There is _no_ explicit transaction active, that is, _autocommit_ is used,
 * there is an _explicitly_ read-only transaction (that is,`START TRANSACTION
   READ ONLY`) active, or
@@ -22,6 +22,13 @@ been started with `BEGIN`, `START TRANSACTION` or `START TRANSACTION READ
 WRITE`, then the cache will be used and populated until the first `UPDATE`,
 `INSERT` or `DELETE` statement is encountered.
 
+That is, in default mode the cache effectively causes the system to behave
+as if the _isolation level_ would be `READ COMMITTED`, irrespective of what
+the isolation level of the backends actually is.
+
+The default behaviour can be altered using the configuration parameter
+[cache_inside_transactions](#cache_inside_transactions).
+
 By default, it is *ensured* that the cache is **not** used in the following
 circumstances:
 
@@ -251,6 +258,32 @@ If `assume_cacheable` is specified, then all `SELECT` statements are
 assumed to be cacheable and will be parsed *only* if some specific rule
 requires that.
 
+#### `cache_inside_transactions`
+
+An enumeration option specifying how the cache should behave when there
+are active transactions:
+
+   * `never`: When there is an active transaction, no data will be returned
+     from the cache, but all requests will always be sent to the backend.
+     The cache will be populated inside _explicitly_ read-only transactions.
+     Inside transactions that are not explicitly read-only, the cache will
+     be populated _until_ the first non-SELECT statement.
+   * `read_only_transactions`: The cache will be used and populated inside
+     _explicitly_ read-only transactions. Inside transactions that are not
+     explicitly read-only, the cache will be populated, but not used
+     _until_ the first non-SELECT statement.
+   * `all_transactions`: The cache will be used and populated inside
+     _explicitly_ read-only transactions. Inside transactions that are not
+     explicitly read-only, the cache will be used and populated _until_ the
+     first non-SELECT statement.
+```
+cache_inside_transactions=never
+```
+Default is `all_transactions`.
+
+The values `read_only_transactions` and `all_transactions` have roughly the
+same effect as changing the isolation level of the backend to `read_committed`.
+
 #### `debug`
 
 An integer value, using which the level of debug logging made by the cache

Documentation/Release-Notes/MaxScale-2.1.10-Release-Notes.md

@@ -40,6 +40,7 @@ To enable this functionality, add `query_retries=<number-of-retries>` under the
 
 [Here is a list of bugs fixed in MaxScale 2.1.10.](https://jira.mariadb.org/issues/?jql=project%20%3D%20MXS%20AND%20issuetype%20%3D%20Bug%20AND%20status%20%3D%20Closed%20AND%20fixVersion%20%3D%202.1.10)
 
+* [MXS-1497](https://jira.mariadb.org/browse/MXS-1497) Don't skip events with LOG_EVENT_IGNORABLE_F flag
 * [MXS-1468](https://jira.mariadb.org/browse/MXS-1468) Using dynamic commands to create readwritesplit configs fail after restart
 * [MXS-1459](https://jira.mariadb.org/browse/MXS-1459) Binlog checksum default value is wrong if a slave connects with checksum = NONE before master registration or master is not accessible at startup
 * [MXS-1457](https://jira.mariadb.org/browse/MXS-1457) Deleted servers are not ignored when users are loaded

Documentation/Release-Notes/MaxScale-2.2.1-Release-Notes.md

@@ -46,7 +46,7 @@ then if the first character of a value in the configuration file is a `$`
 then everything following that is interpreted as an environment variable
 and the configuration value is replaced with the value of the environment
 variable. For more information please consult the
-[Configuration Guide](Getting-Started/Configuration-Guide.md).
+[Configuration Guide](../Getting-Started/Configuration-Guide.md).
 
 ## Bug fixes
 

Documentation/Tutorials/Replication-Proxy-Binlog-Router-Tutorial.md

@@ -269,12 +269,14 @@ Gtid_IO_Pos: 0-10116-196
 
 # Binlog router compatibility
 
-Binlog Router Plugin is compatible with MariaDB 5.5 and MySQL 5.6, the current default.
+Binlog Router Plugin is compatible with MariaDB 5.5 and MySQL 5.6/5.7.
 
-In order to use it with MySQL 5.6, the *GTID_MODE* setting must be OFF and connecting
+In order to use it with MySQL 5.6/5.7, the *GTID_MODE* setting must be OFF
-slaves must not use *MASTER_AUTO_POSITION = 1* option.
+and connecting slaves must not use *MASTER_AUTO_POSITION = 1* option.
+Additionally with MySQL 5.7 slaves the `send_slave_heartbeat` option must be set to on.
 
-It also works with a MariaDB 10.X setup (master and slaves).
+It’s also works with a MariaDB 10.X setup (master and slaves).
+Slave connection must not include any GTID feature if MaxScale version is less than 2.2.
 
 Starting from MaxScale 2.2.1 the slave connections might optionally include
 **GTID** feature `MASTER_USE_GTID=Slave_pos`: only option *mariadb10-compatibility* is required.
@@ -293,6 +295,8 @@ all operations. All slave servers must use the same replication domain as the ma
 **Note:** Binlog Router currently does not work for MySQL 5.5 due to
 missing *@@global.binlog_checksum* variable.
 
+The default compatibility is MariaDB 10 since MaxScale 2.2.
+
 # Master server setup/change
 
 In the MariaDB MaxScale ini file the server section for master is no longer required, same for *servers=master_server* in the service section. The master server setup is currently managed via *CHANGE MASTER TO* command issued in MySQL client connection to MariaDB MaxScale or by providing a proper *master.ini* file in the *binlogdir*.

server/modules/filter/cache/cachefilter.cc

@@ -138,6 +138,15 @@ static const MXS_ENUM_VALUE parameter_selects_values[] =
     {NULL}
 };
 
+// Enumeration values for `cache_in_transaction`
+static const MXS_ENUM_VALUE parameter_cache_in_trxs_values[] =
+{
+    {"never",                  CACHE_IN_TRXS_NEVER},
+    {"read_only_transactions", CACHE_IN_TRXS_READ_ONLY},
+    {"all_transactions",       CACHE_IN_TRXS_ALL},
+    {NULL}
+};
+
 extern "C" MXS_MODULE* MXS_CREATE_MODULE()
 {
     static modulecmd_arg_type_t show_argv[] =
@@ -227,6 +236,13 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE()
                 MXS_MODULE_OPT_NONE,
                 parameter_selects_values
             },
+            {
+                "cache_in_transactions",
+                MXS_MODULE_PARAM_ENUM,
+                CACHE_DEFAULT_CACHE_IN_TRXS,
+                MXS_MODULE_OPT_NONE,
+                parameter_cache_in_trxs_values
+            },
             {MXS_END_MODULE_PARAMS}
         }
     };
@@ -332,6 +348,9 @@ bool CacheFilter::process_params(char **pzOptions, MXS_CONFIG_PARAMETER *ppParam
     config.selects = static_cast<cache_selects_t>(config_get_enum(ppParams,
                                                                   "selects",
                                                                   parameter_selects_values));
+    config.cache_in_trxs = static_cast<cache_in_trxs_t>(config_get_enum(ppParams,
+                                                                        "cache_in_transactions",
+                                                                        parameter_cache_in_trxs_values));
 
     if (!config.storage)
     {

server/modules/filter/cache/cachefilter.h

@@ -58,6 +58,8 @@
 #define CACHE_DEFAULT_SELECTS            "verify_cacheable"
 // Storage
 #define CACHE_DEFAULT_STORAGE            "storage_inmemory"
+// Transaction behaviour
+#define CACHE_DEFAULT_CACHE_IN_TRXS      "all_transactions"
 
 typedef enum cache_selects
 {
@@ -65,6 +67,14 @@ typedef enum cache_selects
     CACHE_SELECTS_VERIFY_CACHEABLE,
 } cache_selects_t;
 
+typedef enum cache_in_trxs
+{
+    // Do NOT change the order. Code relies upon NEVER < READ_ONLY < ALL.
+    CACHE_IN_TRXS_NEVER,
+    CACHE_IN_TRXS_READ_ONLY,
+    CACHE_IN_TRXS_ALL,
+} cache_in_trxs_t;
+
 typedef struct cache_config
 {
     uint64_t max_resultset_rows;       /**< The maximum number of rows of a resultset for it to be cached. */
@@ -81,4 +91,5 @@ typedef struct cache_config
     uint32_t debug;                    /**< Debug settings. */
     cache_thread_model_t thread_model; /**< Thread model. */
     cache_selects_t selects;           /**< Assume/verify that selects are cacheable. */
+    cache_in_trxs_t cache_in_trxs;     /**< To cache or not to cache inside transactions. */
 } CACHE_CONFIG;

server/modules/filter/cache/cachefiltersession.cc

@@ -239,12 +239,12 @@ int CacheFilterSession::routeQuery(GWBUF* pPacket)
     ss_dassert(GWBUF_LENGTH(pPacket) >= MYSQL_HEADER_LEN + 1);
     ss_dassert(MYSQL_GET_PAYLOAD_LEN(pData) + MYSQL_HEADER_LEN == GWBUF_LENGTH(pPacket));
 
-    bool fetch_from_server = true;
+    routing_action_t action = ROUTING_CONTINUE;
 
     reset_response_state();
     m_state = CACHE_IGNORING_RESPONSE;
 
-    int rv;
+    int rv = 1;
 
     switch ((int)MYSQL_GET_COMMAND(pData))
     {
@@ -287,113 +287,14 @@ int CacheFilterSession::routeQuery(GWBUF* pPacket)
         break;
 
     case MXS_COM_QUERY:
-        if (should_consult_cache(pPacket))
+        action = route_COM_QUERY(pPacket);
-        {
-            if (m_pCache->should_store(m_zDefaultDb, pPacket))
-            {
-                cache_result_t result = m_pCache->get_key(m_zDefaultDb, pPacket, &m_key);
-
-                if (CACHE_RESULT_IS_OK(result))
-                {
-                    if (m_pCache->should_use(m_pSession))
-                    {
-                        uint32_t flags = CACHE_FLAGS_INCLUDE_STALE;
-                        GWBUF* pResponse;
-                        result = m_pCache->get_value(m_key, flags, &pResponse);
-
-                        if (CACHE_RESULT_IS_OK(result))
-                        {
-                            if (CACHE_RESULT_IS_STALE(result))
-                            {
-                                // The value was found, but it was stale. Now we need to
-                                // figure out whether somebody else is already fetching it.
-
-                                if (m_pCache->must_refresh(m_key, this))
-                                {
-                                    // We were the first ones who hit the stale item. It's
-                                    // our responsibility now to fetch it.
-                                    if (log_decisions())
-                                    {
-                                        MXS_NOTICE("Cache data is stale, fetching fresh from server.");
-                                    }
-
-                                    // As we don't use the response it must be freed.
-                                    gwbuf_free(pResponse);
-
-                                    m_refreshing = true;
-                                    fetch_from_server = true;
-                                }
-                                else
-                                {
-                                    // Somebody is already fetching the new value. So, let's
-                                    // use the stale value. No point in hitting the server twice.
-                                    if (log_decisions())
-                                    {
-                                        MXS_NOTICE("Cache data is stale but returning it, fresh "
-                                                   "data is being fetched already.");
-                                    }
-                                    fetch_from_server = false;
-                                }
-                            }
-                            else
-                            {
-                                if (log_decisions())
-                                {
-                                    MXS_NOTICE("Using fresh data from cache.");
-                                }
-                                fetch_from_server = false;
-                            }
-                        }
-                        else
-                        {
-                            fetch_from_server = true;
-                        }
-
-                        if (fetch_from_server)
-                        {
-                            m_state = CACHE_EXPECTING_RESPONSE;
-                        }
-                        else
-                        {
-                            m_state = CACHE_EXPECTING_NOTHING;
-                            gwbuf_free(pPacket);
-                            DCB *dcb = m_pSession->client_dcb;
-
-                            // TODO: This is not ok. Any filters before this filter, will not
-                            // TODO: see this data.
-                            rv = dcb->func.write(dcb, pResponse);
-                        }
-                    }
-                    else
-                    {
-                        // We will not use any value in the cache, but we will update
-                        // the existing value.
-                        if (log_decisions())
-                        {
-                            MXS_NOTICE("Unconditionally fetching data from the server, "
-                                       "refreshing cache entry.");
-                        }
-                        m_state = CACHE_EXPECTING_RESPONSE;
-                    }
-                }
-                else
-                {
-                    MXS_ERROR("Could not create cache key.");
-                    m_state = CACHE_IGNORING_RESPONSE;
-                }
-            }
-            else
-            {
-                m_state = CACHE_IGNORING_RESPONSE;
-            }
-        }
         break;
 
     default:
         break;
     }
 
-    if (fetch_from_server)
+    if (action == ROUTING_CONTINUE)
     {
         rv = m_down.routeQuery(pPacket);
     }
@@ -839,15 +740,16 @@ void CacheFilterSession::store_result()
  *
  * @param pParam The GWBUF being handled.
  *
- * @return True, if the cache should be consulted, false otherwise.
+ * @return Enum value indicating appropriate action.
  */
-bool CacheFilterSession::should_consult_cache(GWBUF* pPacket)
+CacheFilterSession::cache_action_t CacheFilterSession::get_cache_action(GWBUF* pPacket)
 {
-    bool consult_cache = false;
+    cache_action_t action = CACHE_IGNORE;
 
     uint32_t type_mask = qc_get_trx_type_mask(pPacket); // Note, only trx-related type mask
 
     const char* zReason = NULL;
+    const CACHE_CONFIG& config = m_pCache->config();
 
     if (qc_query_is_type(type_mask, QUERY_TYPE_BEGIN_TRX))
     {
@@ -865,23 +767,55 @@ bool CacheFilterSession::should_consult_cache(GWBUF* pPacket)
         {
             zReason = "no transaction";
         }
-        consult_cache = true;
+        action = CACHE_USE_AND_POPULATE;
     }
     else if (session_trx_is_read_only(m_pSession))
     {
-        if (log_decisions())
+        if (config.cache_in_trxs >= CACHE_IN_TRXS_READ_ONLY)
         {
-            zReason = "explicitly read-only transaction";
+            if (log_decisions())
+            {
+                zReason = "explicitly read-only transaction";
+            }
+            action = CACHE_USE_AND_POPULATE;
+        }
+        else
+        {
+            ss_dassert(config.cache_in_trxs == CACHE_IN_TRXS_NEVER);
+
+            if (log_decisions())
+            {
+                zReason = "populating but not using cache inside read-only transactions";
+            }
+            action = CACHE_POPULATE;
         }
-        consult_cache = true;
     }
     else if (m_is_read_only)
     {
-        if (log_decisions())
+        // There is a transaction and it is *not* explicitly read-only,
+        // although so far there has only been SELECTs.
+
+        if (config.cache_in_trxs >= CACHE_IN_TRXS_ALL)
         {
-            zReason = "ordinary transaction that has so far been read-only";
+            if (log_decisions())
+            {
+                zReason = "ordinary transaction that has so far been read-only";
+            }
+            action = CACHE_USE_AND_POPULATE;
+        }
+        else
+        {
+            ss_dassert((config.cache_in_trxs == CACHE_IN_TRXS_NEVER) ||
+                       (config.cache_in_trxs == CACHE_IN_TRXS_READ_ONLY));
+
+            if (log_decisions())
+            {
+                zReason =
+                    "populating but not using cache inside transaction that is not "
+                    "explicitly read-only, but that has used only SELECTs sofar";
+            }
+            action = CACHE_POPULATE;
         }
-        consult_cache = true;
     }
     else
     {
@@ -891,11 +825,11 @@ bool CacheFilterSession::should_consult_cache(GWBUF* pPacket)
         }
     }
 
-    if (consult_cache)
+    if (action != CACHE_IGNORE)
     {
         if (is_select_statement(pPacket))
         {
-            if (m_pCache->config().selects == CACHE_SELECTS_VERIFY_CACHEABLE)
+            if (config.selects == CACHE_SELECTS_VERIFY_CACHEABLE)
             {
                 // Note that the type mask must be obtained a new. A few lines
                 // above we only got the transaction state related type mask.
@@ -903,22 +837,22 @@ bool CacheFilterSession::should_consult_cache(GWBUF* pPacket)
 
                 if (qc_query_is_type(type_mask, QUERY_TYPE_USERVAR_READ))
                 {
-                    consult_cache = false;
+                    action = CACHE_IGNORE;
                     zReason = "user variables are read";
                 }
                 else if (qc_query_is_type(type_mask, QUERY_TYPE_SYSVAR_READ))
                 {
-                    consult_cache = false;
+                    action = CACHE_IGNORE;
                     zReason = "system variables are read";
                 }
                 else if (uses_non_cacheable_function(pPacket))
                 {
-                    consult_cache = false;
+                    action = CACHE_IGNORE;
                     zReason = "uses non-cacheable function";
                 }
                 else if (uses_non_cacheable_variable(pPacket))
                 {
-                    consult_cache = false;
+                    action = CACHE_IGNORE;
                     zReason = "uses non-cacheable variable";
                 }
             }
@@ -927,9 +861,11 @@ bool CacheFilterSession::should_consult_cache(GWBUF* pPacket)
         {
             // A bit broad, as e.g. SHOW will cause the read only state to be turned
             // off. However, during normal use this will always be an UPDATE, INSERT
-            // or DELETE.
+            // or DELETE. Note that 'm_is_read_only' only affects transactions that
+            // are not explicitly read-only.
             m_is_read_only = false;
-            consult_cache = false;
+
+            action = CACHE_IGNORE;
             zReason = "statement is not SELECT";
         }
     }
@@ -955,11 +891,154 @@ bool CacheFilterSession::should_consult_cache(GWBUF* pPacket)
             length = max_length - 3; // strlen("...");
         }
 
-        const char* zDecision = (consult_cache ? "CONSULT" : "IGNORE ");
+        const char* zDecision = (action == CACHE_IGNORE) ? "IGNORE" : "CONSULT";
 
         ss_dassert(zReason);
         MXS_NOTICE(zFormat, zDecision, length, pSql, zReason);
     }
 
-    return consult_cache;
+    return action;
+}
+
+/**
+ * Routes a COM_QUERY packet.
+ *
+ * @param pPacket  A contiguous COM_QUERY packet.
+ *
+ * @return ROUTING_ABORT if the processing of the packet should be aborted
+ *         (as the data is obtained from the cache) or
+ *         ROUTING_CONTINUE if the normal processing should continue.
+ */
+CacheFilterSession::routing_action_t CacheFilterSession::route_COM_QUERY(GWBUF* pPacket)
+{
+    ss_debug(uint8_t* pData = static_cast<uint8_t*>(GWBUF_DATA(pPacket)));
+    ss_dassert((int)MYSQL_GET_COMMAND(pData) == MXS_COM_QUERY);
+
+    routing_action_t routing_action = ROUTING_CONTINUE;
+    cache_action_t cache_action = get_cache_action(pPacket);
+
+    if (cache_action != CACHE_IGNORE)
+    {
+        if (m_pCache->should_store(m_zDefaultDb, pPacket))
+        {
+            cache_result_t result = m_pCache->get_key(m_zDefaultDb, pPacket, &m_key);
+
+            if (CACHE_RESULT_IS_OK(result))
+            {
+                routing_action = route_SELECT(cache_action, pPacket);
+            }
+            else
+            {
+                MXS_ERROR("Could not create cache key.");
+                m_state = CACHE_IGNORING_RESPONSE;
+            }
+        }
+        else
+        {
+            m_state = CACHE_IGNORING_RESPONSE;
+        }
+    }
+
+    return routing_action;
+}
+
+
+/**
+ * Routes a SELECT packet.
+ *
+ * @param cache_action  The desired action.
+ * @param pPacket       A contiguous COM_QUERY packet containing a SELECT.
+ *
+ * @return ROUTING_ABORT if the processing of the packet should be aborted
+ *         (as the data is obtained from the cache) or
+ *         ROUTING_CONTINUE if the normal processing should continue.
+ */
+CacheFilterSession::routing_action_t CacheFilterSession::route_SELECT(cache_action_t cache_action, GWBUF* pPacket)
+{
+    routing_action_t routing_action = ROUTING_CONTINUE;
+
+    if (should_use(cache_action) && m_pCache->should_use(m_pSession))
+    {
+        uint32_t flags = CACHE_FLAGS_INCLUDE_STALE;
+        GWBUF* pResponse;
+        cache_result_t result = m_pCache->get_value(m_key, flags, &pResponse);
+
+        if (CACHE_RESULT_IS_OK(result))
+        {
+            if (CACHE_RESULT_IS_STALE(result))
+            {
+                // The value was found, but it was stale. Now we need to
+                // figure out whether somebody else is already fetching it.
+
+                if (m_pCache->must_refresh(m_key, this))
+                {
+                    // We were the first ones who hit the stale item. It's
+                    // our responsibility now to fetch it.
+                    if (log_decisions())
+                    {
+                        MXS_NOTICE("Cache data is stale, fetching fresh from server.");
+                    }
+
+                    // As we don't use the response it must be freed.
+                    gwbuf_free(pResponse);
+
+                    m_refreshing = true;
+                    routing_action = ROUTING_CONTINUE;
+                }
+                else
+                {
+                    // Somebody is already fetching the new value. So, let's
+                    // use the stale value. No point in hitting the server twice.
+                    if (log_decisions())
+                    {
+                        MXS_NOTICE("Cache data is stale but returning it, fresh "
+                                   "data is being fetched already.");
+                    }
+                    routing_action = ROUTING_ABORT;
+                }
+            }
+            else
+            {
+                if (log_decisions())
+                {
+                    MXS_NOTICE("Using fresh data from cache.");
+                }
+                routing_action = ROUTING_ABORT;
+            }
+        }
+        else
+        {
+            routing_action = ROUTING_CONTINUE;
+        }
+
+        if (routing_action == ROUTING_CONTINUE)
+        {
+            m_state = CACHE_EXPECTING_RESPONSE;
+        }
+        else
+        {
+            m_state = CACHE_EXPECTING_NOTHING;
+            gwbuf_free(pPacket);
+            DCB *dcb = m_pSession->client_dcb;
+
+            // TODO: This is not ok. Any filters before this filter, will not
+            // TODO: see this data.
+            dcb->func.write(dcb, pResponse);
+        }
+    }
+    else
+    {
+        ss_dassert(should_populate(cache_action));
+
+        // We will not use any value in the cache, but we will update
+        // the existing value.
+        if (log_decisions())
+        {
+            MXS_NOTICE("Unconditionally fetching data from the server, "
+                       "refreshing cache entry.");
+        }
+        m_state = CACHE_EXPECTING_RESPONSE;
+    }
+
+    return routing_action;
 }

server/modules/filter/cache/cachefiltersession.hh

@@ -109,7 +109,34 @@ private:
 
     void store_result();
 
-    bool should_consult_cache(GWBUF* pPacket);
+    enum cache_action_t
+    {
+        CACHE_IGNORE           = 0,
+        CACHE_USE              = 1,
+        CACHE_POPULATE         = 2,
+        CACHE_USE_AND_POPULATE = (CACHE_USE | CACHE_POPULATE)
+    };
+
+    static bool should_use(cache_action_t action)
+    {
+        return action & CACHE_USE ? true : false;
+    }
+
+    static bool should_populate(cache_action_t action)
+    {
+        return action & CACHE_POPULATE ? true : false;
+    }
+
+    cache_action_t get_cache_action(GWBUF* pPacket);
+
+    enum routing_action_t
+    {
+        ROUTING_ABORT,    /**< Abort normal routing activity, data is coming from cache. */
+        ROUTING_CONTINUE, /**< Continue normal routing activity. */
+    };
+
+    routing_action_t route_COM_QUERY(GWBUF* pPacket);
+    routing_action_t route_SELECT(cache_action_t action, GWBUF* pPacket);
 
 private:
     CacheFilterSession(MXS_SESSION* pSession, Cache* pCache, char* zDefaultDb);