From ab487e687f79facbb61c23556824dcadffb8ddca Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 1 Nov 2016 16:06:51 +0200 Subject: [PATCH 1/8] qc: Remove QUERY_IS_TYPE macro Explicit call to qc_query_is_type() instead. --- include/maxscale/query_classifier.h | 8 -- .../qc_mysqlembedded/qc_mysqlembedded.cc | 12 +- .../readwritesplit/rwsplit_route_stmt.c | 104 +++++++++--------- .../readwritesplit/rwsplit_tmp_table_multi.c | 12 +- .../routing/schemarouter/schemarouter.c | 32 +++--- 5 files changed, 80 insertions(+), 88 deletions(-) diff --git a/include/maxscale/query_classifier.h b/include/maxscale/query_classifier.h index b1b4a9f18..438613eac 100644 --- a/include/maxscale/query_classifier.h +++ b/include/maxscale/query_classifier.h @@ -408,12 +408,4 @@ const char* qc_type_to_string(qc_query_type_t type); */ char* qc_typemask_to_string(uint32_t typemask); -/** - * @deprecated - * Synonym for qc_query_is_type(). - * - * @see qc_query_is_type - */ -#define QUERY_IS_TYPE(typemask, type) qc_query_is_type(typemask, type) - MXS_END_DECLS diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index dd5789b59..48330808e 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -681,12 +681,12 @@ static uint32_t resolve_query_type(THD* thd) #endif // TODO: This test is meaningless, since at this point // TODO: qtype (not type) is QUERY_TYPE_UNKNOWN. - if (QUERY_IS_TYPE(qtype, QUERY_TYPE_UNKNOWN) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_LOCAL_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ)) + if (qc_query_is_type(qtype, QUERY_TYPE_UNKNOWN) || + qc_query_is_type(qtype, QUERY_TYPE_LOCAL_READ) || + qc_query_is_type(qtype, QUERY_TYPE_READ) || + qc_query_is_type(qtype, QUERY_TYPE_USERVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_SYSVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_GSYSVAR_READ)) { /** * These values won't change qtype more restrictive than write. diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.c b/server/modules/routing/readwritesplit/rwsplit_route_stmt.c index 73f0120f0..cdaddc5ca 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.c +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.c @@ -84,7 +84,7 @@ bool route_single_stmt(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses, * transaction is committed and autocommit is enabled again. */ if (rses->rses_autocommit_enabled && - QUERY_IS_TYPE(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT)) + qc_query_is_type(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT)) { rses->rses_autocommit_enabled = false; @@ -94,7 +94,7 @@ bool route_single_stmt(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses, } } else if (!rses->rses_transaction_active && - QUERY_IS_TYPE(qtype, QUERY_TYPE_BEGIN_TRX)) + qc_query_is_type(qtype, QUERY_TYPE_BEGIN_TRX)) { rses->rses_transaction_active = true; } @@ -102,13 +102,13 @@ bool route_single_stmt(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses, * Explicit COMMIT and ROLLBACK, implicit COMMIT. */ if (rses->rses_autocommit_enabled && rses->rses_transaction_active && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_COMMIT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_ROLLBACK))) + (qc_query_is_type(qtype, QUERY_TYPE_COMMIT) || + qc_query_is_type(qtype, QUERY_TYPE_ROLLBACK))) { rses->rses_transaction_active = false; } else if (!rses->rses_autocommit_enabled && - QUERY_IS_TYPE(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT)) + qc_query_is_type(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT)) { rses->rses_autocommit_enabled = true; rses->rses_transaction_active = false; @@ -727,13 +727,13 @@ route_target_t get_route_target(ROUTER_CLIENT_SES *rses, * These queries are not affected by hints */ else if (!load_active && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_SESSION_WRITE) || + (qc_query_is_type(qtype, QUERY_TYPE_SESSION_WRITE) || /** Configured to allow writing variables to all nodes */ (use_sql_variables_in == TYPE_ALL && - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_WRITE)) || + qc_query_is_type(qtype, QUERY_TYPE_GSYSVAR_WRITE)) || /** enable or disable autocommit are always routed to all */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT))) + qc_query_is_type(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) || + qc_query_is_type(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT))) { /** * This is problematic query because it would be routed to all @@ -750,9 +750,9 @@ route_target_t get_route_target(ROUTER_CLIENT_SES *rses, * the execution of the prepared statements to the right server would be * an easy one. Currently this is not supported. */ - if (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) && - !(QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_NAMED_STMT))) + if (qc_query_is_type(qtype, QUERY_TYPE_READ) && + !(qc_query_is_type(qtype, QUERY_TYPE_PREPARE_STMT) || + qc_query_is_type(qtype, QUERY_TYPE_PREPARE_NAMED_STMT))) { MXS_WARNING("The query can't be routed to all " "backend servers because it includes SELECT and " @@ -770,40 +770,40 @@ route_target_t get_route_target(ROUTER_CLIENT_SES *rses, * Hints may affect on routing of the following queries */ else if (!trx_active && !load_active && - !QUERY_IS_TYPE(qtype, QUERY_TYPE_WRITE) && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) || /*< any SELECT */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_SHOW_TABLES) || /*< 'SHOW TABLES' */ - QUERY_IS_TYPE(qtype, - QUERY_TYPE_USERVAR_READ) || /*< read user var */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || /*< read sys var */ - QUERY_IS_TYPE(qtype, - QUERY_TYPE_EXEC_STMT) || /*< prepared stmt exec */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || - QUERY_IS_TYPE(qtype, - QUERY_TYPE_GSYSVAR_READ))) /*< read global sys var */ + !qc_query_is_type(qtype, QUERY_TYPE_WRITE) && + (qc_query_is_type(qtype, QUERY_TYPE_READ) || /*< any SELECT */ + qc_query_is_type(qtype, QUERY_TYPE_SHOW_TABLES) || /*< 'SHOW TABLES' */ + qc_query_is_type(qtype, + QUERY_TYPE_USERVAR_READ) || /*< read user var */ + qc_query_is_type(qtype, QUERY_TYPE_SYSVAR_READ) || /*< read sys var */ + qc_query_is_type(qtype, + QUERY_TYPE_EXEC_STMT) || /*< prepared stmt exec */ + qc_query_is_type(qtype, QUERY_TYPE_PREPARE_STMT) || + qc_query_is_type(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || + qc_query_is_type(qtype, + QUERY_TYPE_GSYSVAR_READ))) /*< read global sys var */ { /** First set expected targets before evaluating hints */ - if (!QUERY_IS_TYPE(qtype, QUERY_TYPE_MASTER_READ) && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SHOW_TABLES) || /*< 'SHOW TABLES' */ + if (!qc_query_is_type(qtype, QUERY_TYPE_MASTER_READ) && + (qc_query_is_type(qtype, QUERY_TYPE_READ) || + qc_query_is_type(qtype, QUERY_TYPE_SHOW_TABLES) || /*< 'SHOW TABLES' */ /** Configured to allow reading variables from slaves */ (use_sql_variables_in == TYPE_ALL && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ))))) + (qc_query_is_type(qtype, QUERY_TYPE_USERVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_SYSVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_GSYSVAR_READ))))) { target = TARGET_SLAVE; } - if (QUERY_IS_TYPE(qtype, QUERY_TYPE_MASTER_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_EXEC_STMT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || + if (qc_query_is_type(qtype, QUERY_TYPE_MASTER_READ) || + qc_query_is_type(qtype, QUERY_TYPE_EXEC_STMT) || + qc_query_is_type(qtype, QUERY_TYPE_PREPARE_STMT) || + qc_query_is_type(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || /** Configured not to allow reading variables from slaves */ (use_sql_variables_in == TYPE_MASTER && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ)))) + (qc_query_is_type(qtype, QUERY_TYPE_USERVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_SYSVAR_READ)))) { target = TARGET_MASTER; } @@ -818,26 +818,26 @@ route_target_t get_route_target(ROUTER_CLIENT_SES *rses, { /** hints don't affect on routing */ ss_dassert(trx_active || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_WRITE) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_MASTER_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SESSION_WRITE) || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) && + (qc_query_is_type(qtype, QUERY_TYPE_WRITE) || + qc_query_is_type(qtype, QUERY_TYPE_MASTER_READ) || + qc_query_is_type(qtype, QUERY_TYPE_SESSION_WRITE) || + (qc_query_is_type(qtype, QUERY_TYPE_USERVAR_READ) && use_sql_variables_in == TYPE_MASTER) || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) && + (qc_query_is_type(qtype, QUERY_TYPE_SYSVAR_READ) && use_sql_variables_in == TYPE_MASTER) || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ) && + (qc_query_is_type(qtype, QUERY_TYPE_GSYSVAR_READ) && use_sql_variables_in == TYPE_MASTER) || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_WRITE) && + (qc_query_is_type(qtype, QUERY_TYPE_GSYSVAR_WRITE) && use_sql_variables_in == TYPE_MASTER) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_BEGIN_TRX) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_ROLLBACK) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_COMMIT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_EXEC_STMT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_CREATE_TMP_TABLE) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_READ_TMP_TABLE) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_UNKNOWN))); + qc_query_is_type(qtype, QUERY_TYPE_BEGIN_TRX) || + qc_query_is_type(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) || + qc_query_is_type(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT) || + qc_query_is_type(qtype, QUERY_TYPE_ROLLBACK) || + qc_query_is_type(qtype, QUERY_TYPE_COMMIT) || + qc_query_is_type(qtype, QUERY_TYPE_EXEC_STMT) || + qc_query_is_type(qtype, QUERY_TYPE_CREATE_TMP_TABLE) || + qc_query_is_type(qtype, QUERY_TYPE_READ_TMP_TABLE) || + qc_query_is_type(qtype, QUERY_TYPE_UNKNOWN))); target = TARGET_MASTER; } diff --git a/server/modules/routing/readwritesplit/rwsplit_tmp_table_multi.c b/server/modules/routing/readwritesplit/rwsplit_tmp_table_multi.c index a82945982..e008ed031 100644 --- a/server/modules/routing/readwritesplit/rwsplit_tmp_table_multi.c +++ b/server/modules/routing/readwritesplit/rwsplit_tmp_table_multi.c @@ -147,11 +147,11 @@ qc_query_type_t is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, dbname = (char *)data->db; - if (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_LOCAL_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ)) + if (qc_query_is_type(qtype, QUERY_TYPE_READ) || + qc_query_is_type(qtype, QUERY_TYPE_LOCAL_READ) || + qc_query_is_type(qtype, QUERY_TYPE_USERVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_SYSVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_GSYSVAR_READ)) { tbl = qc_get_table_names(querybuf, &tsize, false); @@ -199,7 +199,7 @@ qc_query_type_t is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, void check_create_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, GWBUF *querybuf, qc_query_type_t type) { - if (!QUERY_IS_TYPE(type, QUERY_TYPE_CREATE_TMP_TABLE)) + if (!qc_query_is_type(type, QUERY_TYPE_CREATE_TMP_TABLE)) { return; } diff --git a/server/modules/routing/schemarouter/schemarouter.c b/server/modules/routing/schemarouter/schemarouter.c index 7ef6d8681..ce6bcecec 100644 --- a/server/modules/routing/schemarouter/schemarouter.c +++ b/server/modules/routing/schemarouter/schemarouter.c @@ -524,7 +524,7 @@ char* get_shard_target_name(ROUTER_INSTANCE* router, /* Check if the query is a show tables query with a specific database */ - if (QUERY_IS_TYPE(qtype, QUERY_TYPE_SHOW_TABLES)) + if (qc_query_is_type(qtype, QUERY_TYPE_SHOW_TABLES)) { query = modutil_get_SQL(buffer); if ((tmp = strcasestr(query, "from"))) @@ -1457,19 +1457,19 @@ static route_target_t get_shard_route_target(qc_query_type_t qtype, /** * These queries are not affected by hints */ - if (QUERY_IS_TYPE(qtype, QUERY_TYPE_SESSION_WRITE) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_WRITE) || + if (qc_query_is_type(qtype, QUERY_TYPE_SESSION_WRITE) || + qc_query_is_type(qtype, QUERY_TYPE_PREPARE_STMT) || + qc_query_is_type(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || + qc_query_is_type(qtype, QUERY_TYPE_GSYSVAR_WRITE) || /** enable or disable autocommit are always routed to all */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT)) + qc_query_is_type(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) || + qc_query_is_type(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT)) { /** hints don't affect on routing */ target = TARGET_ALL; } - else if (QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ)) + else if (qc_query_is_type(qtype, QUERY_TYPE_SYSVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_GSYSVAR_READ)) { target = TARGET_ANY; } @@ -1559,11 +1559,11 @@ qc_query_type_t is_read_tmp_table(ROUTER* instance, rses_prop_tmp = router_cli_ses->rses_properties[RSES_PROP_TYPE_TMPTABLES]; dbname = router_cli_ses->current_db; - if (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_LOCAL_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ)) + if (qc_query_is_type(qtype, QUERY_TYPE_READ) || + qc_query_is_type(qtype, QUERY_TYPE_LOCAL_READ) || + qc_query_is_type(qtype, QUERY_TYPE_USERVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_SYSVAR_READ) || + qc_query_is_type(qtype, QUERY_TYPE_GSYSVAR_READ)) { tbl = qc_get_table_names(querybuf, &tsize, false); @@ -1633,7 +1633,7 @@ void check_create_tmp_table(ROUTER* instance, rses_prop_tmp = router_cli_ses->rses_properties[RSES_PROP_TYPE_TMPTABLES]; dbname = router_cli_ses->current_db; - if (QUERY_IS_TYPE(type, QUERY_TYPE_CREATE_TMP_TABLE)) + if (qc_query_is_type(type, QUERY_TYPE_CREATE_TMP_TABLE)) { bool is_temp = true; char* tblname = NULL; @@ -2081,7 +2081,7 @@ static int routeQuery(ROUTER* instance, } /** Create the response to the SHOW DATABASES from the mapped databases */ - if (QUERY_IS_TYPE(qtype, QUERY_TYPE_SHOW_DATABASES)) + if (qc_query_is_type(qtype, QUERY_TYPE_SHOW_DATABASES)) { if (send_database_list(inst, router_cli_ses)) { From c652f1330a4626a73fa96434110c37c612befedd Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 1 Nov 2016 16:34:28 +0200 Subject: [PATCH 2/8] qc: Add qc_get_field_info This function returns more detailed information about the fields of a statement. Supersedes qc_get_affected_fields() that will be deprecated and removed. Note that this function now introduced new kind of behaviour; the returned data belongs to the GWBUF and remains valid for as long as the GWBUF is alive. That means that unnecessary copying need not be done. --- include/maxscale/query_classifier.h | 28 +++++++++++++++++++ .../qc_mysqlembedded/qc_mysqlembedded.cc | 9 ++++++ query_classifier/qc_sqlite/qc_sqlite.c | 28 +++++++++++++++++++ server/core/query_classifier.c | 8 ++++++ 4 files changed, 73 insertions(+) diff --git a/include/maxscale/query_classifier.h b/include/maxscale/query_classifier.h index 438613eac..02db99d5d 100644 --- a/include/maxscale/query_classifier.h +++ b/include/maxscale/query_classifier.h @@ -87,6 +87,18 @@ typedef enum qc_parse_result } qc_parse_result_t; +/** + * QC_FIELD_INFO contains information about a field used in a statement. + */ +typedef struct qc_field_info +{ + char* database; /** Present if the field is of the form "a.b.c", NULL otherwise. */ + char* table; /** Present if the field is of the form "a.b", NULL otherwise. */ + char* column; /** Always present. */ + // TODO: Possibly add bits telling where the field is used; e.g. in the select + // TODO: part or the where part, or both. +} QC_FIELD_INFO; + /** * QUERY_CLASSIFIER defines the object a query classifier plugin must * implement and return. @@ -117,6 +129,7 @@ typedef struct query_classifier char** (*qc_get_database_names)(GWBUF* stmt, int* size); char* (*qc_get_prepare_name)(GWBUF* stmt); qc_query_op_t (*qc_get_prepare_operation)(GWBUF* stmt); + void (*qc_get_field_info)(GWBUF* stmt, const QC_FIELD_INFO** infos, size_t* n_infos); } QUERY_CLASSIFIER; /** @@ -223,6 +236,21 @@ qc_parse_result_t qc_parse(GWBUF* stmt); */ char* qc_get_affected_fields(GWBUF* stmt); +/** + * Returns information about affected fields. + * + * @param stmt A buffer containing a COM_QUERY packet. + * @param infos Pointer to pointer that after the call will point to an + * array of QC_FIELD_INFO:s. + * @param n_infos Pointer to size_t variable where the number of items + * in @c infos will be returned. + * + * @note The returned array belongs to the GWBUF and remains valid for as + * long as the GWBUF is valid. If the data is needed for longer than + * that, it must be copied. + */ +void qc_get_field_info(GWBUF* stmt, const QC_FIELD_INFO** infos, size_t* n_infos); + /** * Returns the statement, with literals replaced with question marks. * diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index 48330808e..1041b6d44 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -2010,6 +2010,14 @@ qc_query_op_t qc_get_prepare_operation(GWBUF* stmt) return operation; } +void qc_get_field_info(GWBUF* stmt, const QC_FIELD_INFO** infos, size_t* n_infos) +{ + MXS_ERROR("qc_get_field_info not implemented yet."); + + *infos = NULL; + *n_infos = 0; +} + namespace { @@ -2156,6 +2164,7 @@ static QUERY_CLASSIFIER qc = qc_get_database_names, qc_get_prepare_name, qc_get_prepare_operation, + qc_get_field_info, }; /* @see function load_module in load_utils.c for explanation of the following diff --git a/query_classifier/qc_sqlite/qc_sqlite.c b/query_classifier/qc_sqlite/qc_sqlite.c index ccec8c165..d73d06bea 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.c +++ b/query_classifier/qc_sqlite/qc_sqlite.c @@ -248,6 +248,21 @@ static bool ensure_query_is_parsed(GWBUF* query) return parsed; } +void free_field_infos(QC_FIELD_INFO* infos, size_t n_infos) +{ + if (infos) + { + for (int i = 0; i < n_infos; ++i) + { + MXS_FREE(infos[i].database); + MXS_FREE(infos[i].table); + MXS_FREE(infos[i].column); + } + + MXS_FREE(infos); + } +} + static void free_string_array(char** sa) { if (sa) @@ -2954,6 +2969,18 @@ static qc_query_op_t qc_sqlite_get_prepare_operation(GWBUF* query) return op; } +void qc_sqlite_get_field_info(GWBUF* stmt, const QC_FIELD_INFO** infos, size_t* n_infos) +{ + QC_TRACE(); + ss_dassert(this_unit.initialized); + ss_dassert(this_thread.initialized); + + MXS_ERROR("qc_get_field_info not implemented yet."); + + *infos = NULL; + *n_infos = 0; +} + /** * EXPORTS */ @@ -2979,6 +3006,7 @@ static QUERY_CLASSIFIER qc = qc_sqlite_get_database_names, qc_sqlite_get_prepare_name, qc_sqlite_get_prepare_operation, + qc_sqlite_get_field_info, }; diff --git a/server/core/query_classifier.c b/server/core/query_classifier.c index 5800f38e1..70d7f3cfe 100644 --- a/server/core/query_classifier.c +++ b/server/core/query_classifier.c @@ -197,6 +197,14 @@ char* qc_get_affected_fields(GWBUF* query) return classifier->qc_get_affected_fields(query); } +void qc_get_field_info(GWBUF* query, const QC_FIELD_INFO** infos, size_t* n_infos) +{ + QC_TRACE(); + ss_dassert(classifier); + + classifier->qc_get_field_info(query, infos, n_infos); +} + char** qc_get_database_names(GWBUF* query, int* sizep) { QC_TRACE(); From af65ee0ef99ee7ce5cbacddd007d7e22222bd535 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 3 Nov 2016 08:30:45 +0200 Subject: [PATCH 3/8] qc: Ensure type is uint32_t Some C++ compiler complains about signed being compared with unsigned. --- include/maxscale/query_classifier.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/maxscale/query_classifier.h b/include/maxscale/query_classifier.h index 02db99d5d..00a7688c5 100644 --- a/include/maxscale/query_classifier.h +++ b/include/maxscale/query_classifier.h @@ -401,7 +401,7 @@ const char* qc_op_to_string(qc_query_op_t op); */ static inline bool qc_query_is_type(uint32_t typemask, qc_query_type_t type) { - return (typemask & type) == type; + return (typemask & (uint32_t)type) == (uint32_t)type; } /** From 02d28a8d8d242b2dc7b6cc98790727205a3d1798 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 3 Nov 2016 08:37:24 +0200 Subject: [PATCH 4/8] Always use format strings in dcb_printf The luafilter didn't use a format string with dcb_printf which can lead to unexpected results if the returned string contains printf special characters. --- server/modules/filter/luafilter/luafilter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/filter/luafilter/luafilter.c b/server/modules/filter/luafilter/luafilter.c index 137446f8c..dbfe5b2ae 100644 --- a/server/modules/filter/luafilter/luafilter.c +++ b/server/modules/filter/luafilter/luafilter.c @@ -570,7 +570,7 @@ static void diagnostic(FILTER *instance, void *fsession, DCB *dcb) lua_gettop(my_instance->global_lua_state); if (lua_isstring(my_instance->global_lua_state, -1)) { - dcb_printf(dcb, lua_tostring(my_instance->global_lua_state, -1)); + dcb_printf(dcb, "%s", lua_tostring(my_instance->global_lua_state, -1)); dcb_printf(dcb, "\n"); } } From 45f463c438271097a0c112d42b31f1198f0f307b Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 3 Nov 2016 09:19:28 +0200 Subject: [PATCH 5/8] Require GCC 4.8 for RocksDB GCC 4.8 is the first GCC version that fully implements C++11 --- .../filter/cache/storage/storage_rocksdb/BuildRocksDB.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/modules/filter/cache/storage/storage_rocksdb/BuildRocksDB.cmake b/server/modules/filter/cache/storage/storage_rocksdb/BuildRocksDB.cmake index bfe2aa37e..bdc630b4e 100644 --- a/server/modules/filter/cache/storage/storage_rocksdb/BuildRocksDB.cmake +++ b/server/modules/filter/cache/storage/storage_rocksdb/BuildRocksDB.cmake @@ -1,7 +1,7 @@ # Build RocksDB -if ((CMAKE_CXX_COMPILER_ID STREQUAL "GNU") AND (NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.7))) - message(STATUS "GCC >= 4.7, RocksDB is built.") +if ((CMAKE_CXX_COMPILER_ID STREQUAL "GNU") AND (NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.8))) + message(STATUS "GCC >= 4.8, RocksDB is built.") set(ROCKSDB_REPO "https://github.com/facebook/rocksdb.git" CACHE STRING "RocksDB Git repository") From 8778e0c81e5f771ac2af730085e81f54d834da0e Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 3 Nov 2016 09:35:49 +0200 Subject: [PATCH 6/8] Make Users const correct --- include/maxscale/users.h | 22 ++++++++-------- server/core/test/testusers.c | 6 ++--- server/core/users.c | 25 ++++++++++--------- server/modules/authenticator/cdc_plain_auth.c | 2 +- 4 files changed, 28 insertions(+), 27 deletions(-) diff --git a/include/maxscale/users.h b/include/maxscale/users.h index cdd919de4..d037d6cf4 100644 --- a/include/maxscale/users.h +++ b/include/maxscale/users.h @@ -61,16 +61,16 @@ typedef struct users unsigned char cksum[SHA_DIGEST_LENGTH]; /**< The users' table ckecksum */ } USERS; -extern USERS *users_alloc(); /**< Allocate a users table */ -extern void users_free(USERS *); /**< Free a users table */ -extern int users_add(USERS *, char *, char *); /**< Add a user to the users table */ -extern int users_delete(USERS *, char *); /**< Delete a user from the users table */ -extern char *users_fetch(USERS *, char *); /**< Fetch the authentication data for a user */ -extern int users_update(USERS *, char *, char *); /**< Change the password data for a user in - the users table */ -extern int users_default_loadusers(SERV_LISTENER *port); /**< A generic implementation of the authenticator - * loadusers entry point */ -extern void usersPrint(USERS *); /**< Print data about the users loaded */ -extern void dcb_usersPrint(DCB *, USERS *); /**< Print data about the users loaded */ +extern USERS *users_alloc(); /**< Allocate a users table */ +extern void users_free(USERS *); /**< Free a users table */ +extern int users_add(USERS *, const char *, const char *); /**< Add a user to the users table */ +extern int users_delete(USERS *, const char *); /**< Delete a user from the users table */ +extern const char *users_fetch(USERS *, const char *); /**< Fetch the authentication data for a user*/ +extern int users_update(USERS *, const char *, const char *); /**< Change the password data for a user in + the users table */ +extern int users_default_loadusers(SERV_LISTENER *port); /**< A generic implementation of the + authenticator loadusers entry point */ +extern void usersPrint(const USERS *); /**< Print data about the users loaded */ +extern void dcb_usersPrint(DCB *, const USERS *); /**< Print data about the users loaded */ MXS_END_DECLS diff --git a/server/core/test/testusers.c b/server/core/test/testusers.c index f6bea1ef6..5986e5a00 100644 --- a/server/core/test/testusers.c +++ b/server/core/test/testusers.c @@ -45,9 +45,9 @@ static int test1() { - USERS *users; - char *authdata; - int result, count; + USERS *users; + const char *authdata; + int result, count; /* Poll tests */ ss_dfprintf(stderr, diff --git a/server/core/users.c b/server/core/users.c index 2c2a7ca56..c136a7a6d 100644 --- a/server/core/users.c +++ b/server/core/users.c @@ -87,12 +87,12 @@ users_free(USERS *users) * @return The number of users added to the table */ int -users_add(USERS *users, char *user, char *auth) +users_add(USERS *users, const char *user, const char *auth) { int add; atomic_add(&users->stats.n_adds, 1); - add = hashtable_add(users->data, user, auth); + add = hashtable_add(users->data, (char*)user, (char*)auth); atomic_add(&users->stats.n_entries, add); return add; } @@ -105,12 +105,12 @@ users_add(USERS *users, char *user, char *auth) * @return The number of users deleted from the table */ int -users_delete(USERS *users, char *user) +users_delete(USERS *users, const char *user) { int del; atomic_add(&users->stats.n_deletes, 1); - del = hashtable_delete(users->data, user); + del = hashtable_delete(users->data, (char*)user); atomic_add(&users->stats.n_entries, -del); return del; } @@ -122,11 +122,12 @@ users_delete(USERS *users, char *user) * @param user The user name * @return The authentication data or NULL on error */ -char -*users_fetch(USERS *users, char *user) +const char +*users_fetch(USERS *users, const char *user) { atomic_add(&users->stats.n_fetches, 1); - return hashtable_fetch(users->data, user); + // TODO: Returning data from the hashtable is not threadsafe. + return hashtable_fetch(users->data, (char*)user); } /** @@ -139,13 +140,13 @@ char * @return Number of users updated */ int -users_update(USERS *users, char *user, char *auth) +users_update(USERS *users, const char *user, const char *auth) { - if (hashtable_delete(users->data, user) == 0) + if (hashtable_delete(users->data, (char*)user) == 0) { return 0; } - return hashtable_add(users->data, user, auth); + return hashtable_add(users->data, (char*)user, (char*)auth); } /** @@ -154,7 +155,7 @@ users_update(USERS *users, char *user, char *auth) * @param users The users table */ void -usersPrint(USERS *users) +usersPrint(const USERS *users) { printf("Users table data\n"); hashtable_stats(users->data); @@ -167,7 +168,7 @@ usersPrint(USERS *users) * @param users The users table */ void -dcb_usersPrint(DCB *dcb, USERS *users) +dcb_usersPrint(DCB *dcb, const USERS *users) { if (users == NULL || users->data == NULL) { diff --git a/server/modules/authenticator/cdc_plain_auth.c b/server/modules/authenticator/cdc_plain_auth.c index 1b19d9b20..19c60a29f 100644 --- a/server/modules/authenticator/cdc_plain_auth.c +++ b/server/modules/authenticator/cdc_plain_auth.c @@ -139,7 +139,7 @@ static int cdc_auth_check(DCB *dcb, CDC_protocol *protocol, char *username, uint { if (dcb->listener->users) { - char *user_password = users_fetch(dcb->listener->users, username); + const char *user_password = users_fetch(dcb->listener->users, username); if (user_password) { From f38b510d2b0c72f470882add0e73364290983795 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 1 Nov 2016 23:16:25 +0200 Subject: [PATCH 7/8] MXS-962: Add nonpositive priority for Galera nodes If a Galera node has a nonpositive priority, the node will never be chosen as the master. This gives the user more control over how the master is chosen. --- Documentation/Monitors/Galera-Monitor.md | 28 +++++++++++++++++--- server/modules/monitor/galeramon/galeramon.c | 12 ++++++--- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/Documentation/Monitors/Galera-Monitor.md b/Documentation/Monitors/Galera-Monitor.md index a74545ad7..d4d65488e 100644 --- a/Documentation/Monitors/Galera-Monitor.md +++ b/Documentation/Monitors/Galera-Monitor.md @@ -64,7 +64,16 @@ use_priority=true ## Interaction with Server Priorities -If the `use_priority` option is set and a server is configured with the `priority=` parameter, galeramon will use that as the basis on which the master node is chosen. This requires the `disable_master_role_setting` to be undefined or disabled. The server with the lowest value in `priority` will be chosen as the master node when a replacement Galera node is promoted to a master server inside MaxScale. +If the `use_priority` option is set and a server is configured with the +`priority=` parameter, galeramon will use that as the basis on which the +master node is chosen. This requires the `disable_master_role_setting` to be +undefined or disabled. The server with the lowest positive value in _priority_ +will be chosen as the master node when a replacement Galera node is promoted to +a master server inside MaxScale. + +Nodes with a non-positive value (_priority_ <= 0) will never be chosen as the master. This allows +you to mark some servers as permanent slaves by assigning a non-positive value +into _priority_. Here is an example with two servers. @@ -86,8 +95,21 @@ type=server address=192.168.122.103 port=3306 priority=2 + +[node-4] +type=server +address=192.168.122.104 +port=3306 +priority=0 ``` -In this example `node-1` is always used as the master if available. If `node-1` is not available, then the next node with the highest priority rank is used. In this case it would be `node-3`. If both `node-1` and `node-3` were down, then `node-2` would be used. Nodes without priority are considered as having the lowest priority rank and will be used only if all nodes with priority ranks are not available. +In this example `node-1` is always used as the master if available. If `node-1` +is not available, then the next node with the highest priority rank is used. In +this case it would be `node-3`. If both `node-1` and `node-3` were down, then +`node-2` would be used. Because `node-4` has a value of 0 in _priority_, it will +never be the master. Nodes without priority are considered as having the lowest +priority rank and will be used only if all nodes with priority ranks are not +available. -With priority ranks you can control the order in which MaxScale chooses the master node. This will allow for a controlled failure and replacement of nodes. +With priority ranks you can control the order in which MaxScale chooses the +master node. This will allow for a controlled failure and replacement of nodes. diff --git a/server/modules/monitor/galeramon/galeramon.c b/server/modules/monitor/galeramon/galeramon.c index c3b96a68e..db28a6123 100644 --- a/server/modules/monitor/galeramon/galeramon.c +++ b/server/modules/monitor/galeramon/galeramon.c @@ -681,11 +681,15 @@ static MONITOR_SERVERS *get_candidate_master(MONITOR* mon) if (handle->use_priority && (value = serverGetParameter(moitor_servers->server, "priority")) != NULL) { - currval = atoi(value); - if (currval < minval && currval > 0) + /** The server has a priority */ + if ((currval = atoi(value)) > 0) { - minval = currval; - candidate_master = moitor_servers; + /** The priority is valid */ + if (currval < minval && currval > 0) + { + minval = currval; + candidate_master = moitor_servers; + } } } else if (moitor_servers->server->node_id >= 0 && From 96547a1c0db15423da4dee47c1ef003b9c62f2f2 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 4 Nov 2016 10:48:02 +0200 Subject: [PATCH 8/8] Fix failure to exit on configuration error The check for the success of the configuration file always resulted in a successful return value even if the loading failed. In addition to this, a log message referred to the active configuration when the active configuration was set only after the processing was complete. Since configuration failures are always fatal, there's no harm in preemptively setting the active configuration to the one currently being processed. --- server/core/config.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/server/core/config.c b/server/core/config.c index f85d685d6..7958cb175 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -601,9 +601,9 @@ config_load_and_process(const char* filename, bool (*process_config)(CONFIG_CONT if (rval) { - if (check_config_objects(ccontext.next) && process_config(ccontext.next)) + if (!check_config_objects(ccontext.next) || !process_config(ccontext.next)) { - rval = true; + rval = false; } } } @@ -633,13 +633,9 @@ config_load(const char *filename) global_defaults(); feedback_defaults(); + config_file = filename; bool rval = config_load_and_process(filename, process_config_context); - if (rval) - { - config_file = filename; - } - return rval; }