diff --git a/Documentation/Filters/Cache.md b/Documentation/Filters/Cache.md index 046bc51c0..98448b64a 100644 --- a/Documentation/Filters/Cache.md +++ b/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 `-1`. +The default value is `0`, which means no limit. #### `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` diff --git a/server/modules/filter/cache/cache_storage_api.h b/server/modules/filter/cache/cache_storage_api.h index 30b491f30..768c3c8b9 100644 --- a/server/modules/filter/cache/cache_storage_api.h +++ b/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` diff --git a/server/modules/filter/cache/cachefilter.cc b/server/modules/filter/cache/cachefilter.cc index b5dbea90d..6ba5d8921 100644 --- a/server/modules/filter/cache/cachefilter.cc +++ b/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; - int32_t value = strtol(pParam->value, &end, 0); - - if ((*end == 0) && (value >= 0)) + if (!config_get_uint64(*pParam, &config.max_resultset_rows)) { - 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; - int64_t value = strtoll(pParam->value, &end, 0); - - if ((*end == 0) && (value >= 0)) + if (config_get_uint64(*pParam, &config.max_resultset_size)) { - if (value != 0) - { - config.max_resultset_size = value * 1024; - } - else - { - config.max_resultset_size = CACHE_DEFAULT_MAX_RESULTSET_SIZE; - } + config.max_resultset_size *= 1024; } 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 (v > 0) + if (!config_get_uint32(*pParam, &config.ttl)) { - 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; - int32_t value = strtoul(pParam->value, &end, 0); - - if ((*end == 0) && (value >= 0)) + if (!config_get_uint64(*pParam, &config.max_count)) { - 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; - int64_t value = strtoull(pParam->value, &end, 0); - - if ((*end == 0) && (value >= 0)) + if (config_get_uint64(*pParam, &config.max_size)) { - if (value != 0) - { - config.max_size = value * 1024; - } - else - { - config.max_size = CACHE_DEFAULT_MAX_SIZE; - } + config.max_size = config.max_size * 1024; } else { - MXS_ERROR("The value of the configuration entry '%s' must " - "be an integer larger than or equal to 0.", pParam->name); error = true; } } diff --git a/server/modules/filter/cache/cachefilter.h b/server/modules/filter/cache/cachefilter.h index 5ff0d4c52..9f14b5b71 100644 --- a/server/modules/filter/cache/cachefilter.h +++ b/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. */ - uint32_t max_resultset_size; /**< The maximum size 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. */ + 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. */ diff --git a/server/modules/filter/cache/cachefiltersession.cc b/server/modules/filter/cache/cachefiltersession.cc index 193a34944..c3337e014 100644 --- a/server/modules/filter/cache/cachefiltersession.cc +++ b/server/modules/filter/cache/cachefiltersession.cc @@ -19,6 +19,21 @@ #include #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()) { diff --git a/server/modules/filter/cache/lrustorage.cc b/server/modules/filter/cache/lrustorage.cc index 77eb1e07d..46c90ee1e 100644 --- a/server/modules/filter/cache/lrustorage.cc +++ b/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_size_(max_size) + , max_count_(max_count != 0 ? max_count : UINT64_MAX) + , max_size_(max_size != 0 ? max_size : UINT64_MAX) , phead_(NULL) , ptail_(NULL) { diff --git a/server/modules/filter/cache/lrustorage.hh b/server/modules/filter/cache/lrustorage.hh index 1c72f49c4..2c1c7881e 100644 --- a/server/modules/filter/cache/lrustorage.hh +++ b/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, */ - size_t max_size_; /*< The maximum size of all cached items. */ + uint64_t max_count_; /*< The maximum number of items in the LRU list, */ + 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. */ diff --git a/server/modules/filter/cache/lrustoragemt.cc b/server/modules/filter/cache/lrustoragemt.cc index b373c45cc..f67e3cf47 100644 --- a/server/modules/filter/cache/lrustoragemt.cc +++ b/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; diff --git a/server/modules/filter/cache/lrustoragemt.hh b/server/modules/filter/cache/lrustoragemt.hh index d77ad54d6..375daf883 100644 --- a/server/modules/filter/cache/lrustoragemt.hh +++ b/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&); diff --git a/server/modules/filter/cache/lrustoragest.cc b/server/modules/filter/cache/lrustoragest.cc index 7e185e222..c38d7ee0c 100644 --- a/server/modules/filter/cache/lrustoragest.cc +++ b/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; diff --git a/server/modules/filter/cache/lrustoragest.hh b/server/modules/filter/cache/lrustoragest.hh index fd422052c..575d8f1f4 100644 --- a/server/modules/filter/cache/lrustoragest.hh +++ b/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&); diff --git a/server/modules/filter/cache/storage/storage_inmemory/inmemorystorage.cc b/server/modules/filter/cache/storage/storage_inmemory/inmemorystorage.cc index b984990ce..83d04323c 100644 --- a/server/modules/filter/cache/storage/storage_inmemory/inmemorystorage.cc +++ b/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)) {