Cache: 0 is now default of integer parameters

0 is now the default of all cache configuration parameters and in
all cases the meaning is the same; that is, no limit. Internally
all limits but ttl are now for the sake of consistency 64-bit.

Documentation/Filters/Cache.md

@@ -66,9 +66,7 @@ stored in the cache. A resultset larger than this, will not be stored.
 ```
 max_resultset_rows=1000
 ```
-Zero or a negative value is interpreted as no limitation.
+The default value is `0`, which means no limit.
-
-The default value is `-1`.
 
 #### `max_resultset_size`
 
@@ -78,18 +76,17 @@ not be stored.
 ```
 max_resultset_size=128
 ```
-The default value is 64.
+The default value is `0`, which means no limit.
 
 #### `ttl`
 
 _Time to live_; the amount of time - in seconds - the cached result is used
 before it is refreshed from the server.
 
-If nothing is specified, the default _ttl_ value is 10.
-
 ```
 ttl=60
 ```
+The default value is `0`, which means no limit.
 
 #### `max_count`
 
@@ -101,7 +98,7 @@ applied to each cache _separately_.
 ```
 max_count=1000
 ```
-The default value is 0, which means no limit.
+The default value is `0`, which means no limit.
 
 #### `max_size`
 
@@ -117,7 +114,7 @@ applied to each cache _separately_.
 ```
 max_size=1000
 ```
-The default value is 0, which means no limit.
+The default value is `0`, which means no limit.
 
 #### `rules`
 

server/modules/filter/cache/cache_storage_api.h

@@ -117,11 +117,15 @@ typedef struct cache_storage_api
      *                  case it need not.
      * @param name      The name of the cache instance.
      * @param ttl       Time to live; number of seconds the value is valid.
+     *                  A value of 0 means that there is no time-to-live, but that
+     *                  the value is considered fresh as long as it is available.
      * @param max_count The maximum number of items the storage may store, before
-     *                  it should evict some items. Caller should specify 0, unless
+     *                  it should evict some items. A value of 0 means that there is
+     *                  no limit. The caller should specify 0, unless
      *                  CACHE_STORAGE_CAP_MAX_COUNT is returned at initialization.
      * @param max_count The maximum size of the storage may may occupy, before it
-                        should evict some items. Caller should specify 0, unless
+     *                  should evict some items. A value if 0 means that there is
+     *                  no limit. The caller should specify 0, unless
      *                  CACHE_STORAGE_CAP_MAX_SIZE is returned at initialization.
      * @param argc      The number of elements in the argv array.
      * @param argv      Array of arguments, as passed in the `storage_options`

server/modules/filter/cache/cachefilter.cc

@@ -123,6 +123,68 @@ bool cache_command_show(const MODULECMD_ARG* pArgs)
     return true;
 }
 
+/**
+ * Get a 32-bit unsigned value.
+ *
+ * Note that the value itself is converted a signed integer to detect
+ * configuration errors.
+ *
+ * @param param   The parameter entry.
+ * @param pValue  Pointer to variable where result is stored.
+ *
+ * @return True if the parameter was an unsigned integer.
+ */
+bool config_get_uint32(const FILTER_PARAMETER& param, uint32_t* pValue)
+{
+    bool rv = false;
+    char* end;
+    int32_t value = strtol(param.value, &end, 0);
+
+    if ((*end == 0) && (value >= 0))
+    {
+        *pValue = value;
+        rv = true;
+    }
+    else
+    {
+        MXS_ERROR("The value of the configuration entry '%s' must "
+                  "be an integer larger than or equal to 0.", param.name);
+    }
+
+    return rv;
+}
+
+/**
+ * Get a 64-bit unsigned value.
+ *
+ * Note that the value itself is converted a signed integer to detect
+ * configuration errors.
+ *
+ * @param param   The parameter entry.
+ * @param pValue  Pointer to variable where result is stored.
+ *
+ * @return True if the parameter was an unsigned integer.
+ */
+bool config_get_uint64(const FILTER_PARAMETER& param, uint64_t* pValue)
+{
+    bool rv = false;
+    char* end;
+    int64_t value = strtoll(param.value, &end, 0);
+
+    if ((*end == 0) && (value >= 0))
+    {
+        *pValue = value;
+        rv = true;
+    }
+    else
+    {
+        MXS_ERROR("The value of the configuration entry '%s' must "
+                  "be an integer larger than or equal to 0.", param.name);
+    }
+
+    return rv;
+}
+
 }
 
 //
