From 8fc5bdc2f1c1750234a5965af8e1526735bc3153 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 5 Jan 2017 19:24:58 +0200 Subject: [PATCH] Perform query classifier initialization implicitly The setting up and the initialization of the query classifier has now been separated. The gateway explicitly sets up the query classifier (i.e. chooses which one to use and what arguments to provide), but the actual initialization is performed as part of the general module initialization. --- include/maxscale/query_classifier.h | 49 +++++++++++++------ query_classifier/qc_dummy/qc_dummy.cc | 28 +++++------ .../qc_mysqlembedded/qc_mysqlembedded.cc | 30 ++++++------ query_classifier/qc_sqlite/qc_sqlite.c | 30 ++++++------ .../test/canonical_tests/canonizer.c | 7 ++- query_classifier/test/classify.c | 4 +- query_classifier/test/compare.cc | 4 +- query_classifier/test/crash_qc_sqlite.c | 4 +- server/core/gateway.cc | 25 +++------- server/core/query_classifier.c | 21 +++++--- .../filter/cache/test/testkeygeneration.cc | 4 +- server/modules/filter/cache/test/testrules.cc | 4 +- .../modules/filter/cache/test/teststorage.cc | 2 +- 13 files changed, 116 insertions(+), 96 deletions(-) diff --git a/include/maxscale/query_classifier.h b/include/maxscale/query_classifier.h index 101a6ce6d..589687b3c 100644 --- a/include/maxscale/query_classifier.h +++ b/include/maxscale/query_classifier.h @@ -128,10 +128,11 @@ typedef struct qc_field_info typedef struct query_classifier { bool (*qc_setup)(const char* args); - bool (*qc_init)(void); - void (*qc_end)(void); - bool (*qc_thread_init)(void); + int (*qc_process_init)(void); + void (*qc_process_end)(void); + + int (*qc_thread_init)(void); void (*qc_thread_end)(void); qc_parse_result_t (*qc_parse)(GWBUF* stmt); @@ -152,10 +153,12 @@ typedef struct query_classifier } QUERY_CLASSIFIER; /** - * Loads and initializes the default query classifier. + * Loads and sets up the default query classifier. * * This must be called once during the execution of a process. The query - * classifier functions can only be used if this function returns true. + * classifier functions can only be used if this function first and thereafter + * the @c qc_process_init return true. + * * MaxScale calls this function, so plugins should not do that. * * @param plugin_name The name of the plugin from which the query classifier @@ -167,18 +170,32 @@ typedef struct query_classifier * * @see qc_end qc_thread_init */ -bool qc_init(const char* plugin_name, const char* plugin_args); +bool qc_setup(const char* plugin_name, const char* plugin_args); /** - * Finalizes and unloads the query classifier. + * Intializes the query classifier. + * + * This function should be called once, provided @c qc_setup returned true, + * before the query classifier functionality is used. + * + * MaxScale calls this functions, so plugins should not do that. + * + * @return True, if the process wide initialization could be performed. + * + * @see qc_process_end qc_thread_init + */ +bool qc_process_init(void); + +/** + * Finalizes the query classifier. * * A successful call of qc_init() should before program exit be followed * by a call to this function. MaxScale calls this function, so plugins * should not do that. * - * @see qc_init qc_thread_end + * @see qc_process_init qc_thread_end */ -void qc_end(void); +void qc_process_end(void); /** * Loads a particular query classifier. @@ -206,10 +223,11 @@ QUERY_CLASSIFIER* qc_load(const char* plugin_name); void qc_unload(QUERY_CLASSIFIER* classifier); /** - * Performs thread initialization needed by the query classifier. - * Should be called in every thread, except the one where qc_init() - * was called. MaxScale calls this function, so plugins should not - * do that. + * Performs thread initialization needed by the query classifier. Should + * be called in every thread, except the one where qc_process_init() + * was called. + * + * MaxScale calls this function, so plugins should not do that. * * @return True if the initialization succeeded, false otherwise. * @@ -220,8 +238,9 @@ bool qc_thread_init(void); /** * Performs thread finalization needed by the query classifier. * A successful call to qc_thread_init() should at some point be followed - * by a call to this function. MaxScale calls this function, so plugins - * should not do that. + * by a call to this function. + * + * MaxScale calls this function, so plugins should not do that. * * @see qc_thread_init */ diff --git a/query_classifier/qc_dummy/qc_dummy.cc b/query_classifier/qc_dummy/qc_dummy.cc index 719edce2e..eed448169 100644 --- a/query_classifier/qc_dummy/qc_dummy.cc +++ b/query_classifier/qc_dummy/qc_dummy.cc @@ -83,21 +83,21 @@ bool qc_setup(const char* args) return true; } -bool qc_init(void) +int qc_dummy_process_init(void) { - return true; + return 0; } -void qc_end(void) +void qc_dummy_process_end(void) { } -bool qc_thread_init(void) +int qc_dummy_thread_init(void) { - return true; + return 0; } -void qc_thread_end(void) +void qc_dummy_thread_end(void) { } @@ -108,10 +108,10 @@ extern "C" static QUERY_CLASSIFIER qc = { qc_setup, - qc_init, - qc_end, - qc_thread_init, - qc_thread_end, + qc_dummy_process_init, + qc_dummy_process_end, + qc_dummy_thread_init, + qc_dummy_thread_end, qc_parse, qc_get_type, qc_get_operation, @@ -135,10 +135,10 @@ extern "C" "Dummy Query Classifier", "V1.0.0", &qc, - NULL, /* Process init. */ - NULL, /* Process finish. */ - NULL, /* Thread init. */ - NULL, /* Thread finish. */ + qc_dummy_process_init, + qc_dummy_process_end, + qc_dummy_thread_init, + qc_dummy_thread_end, { {MXS_END_MODULE_PARAMS} } diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index 2363c8691..cc90cb5cb 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -2477,7 +2477,7 @@ void configure_options(const char* datadir, const char* langdir) server_options[IDX_DATADIR] = datadir_arg; rv = sprintf(language_arg, "--language=%s", langdir); - ss_dassert(rv < OPTIONS_LANGUAGE_SIZE); // Ensured by qc_init(). + ss_dassert(rv < OPTIONS_LANGUAGE_SIZE); // Ensured by qc_process_init(). server_options[IDX_LANGUAGE] = language_arg; // To prevent warning of unused variable when built in release mode, @@ -2498,7 +2498,7 @@ bool qc_setup(const char* args) return true; } -bool qc_init(void) +int qc_mysql_process_init(void) { bool inited = false; @@ -2530,15 +2530,15 @@ bool qc_init(void) } } - return inited; + return inited ? 0 : -1; } -void qc_end(void) +void qc_mysql_process_end(void) { mysql_library_end(); } -bool qc_thread_init(void) +int qc_mysql_thread_init(void) { bool inited = (mysql_thread_init() == 0); @@ -2547,10 +2547,10 @@ bool qc_thread_init(void) MXS_ERROR("mysql_thread_init() failed."); } - return inited; + return inited ? 0 : -1; } -void qc_thread_end(void) +void qc_mysql_thread_end(void) { mysql_thread_end(); } @@ -2567,10 +2567,10 @@ MXS_MODULE* MXS_CREATE_MODULE() static QUERY_CLASSIFIER qc = { qc_setup, - qc_init, - qc_end, - qc_thread_init, - qc_thread_end, + qc_mysql_process_init, + qc_mysql_process_end, + qc_mysql_thread_init, + qc_mysql_thread_end, qc_parse, qc_get_type, qc_get_operation, @@ -2594,10 +2594,10 @@ MXS_MODULE* MXS_CREATE_MODULE() "Query classifier based upon MySQL Embedded", "V1.0.0", &qc, - NULL, /* Process init. */ - NULL, /* Process finish. */ - NULL, /* Thread init. */ - NULL, /* Thread finish. */ + qc_mysql_process_init, + qc_mysql_process_end, + qc_mysql_thread_init, + qc_mysql_thread_end, { {MXS_END_MODULE_PARAMS} } diff --git a/query_classifier/qc_sqlite/qc_sqlite.c b/query_classifier/qc_sqlite/qc_sqlite.c index f90bc34ea..e63d8078f 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.c +++ b/query_classifier/qc_sqlite/qc_sqlite.c @@ -2598,9 +2598,9 @@ void maxscaleUse(Parse* pParse, Token* pToken) * API */ static bool qc_sqlite_setup(const char* args); -static bool qc_sqlite_init(void); -static void qc_sqlite_end(void); -static bool qc_sqlite_thread_init(void); +static int qc_sqlite_process_init(void); +static void qc_sqlite_process_end(void); +static int qc_sqlite_thread_init(void); static void qc_sqlite_thread_end(void); static qc_parse_result_t qc_sqlite_parse(GWBUF* query); static uint32_t qc_sqlite_get_type(GWBUF* query); @@ -2680,7 +2680,7 @@ static bool qc_sqlite_setup(const char* args) return this_unit.setup; } -static bool qc_sqlite_init(void) +static int qc_sqlite_process_init(void) { QC_TRACE(); assert(this_unit.setup); @@ -2692,7 +2692,7 @@ static bool qc_sqlite_init(void) this_unit.initialized = true; - if (qc_sqlite_thread_init()) + if (qc_sqlite_thread_init() == 0) { if (this_unit.log_level != QC_LOG_NOTHING) { @@ -2731,10 +2731,10 @@ static bool qc_sqlite_init(void) MXS_ERROR("Failed to initialize sqlite3."); } - return this_unit.initialized; + return this_unit.initialized ? 0 : -1; } -static void qc_sqlite_end(void) +static void qc_sqlite_process_end(void) { QC_TRACE(); ss_dassert(this_unit.initialized); @@ -2747,7 +2747,7 @@ static void qc_sqlite_end(void) this_unit.initialized = false; } -static bool qc_sqlite_thread_init(void) +static int qc_sqlite_thread_init(void) { QC_TRACE(); ss_dassert(this_unit.initialized); @@ -2798,7 +2798,7 @@ static bool qc_sqlite_thread_init(void) (unsigned long) pthread_self(), rc, sqlite3_errstr(rc)); } - return this_thread.initialized; + return this_thread.initialized ? 0 : -1; } static void qc_sqlite_thread_end(void) @@ -3190,8 +3190,8 @@ MXS_MODULE* MXS_CREATE_MODULE() static QUERY_CLASSIFIER qc = { qc_sqlite_setup, - qc_sqlite_init, - qc_sqlite_end, + qc_sqlite_process_init, + qc_sqlite_process_end, qc_sqlite_thread_init, qc_sqlite_thread_end, qc_sqlite_parse, @@ -3217,10 +3217,10 @@ MXS_MODULE* MXS_CREATE_MODULE() "Query classifier using sqlite.", "V1.0.0", &qc, - NULL, /* Process init. */ - NULL, /* Process finish. */ - NULL, /* Thread init. */ - NULL, /* Thread finish. */ + qc_sqlite_process_init, + qc_sqlite_process_end, + qc_sqlite_thread_init, + qc_sqlite_thread_end, { {MXS_END_MODULE_PARAMS} } diff --git a/query_classifier/test/canonical_tests/canonizer.c b/query_classifier/test/canonical_tests/canonizer.c index 56c59f1ea..a6d4d4db9 100644 --- a/query_classifier/test/canonical_tests/canonizer.c +++ b/query_classifier/test/canonical_tests/canonizer.c @@ -46,8 +46,8 @@ int main(int argc, char** argv) set_langdir(strdup(".")); set_process_datadir(strdup("/tmp")); - qc_init("qc_sqlite", NULL); - qc_thread_init(); + qc_setup("qc_sqlite", NULL); + qc_process_init(); infile = fopen(argv[1],"rb"); outfile = fopen(argv[2],"wb"); @@ -83,7 +83,6 @@ int main(int argc, char** argv) } fclose(infile); fclose(outfile); - qc_thread_end(); - qc_end(); + qc_process_end(); return 0; } diff --git a/query_classifier/test/classify.c b/query_classifier/test/classify.c index c656abae3..7fe74d39d 100644 --- a/query_classifier/test/classify.c +++ b/query_classifier/test/classify.c @@ -314,10 +314,10 @@ int main(int argc, char** argv) if (mxs_log_init(NULL, ".", MXS_LOG_TARGET_DEFAULT)) { - if (qc_init(lib, NULL)) + if (qc_setup(lib, NULL) && qc_process_init()) { rc = run(input_name, expected_name); - qc_end(); + qc_process_end(); } else { diff --git a/query_classifier/test/compare.cc b/query_classifier/test/compare.cc index ba7847b2c..40e558700 100644 --- a/query_classifier/test/compare.cc +++ b/query_classifier/test/compare.cc @@ -163,7 +163,7 @@ QUERY_CLASSIFIER* get_classifier(const char* zName, const char* zArgs) if (pClassifier) { - if (!pClassifier->qc_setup(zArgs) || !pClassifier->qc_init()) + if (!pClassifier->qc_setup(zArgs) || (pClassifier->qc_process_init() != 0)) { cerr << "error: Could not setup or init classifier " << zName << "." << endl; qc_unload(pClassifier); @@ -178,7 +178,7 @@ void put_classifier(QUERY_CLASSIFIER* pClassifier) { if (pClassifier) { - pClassifier->qc_end(); + pClassifier->qc_process_end(); qc_unload(pClassifier); } } diff --git a/query_classifier/test/crash_qc_sqlite.c b/query_classifier/test/crash_qc_sqlite.c index db4b67dd5..dff7cbd3d 100644 --- a/query_classifier/test/crash_qc_sqlite.c +++ b/query_classifier/test/crash_qc_sqlite.c @@ -41,7 +41,7 @@ int main() set_libdir(strdup("../qc_sqlite")); - if (qc_init("qc_sqlite", NULL)) + if (qc_setup("qc_sqlite", NULL) && qc_process_init()) { const char s[] = "SELECT @@global.max_allowed_packet"; @@ -53,7 +53,7 @@ int main() // code generator. qc_parse(stmt); - qc_end(); + qc_process_end(); rv = EXIT_SUCCESS; } diff --git a/server/core/gateway.cc b/server/core/gateway.cc index c42c9fdb3..0419545a7 100644 --- a/server/core/gateway.cc +++ b/server/core/gateway.cc @@ -976,26 +976,17 @@ void worker_thread_main(void* arg) { if (modules_thread_init()) { - if (qc_thread_init()) + /** Init mysql thread context for use with a mysql handle and a parser */ + if (mysql_thread_init() == 0) { - /** Init mysql thread context for use with a mysql handle and a parser */ - if (mysql_thread_init() == 0) - { - poll_waitevents(arg); + poll_waitevents(arg); - /** Release mysql thread context */ - mysql_thread_end(); - } - else - { - MXS_ERROR("Could not perform thread initialization for MySQL. Exiting thread."); - } - - qc_thread_end(); + /** Release mysql thread context */ + mysql_thread_end(); } else { - MXS_ERROR("Could not perform thread initialization for query classifier. Exiting thread."); + MXS_ERROR("Could not perform thread initialization for MySQL. Exiting thread."); } modules_thread_finish(); @@ -1875,7 +1866,7 @@ int main(int argc, char **argv) cnf = config_get_global_options(); ss_dassert(cnf); - if (!qc_init(cnf->qc_name, cnf->qc_args)) + if (!qc_setup(cnf->qc_name, cnf->qc_args)) { const char* logerr = "Failed to initialise query classifier library."; print_log_n_stderr(true, true, logerr, logerr, eno); @@ -2088,8 +2079,6 @@ int main(int argc, char **argv) /** Release mysql thread context*/ mysql_thread_end(); - qc_end(); - utils_end(); cleanup_process_datadir(); MXS_NOTICE("MaxScale shutdown completed."); diff --git a/server/core/query_classifier.c b/server/core/query_classifier.c index e7358f33f..4a0e2ecb8 100644 --- a/server/core/query_classifier.c +++ b/server/core/query_classifier.c @@ -38,7 +38,7 @@ static const char default_qc_name[] = "qc_sqlite"; static QUERY_CLASSIFIER* classifier; -bool qc_init(const char* plugin_name, const char* plugin_args) +bool qc_setup(const char* plugin_name, const char* plugin_args) { QC_TRACE(); ss_dassert(!classifier); @@ -56,21 +56,30 @@ bool qc_init(const char* plugin_name, const char* plugin_args) { success = classifier->qc_setup(plugin_args); - if (success) + if (!success) { - success = classifier->qc_init(); + qc_unload(classifier); + classifier = NULL; } } return success; } -void qc_end(void) +bool qc_process_init(void) { QC_TRACE(); ss_dassert(classifier); - classifier->qc_end(); + return classifier->qc_process_init() == 0; +} + +void qc_process_end(void) +{ + QC_TRACE(); + ss_dassert(classifier); + + classifier->qc_process_end(); classifier = NULL; } @@ -101,7 +110,7 @@ bool qc_thread_init(void) QC_TRACE(); ss_dassert(classifier); - return classifier->qc_thread_init(); + return classifier->qc_thread_init() == 0; } void qc_thread_end(void) diff --git a/server/modules/filter/cache/test/testkeygeneration.cc b/server/modules/filter/cache/test/testkeygeneration.cc index d2bba2f85..aedc3501b 100644 --- a/server/modules/filter/cache/test/testkeygeneration.cc +++ b/server/modules/filter/cache/test/testkeygeneration.cc @@ -127,7 +127,7 @@ int main(int argc, char* argv[]) { if (mxs_log_init(NULL, ".", MXS_LOG_TARGET_DEFAULT)) { - if (qc_init(NULL, NULL)) + if (qc_setup(NULL, NULL) && qc_process_init()) { const char* zModule = argv[1]; @@ -157,6 +157,8 @@ int main(int argc, char* argv[]) { cerr << "error: Could not initialize factory." << endl; } + + qc_process_end(); } else { diff --git a/server/modules/filter/cache/test/testrules.cc b/server/modules/filter/cache/test/testrules.cc index c613305a8..c73157a79 100644 --- a/server/modules/filter/cache/test/testrules.cc +++ b/server/modules/filter/cache/test/testrules.cc @@ -237,10 +237,12 @@ int main() if (mxs_log_init(NULL, ".", MXS_LOG_TARGET_DEFAULT)) { set_libdir(MXS_STRDUP_A("../../../../../query_classifier/qc_sqlite/")); - if (qc_init("qc_sqlite", "")) + if (qc_setup("qc_sqlite", "") && qc_process_init()) { set_libdir(MXS_STRDUP_A("../")); rc = test(); + + qc_process_end(); } else { diff --git a/server/modules/filter/cache/test/teststorage.cc b/server/modules/filter/cache/test/teststorage.cc index c7dc68d1c..d6cffc687 100644 --- a/server/modules/filter/cache/test/teststorage.cc +++ b/server/modules/filter/cache/test/teststorage.cc @@ -50,7 +50,7 @@ int TestStorage::run(int argc, char** argv) { if (mxs_log_init(NULL, ".", MXS_LOG_TARGET_DEFAULT)) { - if (qc_init(NULL, NULL)) + if (qc_setup(NULL, NULL) && qc_process_init()) { const char* zModule = NULL; size_t threads = m_threads;