From 8067f312a131668ab710709455b56e081ca5d5b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 17 May 2018 19:24:42 +0300 Subject: [PATCH 1/6] MXS-1873: Collect results for all session commands To work around the limitation in the session command handling and multi-part results, all session commands are now treated as gathered results. This allows session commands which return result sets to be used with MaxScale. This change should not cause problems with practical workloads as they usually do not return massive resultsets for session commands. The optimal way to handle the multi-part responses would be to integrate it into the result completion tracking process. This would allow the prepared statement IDs to be extracted while the command is being processed. --- server/modules/routing/readwritesplit/rwsplit_route_stmt.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc index 3db242179..b0ba764af 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc @@ -337,11 +337,11 @@ bool route_session_write(RWSplitSession *rses, GWBUF *querybuf, bool expecting_response = mxs_mysql_command_will_respond(command); int nsucc = 0; uint64_t lowest_pos = id; + gwbuf_set_type(querybuf, GWBUF_TYPE_COLLECT_RESULT); if (qc_query_is_type(type, QUERY_TYPE_PREPARE_NAMED_STMT) || qc_query_is_type(type, QUERY_TYPE_PREPARE_STMT)) { - gwbuf_set_type(querybuf, GWBUF_TYPE_COLLECT_RESULT); rses->ps_manager.store(querybuf, id); } From 4b8206ee1d88d716e0a3c2a9c9a0497a9b448438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 17 May 2018 20:42:57 +0300 Subject: [PATCH 2/6] MXS-1873: Test large session commands The test generates a large session command and makes sure queries after it work correctly. --- maxscale-system-test/CMakeLists.txt | 4 ++++ maxscale-system-test/mxs1873_large_sescmd.cpp | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 maxscale-system-test/mxs1873_large_sescmd.cpp diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 5d97ce698..02d9bf536 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -654,6 +654,10 @@ add_test_executable(mxs1824_double_cursor.cpp mxs1824_double_cursor replication # https://jira.mariadb.org/browse/MXS-1831 add_test_executable(mxs1831_unknown_param.cpp mxs1831_unknown_param replication LABELS REPL_BACKEND) +# MXS-1873: Large session commands cause errors +# https://jira.mariadb.org/browse/MXS-1873 +add_test_executable(mxs1873_large_sescmd.cpp mxs1873_large_sescmd replication LABELS readwritesplit REPL_BACKEND) + # 'namedserverfilter' test add_test_executable(namedserverfilter.cpp namedserverfilter namedserverfilter LABELS namedserverfilter LIGHT REPL_BACKEND) diff --git a/maxscale-system-test/mxs1873_large_sescmd.cpp b/maxscale-system-test/mxs1873_large_sescmd.cpp new file mode 100644 index 000000000..bd6285919 --- /dev/null +++ b/maxscale-system-test/mxs1873_large_sescmd.cpp @@ -0,0 +1,20 @@ +/** + * MXS-1873: Large session commands cause errors + * + * https://jira.mariadb.org/browse/MXS-1873 + */ + +#include "testconnections.h" + +int main(int argc, char** argv) +{ + TestConnections test(argc, argv); + + test.maxscales->connect(); + test.try_query(test.maxscales->conn_rwsplit[0], + "SET STATEMENT max_statement_time=30 FOR SELECT seq FROM seq_0_to_100000"); + test.try_query(test.maxscales->conn_rwsplit[0], "SELECT 1"); + test.maxscales->disconnect(); + + return test.global_result; +} From 3cb389a3b572ce2f46dd3121334265667337b479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 17 May 2018 11:17:52 +0300 Subject: [PATCH 3/6] MXS-1866: Add support for COM_STMT_BULK_EXECUTE Readwritesplit now detects the COM_STMT_BULK_EXECUTE command and handles it correctly. --- include/maxscale/protocol/mysql.h | 1 + server/modules/routing/readwritesplit/rwsplit_internal.hh | 1 + 2 files changed, 2 insertions(+) diff --git a/include/maxscale/protocol/mysql.h b/include/maxscale/protocol/mysql.h index 9f8421001..88f0b0bc7 100644 --- a/include/maxscale/protocol/mysql.h +++ b/include/maxscale/protocol/mysql.h @@ -293,6 +293,7 @@ typedef enum MXS_COM_STMT_RESET = 26, MXS_COM_SET_OPTION = 27, MXS_COM_STMT_FETCH = 28, + MXS_COM_STMT_BULK_EXECUTE = 0xfa, MXS_COM_DAEMON, MXS_COM_END } mxs_mysql_cmd_t; diff --git a/server/modules/routing/readwritesplit/rwsplit_internal.hh b/server/modules/routing/readwritesplit/rwsplit_internal.hh index 6db0c17c3..fd4745a9d 100644 --- a/server/modules/routing/readwritesplit/rwsplit_internal.hh +++ b/server/modules/routing/readwritesplit/rwsplit_internal.hh @@ -34,6 +34,7 @@ do{ \ static inline bool is_ps_command(uint8_t cmd) { return cmd == MXS_COM_STMT_EXECUTE || + cmd == MXS_COM_STMT_BULK_EXECUTE || cmd == MXS_COM_STMT_SEND_LONG_DATA || cmd == MXS_COM_STMT_CLOSE || cmd == MXS_COM_STMT_FETCH || From f665125f1cfa679bacfce0a2bca960ca815e126b Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 18 May 2018 11:00:52 +0300 Subject: [PATCH 4/6] MXS-1874 Add test that reveals problem --- query_classifier/test/maxscale.test | 3 +++ 1 file changed, 3 insertions(+) diff --git a/query_classifier/test/maxscale.test b/query_classifier/test/maxscale.test index 1bfc4a1db..8c34ceb29 100644 --- a/query_classifier/test/maxscale.test +++ b/query_classifier/test/maxscale.test @@ -106,3 +106,6 @@ CALL p1((SELECT f1()), ?); # MXS-1829 SELECT PREVIOUS VALUE FOR SEQ; + +# MXS-1874 +SET STATEMENT max_statement_time=30 FOR SELECT seq FROM seq_0_to_100000; \ No newline at end of file From b12f037b241096a260b27c30fc96bccff3e53297 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 18 May 2018 11:32:39 +0300 Subject: [PATCH 5/6] MXS-1874 Fix leak in qc_mysqlembedded --- query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index 50679dc50..8ea9ce177 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -1793,6 +1793,7 @@ static void parsing_info_done(void* ptr) free(field.table); free(field.column); } + free(fi.fields); } free(pi->function_infos); From 001ae8e29a744f0ffe72308feb152eabc35749cf Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 18 May 2018 12:18:03 +0300 Subject: [PATCH 6/6] MXS-1874 Handle SET STATEMENT ... FOR ... The SET STATEMENT ... FOR part can be ignored and the type of the statement be whatever the type of the part following FOR is. --- .../qc_sqlite/sqlite-src-3110100/src/parse.y | 10 +++++++++- .../qc_sqlite/sqlite-src-3110100/tool/mkkeywordhash.c | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y index d4498cf37..21707bee2 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y @@ -629,7 +629,7 @@ columnid(A) ::= nm(X). { PREVIOUS QUICK RAISE RECURSIVE /*REINDEX*/ RELEASE /*RENAME*/ /*REPLACE*/ RESTRICT ROLLBACK ROLLUP ROW - SAVEPOINT SELECT_OPTIONS_KW /*SEQUENCE*/ SLAVE /*START*/ STATUS + SAVEPOINT SELECT_OPTIONS_KW /*SEQUENCE*/ SLAVE /*START*/ STATEMENT STATUS TABLES TEMP TEMPTABLE /*TRIGGER*/ /*TRUNCATE*/ // TODO: UNSIGNED is a reserved word and should not automatically convert into an identifer. @@ -3157,6 +3157,14 @@ cmd ::= SET set_scope(X) TRANSACTION transaction_characteristics. { maxscaleSet(pParse, X, MXS_SET_TRANSACTION, 0); } +cmd ::= SET STATEMENT variable_assignments(X) FOR cmd. { + // The parsing of cmd will cause the relevant maxscale-callback to + // be called, so we neither need to call it here, nor free cmd (as + // it will be freed by that callback). The variable definitions we + // just throw away, as they are of no interest. + sqlite3ExprListDelete(pParse->db, X); +} + //////////////////////// The USE statement //////////////////////////////////// // cmd ::= use(X). { diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/tool/mkkeywordhash.c b/query_classifier/qc_sqlite/sqlite-src-3110100/tool/mkkeywordhash.c index 5d51ea86a..18017915c 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/tool/mkkeywordhash.c +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/tool/mkkeywordhash.c @@ -442,6 +442,7 @@ static Keyword aKeywordTable[] = { { "SET", "TK_SET", ALWAYS }, #ifdef MAXSCALE { "START", "TK_START", ALWAYS }, + { "STATEMENT", "TK_STATEMENT", ALWAYS }, { "STATUS", "TK_STATUS", ALWAYS }, { "STRAIGHT_JOIN", "TK_STRAIGHT_JOIN",ALWAYS }, #endif