@@ -246,47 +308,19 @@ bool CacheFilter::process_params(char **pzOptions, FILTER_PARAMETER **ppParams,
 
         if (strcmp(pParam->name, "max_resultset_rows") == 0)
         {
-            char* end;
+            if (!config_get_uint64(*pParam, &config.max_resultset_rows))
-            int32_t value = strtol(pParam->value, &end, 0);
-
-            if ((*end == 0) && (value >= 0))
             {
-                if (value != 0)
-                {
-                    config.max_resultset_rows = value;
-                }
-                else
-                {
-                    config.max_resultset_rows = CACHE_DEFAULT_MAX_RESULTSET_ROWS;
-                }
-            }
-            else
-            {
-                MXS_ERROR("The value of the configuration entry '%s' must "
-                          "be an integer larger than 0.", pParam->name);
                 error = true;
             }
         }
         else if (strcmp(pParam->name, "max_resultset_size") == 0)
         {
-            char* end;
+            if (config_get_uint64(*pParam, &config.max_resultset_size))
-            int64_t value = strtoll(pParam->value, &end, 0);
-
-            if ((*end == 0) && (value >= 0))
             {
-                if (value != 0)
+                config.max_resultset_size *= 1024;
-                {
-                    config.max_resultset_size = value * 1024;
             }
             else
             {
-                    config.max_resultset_size = CACHE_DEFAULT_MAX_RESULTSET_SIZE;
-                }
-            }
-            else
-            {
-                MXS_ERROR("The value of the configuration entry '%s' must "
-                          "be an integer larger than 0.", pParam->name);
                 error = true;
             }
         }
@@ -371,62 +405,26 @@ bool CacheFilter::process_params(char **pzOptions, FILTER_PARAMETER **ppParams,
         }
         else if (strcmp(pParam->name, "ttl") == 0)
         {
-            int v = atoi(pParam->value);
+            if (!config_get_uint32(*pParam, &config.ttl))
-
-            if (v > 0)
             {
-                config.ttl = v;
-            }
-            else
-            {
-                MXS_ERROR("The value of the configuration entry '%s' must "
-                          "be an integer larger than 0.", pParam->name);
                 error = true;
             }
         }
         else if (strcmp(pParam->name, "max_count") == 0)
         {
-            char* end;
+            if (!config_get_uint64(*pParam, &config.max_count))
-            int32_t value = strtoul(pParam->value, &end, 0);
-
-            if ((*end == 0) && (value >= 0))
             {
-                if (value != 0)
-                {
-                    config.max_count = value;
-                }
-                else
-                {
-                    config.max_count = CACHE_DEFAULT_MAX_COUNT;
-                }
-            }
-            else
-            {
-                MXS_ERROR("The value of the configuration entry '%s' must "
-                          "be an integer larger than or equal to 0.", pParam->name);
                 error = true;
             }
         }
         else if (strcmp(pParam->name, "max_size") == 0)
         {
-            char* end;
+            if (config_get_uint64(*pParam, &config.max_size))
-            int64_t value = strtoull(pParam->value, &end, 0);
-
-            if ((*end == 0) && (value >= 0))
             {
-                if (value != 0)
+                config.max_size = config.max_size * 1024;
-                {
-                    config.max_size = value * 1024;
             }
             else
             {
-                    config.max_size = CACHE_DEFAULT_MAX_SIZE;
-                }
-            }
-            else
-            {
-                MXS_ERROR("The value of the configuration entry '%s' must "
-                          "be an integer larger than or equal to 0.", pParam->name);
                 error = true;
             }
         }

server/modules/filter/cache/cachefilter.h

@@ -39,31 +39,31 @@
 #endif
 
 // Count
-#define CACHE_DEFAULT_MAX_RESULTSET_ROWS UINT32_MAX
+#define CACHE_DEFAULT_MAX_RESULTSET_ROWS 0
 // Bytes
-#define CACHE_DEFAULT_MAX_RESULTSET_SIZE 64 * 1024
+#define CACHE_DEFAULT_MAX_RESULTSET_SIZE 0
 // Seconds
-#define CACHE_DEFAULT_TTL                10
+#define CACHE_DEFAULT_TTL                0
 // Integer value
 #define CACHE_DEFAULT_DEBUG              0
 // Positive integer
-#define CACHE_DEFAULT_MAX_COUNT          UINT32_MAX
+#define CACHE_DEFAULT_MAX_COUNT          0
 // Positive integer
-#define CACHE_DEFAULT_MAX_SIZE           UINT64_MAX
+#define CACHE_DEFAULT_MAX_SIZE           0
 // Thread model
 #define CACHE_DEFAULT_THREAD_MODEL       CACHE_THREAD_MODEL_MT
 
 typedef struct cache_config
 {
-    uint32_t max_resultset_rows;       /**< The maximum number of rows of a resultset for it to be cached. */
+    uint64_t max_resultset_rows;       /**< The maximum number of rows of a resultset for it to be cached. */
-    uint32_t max_resultset_size;       /**< The maximum size of a resultset for it to be cached. */
+    uint64_t max_resultset_size;       /**< The maximum size of a resultset for it to be cached. */
     char* rules;                       /**< Name of rules file. */
     char* storage;                     /**< Name of storage module. */
     char* storage_options;             /**< Raw options for storage module. */
     char** storage_argv;               /**< Cooked options for storage module. */
     int storage_argc;                  /**< Number of cooked options. */
     uint32_t ttl;                      /**< Time to live. */
-    uint32_t max_count;                /**< Maximum number of entries in the cache.*/
+    uint64_t max_count;                /**< Maximum number of entries in the cache.*/
     uint64_t max_size;                 /**< Maximum size of the cache.*/
     uint32_t debug;                    /**< Debug settings. */
     cache_thread_model_t thread_model; /**< Thread model. */

server/modules/filter/cache/cachefiltersession.cc

@@ -19,6 +19,21 @@
 #include <maxscale/mysql_utils.h>
 #include "storage.hh"
 
+namespace
+{
+
+inline bool cache_max_resultset_rows_exceeded(const CACHE_CONFIG& config, uint64_t rows)
+{
+    return config.max_resultset_rows == 0 ? false : rows > config.max_resultset_rows;
+}
+
+inline bool cache_max_resultset_size_exceeded(const CACHE_CONFIG& config, uint64_t size)
+{
+    return config.max_resultset_size == 0 ? false : size > config.max_resultset_size;
+}
+
+}
+
 CacheFilterSession::CacheFilterSession(SESSION* pSession, Cache* pCache, char* zDefaultDb)
     : maxscale::FilterSession(pSession)
     , m_state(CACHE_EXPECTING_NOTHING)
@@ -240,12 +255,12 @@ int CacheFilterSession::clientReply(GWBUF* pData)
 
     if (m_state != CACHE_IGNORING_RESPONSE)
     {
-        if (gwbuf_length(m_res.pData) > m_pCache->config().max_resultset_size)
+        if (cache_max_resultset_size_exceeded(m_pCache->config(), gwbuf_length(m_res.pData)))
         {
             if (log_decisions())
             {
                 MXS_NOTICE("Current size %uB of resultset, at least as much "
-                           "as maximum allowed size %uKiB. Not caching.",
+                           "as maximum allowed size %luKiB. Not caching.",
                            gwbuf_length(m_res.pData),
                            m_pCache->config().max_resultset_size / 1024);
             }
@@ -479,7 +494,7 @@ int CacheFilterSession::handle_expecting_rows()
                 m_res.offset += packetlen;
                 ++m_res.nRows;
 
-                if (m_res.nRows > m_pCache->config().max_resultset_rows)
+                if (cache_max_resultset_rows_exceeded(m_pCache->config(), m_res.nRows))
                 {
                     if (log_decisions())
                     {

server/modules/filter/cache/lrustorage.cc

@@ -16,8 +16,8 @@
 
 LRUStorage::LRUStorage(Storage* pstorage, size_t max_count, size_t max_size)
     : pstorage_(pstorage)
-    , max_count_(max_count)
+    , max_count_(max_count != 0 ? max_count : UINT64_MAX)
-    , max_size_(max_size)
+    , max_size_(max_size != 0 ? max_size : UINT64_MAX)
     , phead_(NULL)
     , ptail_(NULL)
 {

server/modules/filter/cache/lrustorage.hh

@@ -31,7 +31,7 @@ public:
                            CACHE_KEY* pKey);
 
 protected:
-    LRUStorage(Storage* pstorage, size_t max_count, size_t max_size);
+    LRUStorage(Storage* pstorage, uint64_t max_count, uint64_t max_size);
 
     /**
      * Returns information about the LRU storage and the underlying real
@@ -201,8 +201,8 @@ private:
     };
 
     Storage*    pstorage_;      /*< The actual storage. */
-    size_t      max_count_;     /*< The maximum number of items in the LRU list, */
+    uint64_t    max_count_;     /*< The maximum number of items in the LRU list, */
-    size_t      max_size_;      /*< The maximum size of all cached items. */
+    uint64_t    max_size_;      /*< The maximum size of all cached items. */
     Stats       stats_;         /*< Cache statistics. */
     NodesPerKey nodes_per_key_; /*< Mapping from cache keys to corresponding Node. */
     Node*       phead_;         /*< The node at the LRU list. */

server/modules/filter/cache/lrustoragemt.cc

@@ -16,7 +16,7 @@
 
 using maxscale::SpinLockGuard;
 
-LRUStorageMT::LRUStorageMT(Storage* pstorage, size_t max_count, size_t max_size)
+LRUStorageMT::LRUStorageMT(Storage* pstorage, uint64_t max_count, uint64_t max_size)
     : LRUStorage(pstorage, max_count, max_size)
 {
     spinlock_init(&lock_);
@@ -28,7 +28,7 @@ LRUStorageMT::~LRUStorageMT()
 {
 }
 
-LRUStorageMT* LRUStorageMT::create(Storage* pstorage, size_t max_count, size_t max_size)
+LRUStorageMT* LRUStorageMT::create(Storage* pstorage, uint64_t max_count, uint64_t max_size)
 {
     LRUStorageMT* plru_storage = NULL;
 

server/modules/filter/cache/lrustoragemt.hh

@@ -21,7 +21,7 @@ class LRUStorageMT : public LRUStorage
 public:
     ~LRUStorageMT();
 
-    static LRUStorageMT* create(Storage* pstorage, size_t max_count, size_t max_size);
+    static LRUStorageMT* create(Storage* pstorage, uint64_t max_count, uint64_t max_size);
 
     cache_result_t get_info(uint32_t what,
                             json_t** ppInfo) const;
@@ -36,7 +36,7 @@ public:
     cache_result_t del_value(const CACHE_KEY& key);
 
 private:
-    LRUStorageMT(Storage* pstorage, size_t max_count, size_t max_size);
+    LRUStorageMT(Storage* pstorage, uint64_t max_count, uint64_t max_size);
 
     LRUStorageMT(const LRUStorageMT&);
     LRUStorageMT& operator = (const LRUStorageMT&);

server/modules/filter/cache/lrustoragest.cc

@@ -14,7 +14,7 @@
 #define MXS_MODULE_NAME "cache"
 #include "lrustoragest.hh"
 
-LRUStorageST::LRUStorageST(Storage* pstorage, size_t max_count, size_t max_size)
+LRUStorageST::LRUStorageST(Storage* pstorage, uint64_t max_count, uint64_t max_size)
     : LRUStorage(pstorage, max_count, max_size)
 {
     MXS_NOTICE("Created single threaded LRU storage.");
@@ -24,7 +24,7 @@ LRUStorageST::~LRUStorageST()
 {
 }
 
-LRUStorageST* LRUStorageST::create(Storage* pstorage, size_t max_count, size_t max_size)
+LRUStorageST* LRUStorageST::create(Storage* pstorage, uint64_t max_count, uint64_t max_size)
 {
     LRUStorageST* plru_storage = NULL;
 

server/modules/filter/cache/lrustoragest.hh

@@ -20,7 +20,7 @@ class LRUStorageST : public LRUStorage
 public:
     ~LRUStorageST();
 
-    static LRUStorageST* create(Storage* pstorage, size_t max_count, size_t max_size);
+    static LRUStorageST* create(Storage* pstorage, uint64_t max_count, uint64_t max_size);
 
     cache_result_t get_info(uint32_t what,
                             json_t** ppInfo) const;
@@ -35,7 +35,7 @@ public:
     cache_result_t del_value(const CACHE_KEY& key);
 
 private:
-    LRUStorageST(Storage* pstorage, size_t max_count, size_t max_size);
+    LRUStorageST(Storage* pstorage, uint64_t max_count, uint64_t max_size);
 
     LRUStorageST(const LRUStorageST&);
     LRUStorageST& operator = (const LRUStorageST&);

server/modules/filter/cache/storage/storage_inmemory/inmemorystorage.cc

@@ -133,7 +133,7 @@ cache_result_t InMemoryStorage::do_get_value(const CACHE_KEY& key, uint32_t flag
 
         uint32_t now = time(NULL);
 
-        bool is_stale = (now - entry.time > ttl_);
+        bool is_stale = ttl_ == 0 ? false : (now - entry.time > ttl_);
 
         if (!is_stale || ((flags & CACHE_FLAGS_INCLUDE_STALE) != 0))
         {