From a71d1f08778641a8a6646c2b4f6df98ce9eaf410 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 25 Nov 2016 11:08:36 +0200 Subject: [PATCH] Cache: Param processing moved to cachefilter.cc This in antecipation of different Cache instances being created depending on passed arguments. --- server/modules/filter/cache/cache.cc | 258 ++++----------------- server/modules/filter/cache/cache.h | 6 +- server/modules/filter/cache/cachefilter.cc | 255 +++++++++++++++++++- server/modules/filter/cache/cachefilter.h | 12 +- 4 files changed, 304 insertions(+), 227 deletions(-) diff --git a/server/modules/filter/cache/cache.cc b/server/modules/filter/cache/cache.cc index b6e20f4ab..971322b64 100644 --- a/server/modules/filter/cache/cache.cc +++ b/server/modules/filter/cache/cache.cc @@ -23,19 +23,6 @@ // are being fetches. #define CACHE_PENDING_ITEMS 50 -static const CACHE_CONFIG DEFAULT_CONFIG = -{ - CACHE_DEFAULT_MAX_RESULTSET_ROWS, - CACHE_DEFAULT_MAX_RESULTSET_SIZE, - NULL, - NULL, - NULL, - NULL, - 0, - CACHE_DEFAULT_TTL, - CACHE_DEFAULT_DEBUG -}; - /** * Hashes a cache key to an integer. * @@ -73,7 +60,7 @@ static int hashcmp(const void* address1, const void* address2) Cache::Cache(const char* zName, - const CACHE_CONFIG& config, + CACHE_CONFIG& config, CACHE_RULES* pRules, StorageFactory* pFactory, Storage* pStorage, @@ -85,6 +72,7 @@ Cache::Cache(const char* zName, , m_pStorage(pStorage) , m_pPending(pPending) { + cache_config_reset(config); } Cache::~Cache() @@ -94,78 +82,73 @@ Cache::~Cache() } //static -Cache* Cache::Create(const char* zName, char** pzOptions, FILTER_PARAMETER** ppParams) +Cache* Cache::Create(const char* zName, CACHE_CONFIG& config) { Cache* pCache = NULL; - CACHE_CONFIG config = DEFAULT_CONFIG; + CACHE_RULES* pRules = NULL; - if (process_params(pzOptions, ppParams, &config)) + if (config.rules) { - CACHE_RULES* pRules = NULL; + pRules = cache_rules_load(config.rules, config.debug); + } + else + { + pRules = cache_rules_create(config.debug); + } - if (config.rules) - { - pRules = cache_rules_load(config.rules, config.debug); - } - else - { - pRules = cache_rules_create(config.debug); - } + if (pRules) + { + HASHTABLE* pPending = hashtable_alloc(CACHE_PENDING_ITEMS, hashfn, hashcmp); - if (pRules) + if (pPending) { - HASHTABLE* pPending = hashtable_alloc(CACHE_PENDING_ITEMS, hashfn, hashcmp); + StorageFactory *pFactory = StorageFactory::Open(config.storage); - if (pPending) + if (pFactory) { - StorageFactory *pFactory = StorageFactory::Open(config.storage); + uint32_t ttl = config.ttl; + int argc = config.storage_argc; + char** argv = config.storage_argv; - if (pFactory) + Storage* pStorage = pFactory->createStorage(zName, ttl, argc, argv); + + if (pStorage) { - uint32_t ttl = config.ttl; - int argc = config.storage_argc; - char** argv = config.storage_argv; - - Storage* pStorage = pFactory->createStorage(zName, ttl, argc, argv); - - if (pStorage) - { - pCache = new (std::nothrow) Cache(zName, - config, - pRules, - pFactory, - pStorage, - pPending); - } - else - { - MXS_ERROR("Could not create storage instance for '%s'.", zName); - } + pCache = new (std::nothrow) Cache(zName, + config, + pRules, + pFactory, + pStorage, + pPending); } else { - MXS_ERROR("Could not open storage factory '%s'.", config.storage); - } - - if (!pCache) - { - delete pFactory; + MXS_ERROR("Could not create storage instance for '%s'.", zName); } } + else + { + MXS_ERROR("Could not open storage factory '%s'.", config.storage); + } if (!pCache) { - hashtable_free(pPending); + delete pFactory; } } if (!pCache) { - cache_rules_free(pRules); + hashtable_free(pPending); } } + if (!pCache) + { + cache_rules_free(pRules); + } + return pCache; } @@ -233,162 +216,3 @@ cache_result_t Cache::delValue(const char* pKey) { return m_pStorage->delValue(pKey); } - -/** - * Processes the cache params - * - * @param options Options as passed to the filter. - * @param params Parameters as passed to the filter. - * @param config Pointer to config instance where params will be stored. - * - * @return True if all parameters could be processed, false otherwise. - */ -bool Cache::process_params(char **pzOptions, FILTER_PARAMETER **ppParams, CACHE_CONFIG* pConfig) -{ - bool error = false; - - for (int i = 0; ppParams[i]; ++i) - { - const FILTER_PARAMETER *pParam = ppParams[i]; - - if (strcmp(pParam->name, "max_resultset_rows") == 0) - { - int v = atoi(pParam->value); - - if (v > 0) - { - pConfig->max_resultset_rows = v; - } - else - { - pConfig->max_resultset_rows = CACHE_DEFAULT_MAX_RESULTSET_ROWS; - } - } - else if (strcmp(pParam->name, "max_resultset_size") == 0) - { - int v = atoi(pParam->value); - - if (v > 0) - { - pConfig->max_resultset_size = v * 1024; - } - 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, "rules") == 0) - { - if (*pParam->value == '/') - { - pConfig->rules = MXS_STRDUP(pParam->value); - } - else - { - const char *datadir = get_datadir(); - size_t len = strlen(datadir) + 1 + strlen(pParam->value) + 1; - - char *rules = (char*)MXS_MALLOC(len); - - if (rules) - { - sprintf(rules, "%s/%s", datadir, pParam->value); - pConfig->rules = rules; - } - } - - if (!pConfig->rules) - { - error = true; - } - } - else if (strcmp(pParam->name, "storage_options") == 0) - { - pConfig->storage_options = MXS_STRDUP(pParam->value); - - if (pConfig->storage_options) - { - int argc = 1; - char *arg = pConfig->storage_options; - - while ((arg = strchr(pConfig->storage_options, ','))) - { - ++argc; - } - - pConfig->storage_argv = (char**) MXS_MALLOC((argc + 1) * sizeof(char*)); - - if (pConfig->storage_argv) - { - pConfig->storage_argc = argc; - - int i = 0; - arg = pConfig->storage_options; - pConfig->storage_argv[i++] = arg; - - while ((arg = strchr(pConfig->storage_options, ','))) - { - *arg = 0; - ++arg; - pConfig->storage_argv[i++] = arg; - } - - pConfig->storage_argv[i] = NULL; - } - else - { - MXS_FREE(pConfig->storage_options); - pConfig->storage_options = NULL; - } - } - else - { - error = true; - } - } - else if (strcmp(pParam->name, "storage") == 0) - { - pConfig->storage = pParam->value; - } - else if (strcmp(pParam->name, "ttl") == 0) - { - int v = atoi(pParam->value); - - if (v > 0) - { - pConfig->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, "debug") == 0) - { - int v = atoi(pParam->value); - - if ((v >= CACHE_DEBUG_MIN) && (v <= CACHE_DEBUG_MAX)) - { - pConfig->debug = v; - } - else - { - MXS_ERROR("The value of the configuration entry '%s' must " - "be between %d and %d, inclusive.", - pParam->name, CACHE_DEBUG_MIN, CACHE_DEBUG_MAX); - error = true; - } - } - else if (!filter_standard_parameter(pParam->name)) - { - MXS_ERROR("Unknown configuration entry '%s'.", pParam->name); - error = true; - } - } - - return !error; -} diff --git a/server/modules/filter/cache/cache.h b/server/modules/filter/cache/cache.h index 12def4232..105b5f39b 100644 --- a/server/modules/filter/cache/cache.h +++ b/server/modules/filter/cache/cache.h @@ -26,7 +26,7 @@ class Cache public: ~Cache(); - static Cache* Create(const char* zName, char** pzOptions, FILTER_PARAMETER** ppParams); + static Cache* Create(const char* zName, CACHE_CONFIG& config); /** * Returns whether the results of a particular query should be stored. @@ -77,14 +77,12 @@ public: private: Cache(const char* zName, - const CACHE_CONFIG& config, + CACHE_CONFIG& config, CACHE_RULES* pRules, StorageFactory* pFactory, Storage* pStorage, HASHTABLE* pPending); - static bool process_params(char **options, FILTER_PARAMETER **params, CACHE_CONFIG* config); - private: Cache(const Cache&); Cache& operator = (const Cache&); diff --git a/server/modules/filter/cache/cachefilter.cc b/server/modules/filter/cache/cachefilter.cc index e991bb390..d1b621869 100644 --- a/server/modules/filter/cache/cachefilter.cc +++ b/server/modules/filter/cache/cachefilter.cc @@ -14,12 +14,27 @@ #define MXS_MODULE_NAME "cache" #include "cachefilter.h" #include +#include #include +#include #include "cache.h" #include "sessioncache.h" static char VERSION_STRING[] = "V1.0.0"; +static const CACHE_CONFIG DEFAULT_CONFIG = +{ + CACHE_DEFAULT_MAX_RESULTSET_ROWS, + CACHE_DEFAULT_MAX_RESULTSET_SIZE, + NULL, + NULL, + NULL, + NULL, + 0, + CACHE_DEFAULT_TTL, + CACHE_DEFAULT_DEBUG +}; + static FILTER* createInstance(const char* zName, char** pzOptions, FILTER_PARAMETER** ppParams); static void* newSession(FILTER* pInstance, SESSION* pSession); static void closeSession(FILTER* pInstance, void* pSessionData); @@ -31,6 +46,8 @@ static int clientReply(FILTER* pInstance, void* pSessionData, GWBUF* pPacke static void diagnostics(FILTER* pInstance, void* pSessionData, DCB* pDcb); static uint64_t getCapabilities(void); +static bool process_params(char **pzOptions, FILTER_PARAMETER **ppParams, CACHE_CONFIG& config); + #define CPP_GUARD(statement)\ do { try { statement; } \ catch (const std::exception& x) { MXS_ERROR("Caught standard exception: %s", x.what()); }\ @@ -87,7 +104,7 @@ extern "C" FILTER_OBJECT *GetModuleObject() }; // -// API Implementation +// API Implementation BEGIN // /** @@ -103,7 +120,17 @@ extern "C" FILTER_OBJECT *GetModuleObject() static FILTER *createInstance(const char* zName, char** pzOptions, FILTER_PARAMETER** ppParams) { Cache* pCache = NULL; - CPP_GUARD(pCache = Cache::Create(zName, pzOptions, ppParams)); + CACHE_CONFIG config = DEFAULT_CONFIG; + + if (process_params(pzOptions, ppParams, config)) + { + CPP_GUARD(pCache = Cache::Create(zName, config)); + + if (!pCache) + { + cache_config_finish(config); + } + } return reinterpret_cast(pCache); } @@ -241,3 +268,227 @@ static uint64_t getCapabilities(void) { return RCAP_TYPE_TRANSACTION_TRACKING; } + +// +// API Implementation END +// + +/** + * Processes the cache params + * + * @param options Options as passed to the filter. + * @param params Parameters as passed to the filter. + * @param config Reference to config instance where params will be stored. + * + * @return True if all parameters could be processed, false otherwise. + */ +static bool process_params(char **pzOptions, FILTER_PARAMETER **ppParams, CACHE_CONFIG& config) +{ + bool error = false; + + for (int i = 0; ppParams[i]; ++i) + { + const FILTER_PARAMETER *pParam = ppParams[i]; + + if (strcmp(pParam->name, "max_resultset_rows") == 0) + { + int v = atoi(pParam->value); + + if (v > 0) + { + config.max_resultset_rows = v; + } + else + { + config.max_resultset_rows = CACHE_DEFAULT_MAX_RESULTSET_ROWS; + } + } + else if (strcmp(pParam->name, "max_resultset_size") == 0) + { + int v = atoi(pParam->value); + + if (v > 0) + { + config.max_resultset_size = v * 1024; + } + 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, "rules") == 0) + { + if (*pParam->value == '/') + { + config.rules = MXS_STRDUP(pParam->value); + } + else + { + const char* datadir = get_datadir(); + size_t len = strlen(datadir) + 1 + strlen(pParam->value) + 1; + + char *rules = (char*)MXS_MALLOC(len); + + if (rules) + { + sprintf(rules, "%s/%s", datadir, pParam->value); + config.rules = rules; + } + } + + if (!config.rules) + { + error = true; + } + } + else if (strcmp(pParam->name, "storage_options") == 0) + { + config.storage_options = MXS_STRDUP(pParam->value); + + if (config.storage_options) + { + int argc = 1; + char *arg = config.storage_options; + + while ((arg = strchr(config.storage_options, ','))) + { + ++argc; + } + + config.storage_argv = (char**) MXS_MALLOC((argc + 1) * sizeof(char*)); + + if (config.storage_argv) + { + config.storage_argc = argc; + + int i = 0; + arg = config.storage_options; + config.storage_argv[i++] = arg; + + while ((arg = strchr(config.storage_options, ','))) + { + *arg = 0; + ++arg; + config.storage_argv[i++] = arg; + } + + config.storage_argv[i] = NULL; + } + else + { + MXS_FREE(config.storage_options); + config.storage_options = NULL; + } + } + else + { + error = true; + } + } + else if (strcmp(pParam->name, "storage") == 0) + { + config.storage = MXS_STRDUP(pParam->value); + + if (!config.storage) + { + error = true; + } + } + else if (strcmp(pParam->name, "ttl") == 0) + { + int v = atoi(pParam->value); + + 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, "debug") == 0) + { + int v = atoi(pParam->value); + + if ((v >= CACHE_DEBUG_MIN) && (v <= CACHE_DEBUG_MAX)) + { + config.debug = v; + } + else + { + MXS_ERROR("The value of the configuration entry '%s' must " + "be between %d and %d, inclusive.", + pParam->name, CACHE_DEBUG_MIN, CACHE_DEBUG_MAX); + error = true; + } + } + else if (!filter_standard_parameter(pParam->name)) + { + MXS_ERROR("Unknown configuration entry '%s'.", pParam->name); + error = true; + } + } + + if (error) + { + cache_config_finish(config); + } + + return !error; +} + +/** + * Frees all data of a config object, but not the object itself + * + * @param pConfig Pointer to a config object. + */ +void cache_config_finish(CACHE_CONFIG& config) +{ + MXS_FREE(config.rules); + MXS_FREE(config.storage); + MXS_FREE(config.storage_options); + + for (int i = 0; i < config.storage_argc; ++i) + { + MXS_FREE(config.storage_argv[i]); + } + + config.max_resultset_rows = 0; + config.max_resultset_size = 0; + config.rules = NULL; + config.storage = NULL; + config.storage_options = NULL; + config.storage_argc = 0; + config.storage_argv = NULL; + config.ttl = 0; + config.debug = 0; +} + +/** + * Frees all data of a config object, and the object itself + * + * @param pConfig Pointer to a config object. + */ +void cache_config_free(CACHE_CONFIG* pConfig) +{ + if (pConfig) + { + cache_config_finish(*pConfig); + MXS_FREE(pConfig); + } +} + +/** + * Resets the data without freeing anything. + * + * @param config Reference to a config object. + */ +void cache_config_reset(CACHE_CONFIG& config) +{ + memset(&config, 0, sizeof(config)); +} diff --git a/server/modules/filter/cache/cachefilter.h b/server/modules/filter/cache/cachefilter.h index d31af252d..4f9fc4365 100644 --- a/server/modules/filter/cache/cachefilter.h +++ b/server/modules/filter/cache/cachefilter.h @@ -48,13 +48,17 @@ typedef struct cache_config { uint32_t max_resultset_rows; uint32_t max_resultset_size; - const char *rules; - const char *storage; - char *storage_options; - char **storage_argv; + char* rules; + char* storage; + char* storage_options; + char** storage_argv; int storage_argc; uint32_t ttl; uint32_t debug; } CACHE_CONFIG; +void cache_config_finish(CACHE_CONFIG& config); +void cache_config_free(CACHE_CONFIG* pConfig); +void cache_config_reset(CACHE_CONFIG& config); + #endif