MXS-2026 Separate QC process and thread initialization

qc_thread_init() must now explicitly be called in every thread
and not just in other threads but the one where qc_process_init()
is called.

This change was caused by QC_INIT_SELF initialization actually
being performed in query_classifier.cc. Before this change, there
actually was a leak in the routing worker running in the main
thread, the query classification cache was created twice.
This commit is contained in:
Johan Wikman
2018-08-27 15:39:38 +03:00
parent 9a6f1b2044
commit 5c1a1c2700
6 changed files with 48 additions and 43 deletions

View File

@ -198,16 +198,14 @@ typedef struct query_classifier
void (*qc_process_end)(void); void (*qc_process_end)(void);
/** /**
* Called once per each thread, except for the thread for which @c qc_process_init * Called once per each thread.
* was called.
* *
* @return QC_RESULT_OK, if the thread initialization succeeded. * @return QC_RESULT_OK, if the thread initialization succeeded.
*/ */
int32_t (*qc_thread_init)(void); int32_t (*qc_thread_init)(void);
/** /**
* Called once when a thread finishes, except for the thread for which @c qc_process_init * Called once when a thread finishes.
* was called.
*/ */
void (*qc_thread_end)(void); void (*qc_thread_end)(void);
@ -499,9 +497,9 @@ bool qc_process_init(uint32_t kind);
/** /**
* Finalizes the query classifier. * Finalizes the query classifier.
* *
* A successful call of qc_init() should before program exit be followed * A successful call of @c qc_process_init should before program exit be
* by a call to this function. MaxScale calls this function, so plugins * followed by a call to this function. MaxScale calls this function, so
* should not do that. * plugins should not do that.
* *
* @param kind What kind of finalization should be performed. * @param kind What kind of finalization should be performed.
* Combination of qc_init_kind_t. * Combination of qc_init_kind_t.
@ -537,8 +535,7 @@ void qc_unload(QUERY_CLASSIFIER* classifier);
/** /**
* Performs thread initialization needed by the query classifier. Should * Performs thread initialization needed by the query classifier. Should
* be called in every thread, except the one where qc_process_init() * be called in every thread.
* was called.
* *
* MaxScale calls this function, so plugins should not do that. * MaxScale calls this function, so plugins should not do that.
* *
@ -553,8 +550,8 @@ bool qc_thread_init(uint32_t kind);
/** /**
* Performs thread finalization needed by the query classifier. * Performs thread finalization needed by the query classifier.
* A successful call to qc_thread_init() should at some point be followed * A successful call to @c qc_thread_init should at some point be
* by a call to this function. * followed by a call to this function.
* *
* MaxScale calls this function, so plugins should not do that. * MaxScale calls this function, so plugins should not do that.
* *

View File

@ -317,9 +317,9 @@ int main(int argc, char** argv)
if (mxs_log_init(NULL, ".", MXS_LOG_TARGET_DEFAULT)) if (mxs_log_init(NULL, ".", MXS_LOG_TARGET_DEFAULT))
{ {
if (qc_setup(NULL, QC_SQL_MODE_DEFAULT, lib, NULL) && if (qc_setup(NULL, QC_SQL_MODE_DEFAULT, lib, NULL)
qc_process_init(QC_INIT_BOTH) && && qc_process_init(QC_INIT_BOTH)
qc_thread_init(QC_INIT_BOTH)) && qc_thread_init(QC_INIT_BOTH))
{ {
// Version encoded as MariaDB encodes the version, i.e.: // Version encoded as MariaDB encodes the version, i.e.:
// version = major * 10000 + minor * 100 + patch // version = major * 10000 + minor * 100 + patch
@ -327,6 +327,8 @@ int main(int argc, char** argv)
qc_set_server_version(version); qc_set_server_version(version);
rc = run(input_name, expected_name); rc = run(input_name, expected_name);
qc_thread_end(QC_INIT_BOTH);
qc_process_end(QC_INIT_BOTH); qc_process_end(QC_INIT_BOTH);
} }
else else

View File

@ -441,19 +441,11 @@ bool qc_process_init(uint32_t kind)
} }
} }
bool rc = qc_thread_init(QC_INIT_SELF); bool rc = true;
if (rc) if (kind & QC_INIT_PLUGIN)
{ {
if (kind & QC_INIT_PLUGIN) rc = this_unit.classifier->qc_process_init() == 0;
{
rc = this_unit.classifier->qc_process_init() == 0;
if (!rc)
{
qc_thread_end(QC_INIT_SELF);
}
}
} }
return rc; return rc;
@ -468,8 +460,6 @@ void qc_process_end(uint32_t kind)
{ {
this_unit.classifier->qc_process_end(); this_unit.classifier->qc_process_end();
} }
qc_thread_end(QC_INIT_SELF);
} }
QUERY_CLASSIFIER* qc_load(const char* plugin_name) QUERY_CLASSIFIER* qc_load(const char* plugin_name)
@ -502,12 +492,19 @@ bool qc_thread_init(uint32_t kind)
bool rc = false; bool rc = false;
this_thread.pInfo_cache = new (std::nothrow) QCInfoCache; if (kind & QC_INIT_SELF)
{
if (this_thread.pInfo_cache) mxb_assert(!this_thread.pInfo_cache);
this_thread.pInfo_cache = new (std::nothrow) QCInfoCache;
rc = true;
}
else
{ {
rc = true; rc = true;
}
if (rc)
{
if (kind & QC_INIT_PLUGIN) if (kind & QC_INIT_PLUGIN)
{ {
rc = this_unit.classifier->qc_thread_init() == 0; rc = this_unit.classifier->qc_thread_init() == 0;
@ -515,8 +512,11 @@ bool qc_thread_init(uint32_t kind)
if (!rc) if (!rc)
{ {
delete this_thread.pInfo_cache; if (kind & QC_INIT_SELF)
this_thread.pInfo_cache = nullptr; {
delete this_thread.pInfo_cache;
this_thread.pInfo_cache = nullptr;
}
} }
} }
@ -533,8 +533,11 @@ void qc_thread_end(uint32_t kind)
this_unit.classifier->qc_thread_end(); this_unit.classifier->qc_thread_end();
} }
delete this_thread.pInfo_cache; if (kind & QC_INIT_SELF)
this_thread.pInfo_cache = nullptr; {
delete this_thread.pInfo_cache;
this_thread.pInfo_cache = nullptr;
}
} }
qc_parse_result_t qc_parse(GWBUF* query, uint32_t collect) qc_parse_result_t qc_parse(GWBUF* query, uint32_t collect)

View File

@ -186,9 +186,9 @@ int main(int argc, char* argv[])
set_libdir(strdup("../../../query_classifier/qc_sqlite")); set_libdir(strdup("../../../query_classifier/qc_sqlite"));
// We have to setup something in order for the regexes to be compiled. // We have to setup something in order for the regexes to be compiled.
if (qc_setup(NULL, QC_SQL_MODE_DEFAULT, "qc_sqlite", NULL) && if (qc_setup(NULL, QC_SQL_MODE_DEFAULT, "qc_sqlite", NULL)
qc_process_init(QC_INIT_BOTH) && && qc_process_init(QC_INIT_BOTH)
qc_thread_init(QC_INIT_BOTH)) && qc_thread_init(QC_INIT_BOTH))
{ {
Tester tester(verbosity); Tester tester(verbosity);
@ -218,6 +218,7 @@ int main(int argc, char* argv[])
} }
} }
qc_thread_end(QC_INIT_BOTH);
qc_process_end(QC_INIT_BOTH); qc_process_end(QC_INIT_BOTH);
} }
else else

View File

@ -423,9 +423,9 @@ int main(int argc, char* argv[])
set_libdir(strdup("../../../query_classifier/qc_sqlite")); set_libdir(strdup("../../../query_classifier/qc_sqlite"));
// We have to setup something in order for the regexes to be compiled. // We have to setup something in order for the regexes to be compiled.
if (qc_setup(NULL, QC_SQL_MODE_DEFAULT, "qc_sqlite", NULL) && if (qc_setup(NULL, QC_SQL_MODE_DEFAULT, "qc_sqlite", NULL)
qc_process_init(QC_INIT_BOTH) && && qc_process_init(QC_INIT_BOTH)
qc_thread_init(QC_INIT_BOTH)) && qc_thread_init(QC_INIT_BOTH))
{ {
rc = EXIT_SUCCESS; rc = EXIT_SUCCESS;
@ -451,6 +451,7 @@ int main(int argc, char* argv[])
cout << endl; cout << endl;
} }
qc_thread_end(QC_INIT_BOTH);
qc_process_end(QC_INIT_BOTH); qc_process_end(QC_INIT_BOTH);
} }
else else

View File

@ -397,13 +397,14 @@ int main()
pConfig->n_threads = 1; pConfig->n_threads = 1;
set_libdir(MXS_STRDUP_A("../../../../../query_classifier/qc_sqlite/")); set_libdir(MXS_STRDUP_A("../../../../../query_classifier/qc_sqlite/"));
if (qc_setup(NULL, QC_SQL_MODE_DEFAULT, "qc_sqlite", "") && if (qc_setup(NULL, QC_SQL_MODE_DEFAULT, "qc_sqlite", "")
qc_process_init(QC_INIT_BOTH) && && qc_process_init(QC_INIT_BOTH)
qc_thread_init(QC_INIT_BOTH)) && qc_thread_init(QC_INIT_BOTH))
{ {
set_libdir(MXS_STRDUP_A("../")); set_libdir(MXS_STRDUP_A("../"));
rc = test(); rc = test();
qc_thread_end(QC_INIT_BOTH);
qc_process_end(QC_INIT_BOTH); qc_process_end(QC_INIT_BOTH);
} }
else else