From cd136b11a70c3398fc9c60f4fe3d7bc98214f7bc Mon Sep 17 00:00:00 2001 From: dapeng huang <7xerocha@gmail.com> Date: Tue, 12 Jun 2018 15:45:11 +0800 Subject: [PATCH 1/7] MXS-1891: dealloc named prepare should route to all (#174) * dealloc named prepare route to all * add newline * erase from ps manager too * little refactor --- include/maxscale/query_classifier.h | 3 ++- query_classifier/qc_sqlite/qc_sqlite.cc | 2 +- query_classifier/test/expected.sql | 1 + query_classifier/test/input.sql | 1 + server/modules/routing/readwritesplit/rwsplit_route_stmt.cc | 5 +++++ 5 files changed, 10 insertions(+), 2 deletions(-) diff --git a/include/maxscale/query_classifier.h b/include/maxscale/query_classifier.h index 116e59a1b..5ae2b5731 100644 --- a/include/maxscale/query_classifier.h +++ b/include/maxscale/query_classifier.h @@ -84,7 +84,8 @@ typedef enum qc_query_type QUERY_TYPE_CREATE_TMP_TABLE = 0x080000, /*< Create temporary table:master (could be all) */ QUERY_TYPE_READ_TMP_TABLE = 0x100000, /*< Read temporary table:master (could be any) */ QUERY_TYPE_SHOW_DATABASES = 0x200000, /*< Show list of databases */ - QUERY_TYPE_SHOW_TABLES = 0x400000 /*< Show list of tables */ + QUERY_TYPE_SHOW_TABLES = 0x400000, /*< Show list of tables */ + QUERY_TYPE_DEALLOC_PREPARE = 0x1000000 /*< Dealloc named prepare stmt:all */ } qc_query_type_t; /** diff --git a/query_classifier/qc_sqlite/qc_sqlite.cc b/query_classifier/qc_sqlite/qc_sqlite.cc index a71e501e2..30b6c0195 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.cc +++ b/query_classifier/qc_sqlite/qc_sqlite.cc @@ -2136,7 +2136,7 @@ public: ss_dassert(this_thread.initialized); m_status = QC_QUERY_PARSED; - m_type_mask = QUERY_TYPE_WRITE; + m_type_mask = QUERY_TYPE_DEALLOC_PREPARE; // If information is collected in several passes, then we may // this information already. diff --git a/query_classifier/test/expected.sql b/query_classifier/test/expected.sql index f57402db2..b3ef5061f 100644 --- a/query_classifier/test/expected.sql +++ b/query_classifier/test/expected.sql @@ -23,3 +23,4 @@ QUERY_TYPE_READ|QUERY_TYPE_WRITE QUERY_TYPE_READ|QUERY_TYPE_WRITE QUERY_TYPE_READ|QUERY_TYPE_WRITE QUERY_TYPE_READ|QUERY_TYPE_WRITE +QUERY_TYPE_DEALLOC_PREPARE diff --git a/query_classifier/test/input.sql b/query_classifier/test/input.sql index 643e2c2d0..1ddd21a60 100644 --- a/query_classifier/test/input.sql +++ b/query_classifier/test/input.sql @@ -23,3 +23,4 @@ SELECT GET_LOCK('lock1',10); SELECT IS_FREE_LOCK('lock1'); SELECT IS_USED_LOCK('lock1'); SELECT RELEASE_LOCK('lock1'); +deallocate prepare select_stmt; diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc index 3b2059e8e..75e2cdc81 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc @@ -344,6 +344,10 @@ bool route_session_write(RWSplitSession *rses, GWBUF *querybuf, { rses->ps_manager.store(querybuf, id); } + else if (qc_query_is_type(type, QUERY_TYPE_DEALLOC_PREPARE)) + { + rses->ps_manager.erase(get_text_ps_id(querybuf)); + } MXS_INFO("Session write, routing to all servers."); @@ -624,6 +628,7 @@ route_target_t get_route_target(RWSplitSession *rses, uint8_t command, */ if (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_DEALLOC_PREPARE) || command == MXS_COM_STMT_CLOSE || command == MXS_COM_STMT_RESET) { From f87fc45511b2b9a07b2b21e203986cf2d896b80b Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 12 Jun 2018 12:44:44 +0300 Subject: [PATCH 2/7] MXS-1884 Wrap lines of readconnroute documentation No other changes. --- Documentation/Routers/ReadConnRoute.md | 46 ++++++++++++++++++-------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/Documentation/Routers/ReadConnRoute.md b/Documentation/Routers/ReadConnRoute.md index 7ac74c3b3..1e44b9f20 100644 --- a/Documentation/Routers/ReadConnRoute.md +++ b/Documentation/Routers/ReadConnRoute.md @@ -1,18 +1,25 @@ # Readconnroute -This document provides an overview of the **readconnroute** router module and its intended use case scenarios. It also displays all router configuration parameters with their descriptions. +This document provides an overview of the **readconnroute** router module +and its intended use case scenarios. It also displays all router +configuration parameters with their descriptions. ## Overview -The readconnroute router provides simple and lightweight load balancing across a set of servers. The router can also be configured to balance connections based on a weighting parameter defined in the server's section. +The readconnroute router provides simple and lightweight load balancing +across a set of servers. The router can also be configured to balance +connections based on a weighting parameter defined in the server's section. ## Configuration -For more details about the standard service parameters, refer to the [Configuration Guide](../Getting-Started/Configuration-Guide.md). +For more details about the standard service parameters, refer to the +[Configuration Guide](../Getting-Started/Configuration-Guide.md). ### Router Options -**`router_options`** can contain a list of valid server roles. These roles are used as the valid types of servers the router will form connections to when new sessions are created. +**`router_options`** can contain a list of valid server roles. These roles +are used as the valid types of servers the router will form connections to +when new sessions are created. ``` router_options=slave ``` @@ -26,22 +33,30 @@ synced| A Galera cluster node which is in a synced state with the cluster. ndb|A MySQL Replication Cluster node running|A server that is up and running. All servers that MariaDB MaxScale can connect to are labeled as running. -If no `router_options` parameter is configured in the service definition, the router will use the default value of `running`. This means that it will load balance connections across all running servers defined in the `servers` parameter of the service. +If no `router_options` parameter is configured in the service definition, +the router will use the default value of `running`. This means that it will +load balance connections across all running servers defined in the `servers` +parameter of the service. -When a connection is being created and the candidate server is being chosen, the -list of servers is processed in from first entry to last. This means that if two -servers with equal weight and status are found, the one that's listed first in -the _servers_ parameter for the service is chosen. +When a connection is being created and the candidate server is being chosen, +the list of servers is processed in from first entry to last. This means +that if two servers with equal weight and status are found, the one that's +listed first in the _servers_ parameter for the service is chosen. ## Limitations -For a list of readconnroute limitations, please read the [Limitations](../About/Limitations.md) document. +For a list of readconnroute limitations, please read the +[Limitations](../About/Limitations.md) document. ## Examples -The most common use for the readconnroute is to provide either a read or write port for an application. This provides a more lightweight routing solution than the more complex readwritesplit router but requires the application to be able to use distinct write and read ports. +The most common use for the readconnroute is to provide either a read or +write port for an application. This provides a more lightweight routing +solution than the more complex readwritesplit router but requires the +application to be able to use distinct write and read ports. -To configure a read-only service that tolerates master failures, we first need to add a new section in to the configuration file. +To configure a read-only service that tolerates master failures, we first +need to add a new section in to the configuration file. ``` [Read Service] @@ -51,6 +66,9 @@ servers=slave1,slave2,slave3 router_options=slave ``` -Here the `router_options` designates slaves as the only valid server type. With this configuration, the queries are load balanced across the slave servers. +Here the `router_options` designates slaves as the only valid server +type. With this configuration, the queries are load balanced across the +slave servers. -For more complex examples of the readconnroute router, take a look at the examples in the [Tutorials](../Tutorials) folder. +For more complex examples of the readconnroute router, take a look at the +examples in the [Tutorials](../Tutorials) folder. From f5f454a29b1d3d676b464a43952d25f574181a99 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 12 Jun 2018 12:46:50 +0300 Subject: [PATCH 3/7] MXS-1884 Clarify what a list means --- Documentation/Routers/ReadConnRoute.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Documentation/Routers/ReadConnRoute.md b/Documentation/Routers/ReadConnRoute.md index 1e44b9f20..c726d7469 100644 --- a/Documentation/Routers/ReadConnRoute.md +++ b/Documentation/Routers/ReadConnRoute.md @@ -17,11 +17,14 @@ For more details about the standard service parameters, refer to the ### Router Options -**`router_options`** can contain a list of valid server roles. These roles -are used as the valid types of servers the router will form connections to -when new sessions are created. +**`router_options`** can contain a comma separated list of valid server +roles. These roles are used as the valid types of servers the router will +form connections to when new sessions are created. + +Examples: ``` - router_options=slave +router_options=slave +router_options=master,slave ``` Here is a list of all possible values for the `router_options`. From 776f199d016f5282373cdb416c95bb1c5a55922f Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 8 Jun 2018 11:44:40 +0300 Subject: [PATCH 4/7] MXS-1719 Treat # comments the same way as -- comments sqlite does not treat # as the start of a to-end-of-line comment. It cannot trivially be treated as such because at startup sqlite parses statements containing the #-character. Thus, only after sqlite has been initialized can it be treated the same way as --. --- query_classifier/qc_sqlite/qc_sqlite.cc | 20 ++++++++++++++----- .../sqlite-src-3110100/src/tokenize.c | 11 ++++++++-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/query_classifier/qc_sqlite/qc_sqlite.cc b/query_classifier/qc_sqlite/qc_sqlite.cc index 30b6c0195..4ec882119 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.cc +++ b/query_classifier/qc_sqlite/qc_sqlite.cc @@ -2110,15 +2110,25 @@ public: update_names(zDatabase, table, NULL, NULL); } - void maxscaleComment() + int maxscaleComment() { - ss_dassert(this_thread.initialized); + // We are regularily parsing if the thread has been initialized. + // In that case # should be interpreted as the start of a comment, + // otherwise it should not. + int regular_parsing = false; - if (m_status == QC_QUERY_INVALID) + if (this_thread.initialized) { - m_status = QC_QUERY_PARSED; - m_type_mask = QUERY_TYPE_READ; + regular_parsing = true; + + if (m_status == QC_QUERY_INVALID) + { + m_status = QC_QUERY_PARSED; + m_type_mask = QUERY_TYPE_READ; + } } + + return regular_parsing; } void maxscaleDeclare(Parse* pParse) diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/src/tokenize.c b/query_classifier/qc_sqlite/sqlite-src-3110100/src/tokenize.c index fd875b64e..3284a2e2e 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/src/tokenize.c +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/src/tokenize.c @@ -198,6 +198,7 @@ int sqlite3IsIdChar(u8 c){ return IdChar(c); } ** Store the token type in *tokenType before returning. */ #ifdef MAXSCALE +extern int maxscaleComment(); int sqlite3GetToken(Parse* pParse, const unsigned char *z, int *tokenType){ #else int sqlite3GetToken(const unsigned char *z, int *tokenType){ @@ -219,8 +220,7 @@ int sqlite3GetToken(const unsigned char *z, int *tokenType){ case CC_MINUS: { if( z[1]=='-' ){ #ifdef MAXSCALE - extern void maxscaleComment(); - maxscaleComment(); + maxscaleComment(); #endif for(i=2; (c=z[i])!=0 && c!='\n'; i++){} *tokenType = TK_SPACE; /* IMP: R-22934-25134 */ @@ -466,6 +466,13 @@ int sqlite3GetToken(const unsigned char *z, int *tokenType){ testcase( z[0]=='$' ); testcase( z[0]=='@' ); testcase( z[0]==':' ); testcase( z[0]=='#' ); #ifdef MAXSCALE + if (z[0]=='#') { + if (maxscaleComment()) { + for(i=1; (c=z[i])!=0 && c!='\n'; i++){} + *tokenType = TK_SPACE; + return i; + } + } if (z[0]==':' && z[1]=='=') { *tokenType = TK_EQ; return 2; From d9100278b040dc29a1457304f5b923c421ac5e76 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 11 Jun 2018 09:34:46 +0300 Subject: [PATCH 5/7] MXS-1719 Update qc test parser The recent change to the qc_sqlite parser also requires some modifications to the parser used for reading .test-files. --- query_classifier/test/testreader.cc | 46 ++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/query_classifier/test/testreader.cc b/query_classifier/test/testreader.cc index 7beffb30c..e53561f55 100644 --- a/query_classifier/test/testreader.cc +++ b/query_classifier/test/testreader.cc @@ -37,6 +37,7 @@ enum skip_action_t typedef std::map KeywordActionMapping; static KeywordActionMapping mtl_keywords; +static KeywordActionMapping plsql_keywords; void init_keywords() { @@ -46,7 +47,7 @@ void init_keywords() skip_action_t action; }; - static const Keyword KEYWORDS[] = + static const Keyword MTL_KEYWORDS[] = { { "append_file", SKIP_LINE }, { "cat_file", SKIP_LINE }, @@ -91,10 +92,8 @@ void init_keywords() { "error", SKIP_NEXT_STATEMENT }, { "eval", SKIP_STATEMENT }, { "exec", SKIP_LINE }, - { "exit", SKIP_LINE }, { "file_exists", SKIP_LINE }, { "horizontal_results", SKIP_LINE }, - { "if", SKIP_BLOCK }, { "inc", SKIP_LINE }, { "let", SKIP_LINE }, { "let", SKIP_LINE }, @@ -138,15 +137,28 @@ void init_keywords() { "sync_with_master", SKIP_LINE }, { "system", SKIP_LINE }, { "vertical_results", SKIP_LINE }, - { "while", SKIP_BLOCK }, { "write_file", SKIP_LINE }, }; - const size_t N_KEYWORDS = sizeof(KEYWORDS)/sizeof(KEYWORDS[0]); + const size_t N_MTL_KEYWORDS = sizeof(MTL_KEYWORDS)/sizeof(MTL_KEYWORDS[0]); - for (size_t i = 0; i < N_KEYWORDS; ++i) + for (size_t i = 0; i < N_MTL_KEYWORDS; ++i) { - mtl_keywords[KEYWORDS[i].z_keyword] = KEYWORDS[i].action; + mtl_keywords[MTL_KEYWORDS[i].z_keyword] = MTL_KEYWORDS[i].action; + } + + static const Keyword PLSQL_KEYWORDS[] = + { + { "exit", SKIP_LINE }, + { "if", SKIP_BLOCK }, + { "while", SKIP_BLOCK }, + }; + + const size_t N_PLSQL_KEYWORDS = sizeof(PLSQL_KEYWORDS)/sizeof(PLSQL_KEYWORDS[0]); + + for (size_t i = 0; i < N_PLSQL_KEYWORDS; ++i) + { + plsql_keywords[PLSQL_KEYWORDS[i].z_keyword] = PLSQL_KEYWORDS[i].action; } } @@ -163,18 +175,24 @@ skip_action_t get_action(const string& keyword, const string& delimiter) // be handled explicitly. action = SKIP_DELIMITER; } - else if (delimiter == ";") + else + { + KeywordActionMapping::iterator i = mtl_keywords.find(key); + + if (i != mtl_keywords.end()) + { + action = i->second; + } + } + + if ((action == SKIP_NOTHING) && (delimiter == ";")) { // Some mysqltest keywords, such as "while", "exit" and "if" are also // PL/SQL keywords. We assume they can only be used in the former role, // if the delimiter is ";". - string key(keyword); + KeywordActionMapping::iterator i = plsql_keywords.find(key); - std::transform(key.begin(), key.end(), key.begin(), ::tolower); - - KeywordActionMapping::iterator i = mtl_keywords.find(key); - - if (i != mtl_keywords.end()) + if (i != plsql_keywords.end()) { action = i->second; } From 24870e278c2008ffa41bf8216162a02af8d6afd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 12 Jun 2018 09:31:24 +0300 Subject: [PATCH 6/7] MXS-1913: Check session before invoking dcb_foreach callback If the session is the dummy session, the callback should not be called. --- server/core/dcb.cc | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/server/core/dcb.cc b/server/core/dcb.cc index f4b9e647a..dff407632 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -2909,10 +2909,15 @@ public: dcb && atomic_load_int32(&m_more); dcb = dcb->thread.next) { - if (!m_func(dcb, m_data)) + ss_dassert(dcb->session); + + if (dcb->session->state != SESSION_STATE_DUMMY) { - atomic_store_int32(&m_more, 0); - break; + if (!m_func(dcb, m_data)) + { + atomic_store_int32(&m_more, 0); + break; + } } } } From e99d9826ad1cb7cb293de1c3929a0dd1db28d779 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 12 Jun 2018 11:46:31 +0300 Subject: [PATCH 7/7] Fix route_by_statement return value The return value of route_by_statement was not initialized and not set if a COM_CHANGE_USER was processed. --- server/modules/protocol/MySQL/mariadbclient/mysql_client.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index 577bf867b..428654b7e 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -1575,7 +1575,7 @@ static bool reauthenticate_client(MXS_SESSION* session, GWBUF* packetbuf) */ static int route_by_statement(MXS_SESSION* session, uint64_t capabilities, GWBUF** p_readbuf) { - int rc; + int rc = 1; GWBUF* packetbuf; do { @@ -1682,6 +1682,7 @@ static int route_by_statement(MXS_SESSION* session, uint64_t capabilities, GWBUF // Store the original COM_CHANGE_USER for later proto->stored_query = packetbuf; packetbuf = NULL; + rc = 1; } else if (proto->changing_user) {