From eb05132a5cb01e3750218461a386b7c02c1935f5 Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Mon, 23 Jan 2017 10:57:37 +0200
Subject: [PATCH] Cache: Use MXS_MODULE_PARAM_SIZE

---
 Documentation/Filters/Cache.md             | 23 ++++++++--------
 server/modules/filter/cache/cachefilter.cc | 32 ++++++++++++++++------
 2 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/Documentation/Filters/Cache.md b/Documentation/Filters/Cache.md
index 9baf366df..10101abea 100644
--- a/Documentation/Filters/Cache.md
+++ b/Documentation/Filters/Cache.md
@@ -113,14 +113,17 @@ The default value is `0`, which means no limit.
 
 #### `max_resultset_size`
 
-Specifies the maximum size a resultset can have, measured in kibibytes,
-in order to be stored in the cache. A resultset larger than this, will
-not be stored.
+Specifies the maximum size of a resultset, for it to be stored in the cache.
+A resultset larger than this, will not be stored. The size can be specified
+as described [here](../Getting-Started/Configuration-Guide.md#sizes).
 ```
-max_resultset_size=128
+max_resultset_size=128Ki
 ```
 The default value is `0`, which means no limit.
 
+Note that the value of `max_resultset_size` should not be larger than the
+value of `max_size`.
+
 #### `max_count`
 
 The maximum number of items the cache may contain. If the limit has been
@@ -137,18 +140,16 @@ The default value is `0`, which means no limit.
 
 #### `max_size`
 
-The maximum size - expressed in kibibytes - the cache may occupy. If the limit
-has been reached and a new item should be stored, then some older item(s) will
-be evicted to make space.
-
-Note that the value of `max_size` must be at least as large as the value of
-`max_resultset_size`.
+The maximum size the cache may occupy. If the limit has been reached and a new
+item should be stored, then some older item(s) will be evicted to make space.
+The size can be specified as described
+[here](../Getting-Started/Configuration-Guide.md#sizes).
 
 Note that if `cached_data` is `thread_specific` then this limit will be
 applied to each cache _separately_. That is, if a thread specific cache
 is used, then the total size is #threads * the value of `max_size`.
 ```
-max_size=1000
+max_size=100Mi
 ```
 The default value is `0`, which means no limit.
 
diff --git a/server/modules/filter/cache/cachefilter.cc b/server/modules/filter/cache/cachefilter.cc
index 7b22e356e..c0864a659 100644
--- a/server/modules/filter/cache/cachefilter.cc
+++ b/server/modules/filter/cache/cachefilter.cc
@@ -153,7 +153,7 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE()
             {
                 "hard_ttl",
                 MXS_MODULE_PARAM_COUNT,
-                CACHE_DEFAULT_MAX_RESULTSET_SIZE
+                CACHE_DEFAULT_HARD_TTL
             },
             {
                 "soft_ttl",
@@ -167,7 +167,7 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE()
             },
             {
                 "max_resultset_size",
-                MXS_MODULE_PARAM_COUNT,
+                MXS_MODULE_PARAM_SIZE,
                 CACHE_DEFAULT_MAX_RESULTSET_SIZE
             },
             {
@@ -177,7 +177,7 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE()
             },
             {
                 "max_size",
-                MXS_MODULE_PARAM_COUNT,
+                MXS_MODULE_PARAM_SIZE,
                 CACHE_DEFAULT_MAX_SIZE
             },
             {
@@ -285,11 +285,11 @@ bool CacheFilter::process_params(char **pzOptions, CONFIG_PARAMETER *ppParams, C
     config.debug = config_get_integer(ppParams, "debug");
     config.hard_ttl = config_get_integer(ppParams, "hard_ttl");
     config.soft_ttl = config_get_integer(ppParams, "soft_ttl");
-    config.max_size = config_get_integer(ppParams, "max_size");
+    config.max_size = config_get_size(ppParams, "max_size");
     config.max_count = config_get_integer(ppParams, "max_count");
     config.storage = MXS_STRDUP(config_get_string(ppParams, "storage"));
     config.max_resultset_rows = config_get_integer(ppParams, "max_resultset_rows");
-    config.max_resultset_size = config_get_integer(ppParams, "max_resultset_size");
+    config.max_resultset_size = config_get_size(ppParams, "max_resultset_size");
     config.thread_model = static_cast<cache_thread_model_t>(config_get_enum(ppParams,
                                                                             "cached_data",
                                                                             cached_data_values));
@@ -366,12 +366,26 @@ bool CacheFilter::process_params(char **pzOptions, CONFIG_PARAMETER *ppParams, C
             config.soft_ttl = config.hard_ttl;
         }
 
-        if (config.max_size < config.max_resultset_size)
+        if (config.max_resultset_size == 0)
         {
-            MXS_ERROR("The value of 'max_size' must be at least as larged as that "
-                      "of 'max_resultset_size'.");
+            if (config.max_size != 0)
+            {
+                // If a specific size has been configured for 'max_size' but 'max_resultset_size'
+                // has not been specifically set, then we silently set it to the same as 'max_size'.
+                config.max_resultset_size = config.max_size;
+            }
+        }
+        else
+        {
+            ss_dassert(config.max_resultset_size != 0);
 
-            error = true;
+            if ((config.max_size != 0) && (config.max_resultset_size > config.max_size))
+            {
+                MXS_WARNING("The value of 'max_resultset_size' %ld should not be larger than "
+                            "the value of 'max_size' %ld. Adjusting the value of 'max_resultset_size' "
+                            "down to %ld.", config.max_resultset_size, config.max_size, config.max_size);
+                config.max_resultset_size = config.max_size;
+            }
         }
     }