diff --git a/Documentation/About/Limitations.md b/Documentation/About/Limitations.md index e7ee13c94..fff8ceaf5 100644 --- a/Documentation/About/Limitations.md +++ b/Documentation/About/Limitations.md @@ -106,10 +106,6 @@ usernames in MariaDB MaxScale. ## Filter limitations -Filters are not guaranteed to receive complete MySQL packets if they are used -with the readconnroute router. This can be fixed by using the readwritesplit -router. - ### Database Firewall limitations (dbfwfilter) The Database Firewall filter does not support multi-statements. Using them will @@ -119,13 +115,18 @@ result in an error being sent to the client. The Tee filter does not support binary protocol prepared statements. The execution of a prepared statements through a service that uses the tee filter is -not guaranteed to produce the same result on the service where the filter -branches to as it does on the original service. +not guaranteed to succeed on the service where the filter branches to as it does +on the original service. + +This possibility exists due to the fact that the binary protocol prepared +statements are identified by a server-generated ID. The ID sent to the client +from the main service is not guaranteed to be the same that is sent by the +branch service. ## Monitor limitations -A server can only be monitored by one monitor. If multiple monitors monitor the -same server, the state of the server is non-deterministic. +A server can only be monitored by one monitor. Two or more monitors monitoring +the same server is considered an error. ### Limitations with Galera Cluster Monitoring (galeramon) @@ -156,9 +157,6 @@ on this issue. ### Limitations in the connection router (readconnroute) -If Master changes (ie. new Master promotion) during current connection, the -router cannot check the change. - Sending of binary data with `LOAD DATA LOCAL INFILE` is not supported. ### Limitations in the Read/Write Splitter (readwritesplit) @@ -172,14 +170,9 @@ LAST_INSERT_ID();` #### JDBC Batched Statements -Readwritesplit does not support execution of JDBC batched statements with -non-INSERT statements mixed in it. This is caused by the fact that -readwritesplit expects that the protocol is idle before another command is sent. - -Most clients conform to this expectation but some JDBC drivers send multiple -requests without waiting for the protocol to be idle. If you are using the -MariaDB Connector/J, add `useBatchMultiSend=false` to the JDBC connection string -to disable batched statement execution. +Readwritesplit does not support pipelining of JDBC batched statements. This is +caused by the fact that readwritesplit executes the statements one at a time to +track the state of the response. #### Prepared Statement Limitations @@ -193,18 +186,9 @@ statements to stall. #### Limitations in multi-statement handling When a multi-statement query is executed through the readwritesplit router, it -will always be routed to the master. With the default configuration, all queries -after a multi-statement query will be routed to the master to prevent possible -reads of false data. - -You can override this behavior with the `strict_multi_stmt=false` router option. -In this mode, the multi-statement queries will still be routed to the master but -individual statements are routed normally. If you use multi-statements and you -know they don't modify the session state in any relevant way, you can disable -this option for better performance. - -For more information, read the -[ReadWriteSplit](../Routers/ReadWriteSplit.md) router documentation. +will always be routed to the master. See +[`strict_multi_stmt`](../Routers/ReadWriteSplit.md#strict_multi_stmt) for more +details. #### Limitations in client session handling @@ -234,22 +218,21 @@ SET autocommit=1|0 ``` There is a possibility for misbehavior. If `USE mytable` is executed in one of -the slaves and fails, it may be due to replication lag rather than the -database not existing. Thus, the same command may produce different result in -different backend servers. The slaves which fail to execute a session command -will be dropped from the active list of slaves for this session to guarantee a -consistent session state across all the servers used by the session. +the slaves and fails, it may be due to replication lag rather than the database +not existing. Thus, the same command may produce different result in different +backend servers. The slaves which fail to execute a session command will be +dropped from the active list of slaves for this session to guarantee a +consistent session state across all the servers used by the session. In +addition, the server will not used again for routing for the duration of the +session. + +The above-mentioned behavior for user variables can be partially controlled with +the configuration parameter `use_sql_variables_in`: -The above-mentioned behavior can be partially controlled with the configuration -parameter `use_sql_variables_in`: ``` use_sql_variables_in=[master|all] (default: all) ``` -Server-side session variables are handled similar to SQL variables. If "master" -is set, SQL variables are read and written in master only. Autocommit values and -prepared statements are routed to all nodes always. - **WARNING** If a SELECT query modifies a user variable when the `use_sql_variables_in` @@ -266,28 +249,10 @@ MySQL [(none)]> SELECT @id := @id + 1 FROM test.t1; ERROR 1064 (42000): Routing query to backend failed. See the error log for further details. ``` -Allow user variable modification in SELECT queries by setting the value of -`use_sql_variables_in` to `master`. This will route all queries that use user +Allow user variable modification in SELECT queries by setting +`use_sql_variables_in=master`. This will route all queries that use user variables to the master. -#### Examples of session command limitations - -In a situation where a new database `db` is created, immediately after which a client executes `USE db`, it is possible that the command is routed -to a slave before the `CREATE DATABASE` clause is replicated to all slaves. In this case a query may be executed in the wrong database. Similarly, if any response -that ReadWriteSplit sends back to the client differ from that of the master, -there is a risk for misbehavior. To prevent this, any failures in session -command execution are treated as fatal errors and all connections by the session -to that particular slave server will be closed. In addition, the server will not -used again for routing for the duration of the session. - -The most likely reasons are related to replication lag but it could be possible -that a slave fails to execute something because of some non-fatal, temporary -failure, while the execution of the same command succeeds in other backends. - -The preparation of a prepared statement is routed to all servers. The execution -of a prepared statement is routed to the first available server or to the server -pointed by a routing hint attached to the query. - ### Schemarouter limitations (schemarouter) The schemarouter currently has some limitations due to the nature of the diff --git a/include/maxscale/modutil.h b/include/maxscale/modutil.h index 99c01f59e..ba5cd8425 100644 --- a/include/maxscale/modutil.h +++ b/include/maxscale/modutil.h @@ -45,6 +45,7 @@ int modutil_MySQL_query_len(GWBUF* buf, int* nbytes_missing); void modutil_reply_parse_error(DCB* backend_dcb, char* errstr, uint32_t flags); void modutil_reply_auth_error(DCB* backend_dcb, char* errstr, uint32_t flags); int modutil_count_statements(GWBUF* buffer); +int modutil_count_packets(GWBUF* buffer); GWBUF* modutil_create_query(const char* query); GWBUF* modutil_create_mysql_err_msg(int packet_number, int affected_rows, diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 1a0f9c6f3..f88588ec3 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -662,6 +662,22 @@ add_test_executable(mxs1743_rconn_bitmask.cpp mxs1743_rconn_bitmask mxs1743_rcon # https://jira.mariadb.org/browse/MXS-1751 add_test_executable(mxs1751_available_when_donor_crash.cpp mxs1751_available_when_donor_crash mxs1751_available_when_donor_crash LABELS galeramon GALERA_BACKEND) +# MXS-1773: Failing LOAD DATA LOCAL INFILE confuses readwritesplit +# https://jira.mariadb.org/browse/MXS-1773 +add_test_executable(mxs1773_failing_ldli.cpp mxs1773_failing_ldli replication LABELS readwritesplit REPL_BACKEND) + +# MXS-1776: recursive COM_STMT_EXECUTE execution +# https://jira.mariadb.org/browse/MXS-1776 +add_test_executable(mxs1776_ps_exec_hang.cpp mxs1776_ps_exec_hang replication LABELS readwritesplit REPL_BACKEND) + +# MXS-1786: Hang with COM_STATISTICS +# https://jira.mariadb.org/browse/MXS-1786 +add_test_executable(mxs1786_statistics.cpp mxs1786_statistics replication LABELS readwritesplit REPL_BACKEND) + +# MXS-1787: Crash with PS: CALL p1((SELECT f1()), ?) +# https://jira.mariadb.org/browse/MXS-1787 +add_test_executable(mxs1787_call_ps.cpp mxs1787_call_ps 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/mxs1071_maxrows.cpp b/maxscale-system-test/mxs1071_maxrows.cpp index deec523e0..7aaa84dc7 100644 --- a/maxscale-system-test/mxs1071_maxrows.cpp +++ b/maxscale-system-test/mxs1071_maxrows.cpp @@ -297,8 +297,7 @@ int main(int argc, char *argv[]) create_t1(Test->maxscales->conn_rwsplit[0]); insert_into_t1(Test->maxscales->conn_rwsplit[0], 1); Test->stop_timeout(); - sleep(5); - + Test->repl->sync_slaves(); Test->tprintf("**** Test 1 ****\n"); @@ -316,7 +315,8 @@ int main(int argc, char *argv[]) create_t1(Test->maxscales->conn_rwsplit[0]); insert_into_t1(Test->maxscales->conn_rwsplit[0], 3); Test->stop_timeout(); - sleep(5); + Test->repl->sync_slaves(); + Test->tprintf("**** Test 2 ****\n"); exp_rows[0] = 0; diff --git a/maxscale-system-test/mxs1773_failing_ldli.cpp b/maxscale-system-test/mxs1773_failing_ldli.cpp new file mode 100644 index 000000000..f8e113608 --- /dev/null +++ b/maxscale-system-test/mxs1773_failing_ldli.cpp @@ -0,0 +1,22 @@ +/** + * MXS-1773: Failing LOAD DATA LOCAL INFILE confuses readwritesplit + * + * https://jira.mariadb.org/browse/MXS-1773 + */ +#include "testconnections.h" +#include + +int main(int argc, char** argv) +{ + TestConnections test(argc, argv); + + test.maxscales->connect(); + auto q = std::bind(execute_query, test.maxscales->conn_rwsplit[0], std::placeholders::_1); + q("LOAD DATA LOCAL INFILE '/tmp/this-file-does-not-exist.txt' INTO TABLE this_table_does_not_exist"); + q("SELECT 1"); + q("SELECT 2"); + q("SELECT 3"); + test.maxscales->disconnect(); + + return test.global_result; +} diff --git a/maxscale-system-test/mxs1776_ps_exec_hang.cpp b/maxscale-system-test/mxs1776_ps_exec_hang.cpp new file mode 100644 index 000000000..b98c116ba --- /dev/null +++ b/maxscale-system-test/mxs1776_ps_exec_hang.cpp @@ -0,0 +1,193 @@ +#include "testconnections.h" +#include +#include +#include + +using namespace std; + +struct Bind +{ + Bind() + { + bind.buffer = buffer; + bind.buffer_type = MYSQL_TYPE_LONG; + bind.error = &err; + bind.is_null = &is_null; + bind.length = &length; + } + + MYSQL_BIND bind; + char err = 0; + char is_null = 0; + char is_unsigned = 0; + uint8_t buffer[1024]; + unsigned long length = 0; +}; + +struct TestCase +{ + std::string name; + std::function func; +}; + +bool run_test(TestConnections& test, TestCase test_case) +{ + test.maxscales->connect(); + + MYSQL_STMT* stmt = mysql_stmt_init(test.maxscales->conn_rwsplit[0]); + std::string query = "SELECT * FROM test.t1"; + unsigned long cursor_type = CURSOR_TYPE_READ_ONLY; + mysql_stmt_attr_set(stmt, STMT_ATTR_CURSOR_TYPE, &cursor_type); + + Bind bind; + + test.set_timeout(30); + + if (mysql_stmt_prepare(stmt, query.c_str(), query.size())) + { + test.assert(false, "Prepared statement failure: %s", mysql_stmt_error(stmt)); + } + + cout << test_case.name << endl; + test.assert(test_case.func(test.maxscales->conn_rwsplit[0], stmt, bind), "Test '%s' failed", + test_case.name.c_str()); + + mysql_stmt_close(stmt); + + test.assert(mysql_query(test.maxscales->conn_rwsplit[0], "SELECT 1") == 0, "Normal queries should work"); + + test.maxscales->disconnect(); +} + + +int main(int argc, char* argv[]) +{ + TestConnections test(argc, argv); + + test.maxscales->connect(); + + test.try_query(test.maxscales->conn_rwsplit[0], "CREATE OR REPLACE TABLE test.t1(id INT)"); + test.try_query(test.maxscales->conn_rwsplit[0], "BEGIN"); + + for (int i = 0; i < 100; i++) + { + execute_query(test.maxscales->conn_rwsplit[0], "INSERT INTO test.t1 VALUES (%d)", i); + } + + test.try_query(test.maxscales->conn_rwsplit[0], "COMMIT"); + test.maxscales->disconnect(); + + vector tests = + { + { + "Simple execute and fetch", + [](MYSQL * conn, MYSQL_STMT * stmt, Bind & bind) + { + bool rval = true; + + if (mysql_stmt_execute(stmt) || + mysql_stmt_bind_result(stmt, &bind.bind)) + { + rval = false; + } + + while (mysql_stmt_fetch(stmt) == 0) + { + ; + } + + return rval; + } + }, + { + "Multiple overlapping executions without fetch", + [](MYSQL * conn, MYSQL_STMT * stmt, Bind & bind) + { + bool rval = true; + + if (mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt)) + { + rval = false; + } + + return rval; + } + }, + { + "Multiple overlapping executions with fetch", + [](MYSQL * conn, MYSQL_STMT * stmt, Bind & bind) + { + bool rval = true; + + if (mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_bind_result(stmt, &bind.bind)) + { + rval = false; + } + + while (mysql_stmt_fetch(stmt) == 0) + { + ; + } + + return rval; + } + }, + { + "Execution of queries while fetching", + [](MYSQL * conn, MYSQL_STMT * stmt, Bind & bind) + { + bool rval = true; + + if (mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_bind_result(stmt, &bind.bind)) + { + rval = false; + } + + while (mysql_stmt_fetch(stmt) == 0) + { + mysql_query(conn, "SELECT 1"); + } + + return rval; + } + }, + { + "Multiple overlapping executions and a query", + [](MYSQL * conn, MYSQL_STMT * stmt, Bind & bind) + { + bool rval = true; + + if (mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_stmt_execute(stmt) || + mysql_query(conn, "SET @a = 1")) + { + rval = false; + } + + return rval; + } + } + }; + + for (auto a : tests) + { + run_test(test, a); + } + + return test.global_result; +} diff --git a/maxscale-system-test/mxs1786_statistics.cpp b/maxscale-system-test/mxs1786_statistics.cpp new file mode 100644 index 000000000..0ed4c49b8 --- /dev/null +++ b/maxscale-system-test/mxs1786_statistics.cpp @@ -0,0 +1,24 @@ +/** + * MXS-1786: Hang with COM_STATISTICS + */ + +#include "testconnections.h" + +int main(int argc, char* argv[]) +{ + TestConnections test(argc, argv); + + test.maxscales->connect(); + + for (int i = 0; i < 10; i++) + { + test.set_timeout(10); + mysql_stat(test.maxscales->conn_rwsplit[0]); + test.try_query(test.maxscales->conn_rwsplit[0], "SELECT 1"); + } + + test.maxscales->disconnect(); + + + return test.global_result; +} diff --git a/maxscale-system-test/mxs1787_call_ps.cpp b/maxscale-system-test/mxs1787_call_ps.cpp new file mode 100644 index 000000000..364026874 --- /dev/null +++ b/maxscale-system-test/mxs1787_call_ps.cpp @@ -0,0 +1,62 @@ +/** + * MXS-1787: Crash with PS: CALL p1((SELECT f1()), ?) + */ + +#include "testconnections.h" + +using namespace std; + +struct Bind +{ + Bind() + { + bind.buffer = &data; + bind.buffer_type = MYSQL_TYPE_LONG; + bind.error = &err; + bind.is_null = &is_null; + bind.length = &length; + } + + MYSQL_BIND bind; + char err = 0; + char is_null = 0; + char is_unsigned = 0; + uint32_t data = 1234; + unsigned long length = sizeof(data); +}; + +int main(int argc, char* argv[]) +{ + TestConnections test(argc, argv); + + test.maxscales->connect(); + + execute_query(test.maxscales->conn_rwsplit[0], "USE test"); + execute_query(test.maxscales->conn_rwsplit[0], "CREATE OR REPLACE TABLE t1 AS SELECT 1 AS id"); + execute_query(test.maxscales->conn_rwsplit[0], "CREATE OR REPLACE FUNCTION f1() RETURNS INT DETERMINISTIC BEGIN RETURN 1; END"); + execute_query(test.maxscales->conn_rwsplit[0], "CREATE OR REPLACE PROCEDURE p1(IN i INT, IN j INT) BEGIN SELECT i + j; END"); + + test.maxscales->disconnect(); + + test.maxscales->connect(); + + MYSQL_STMT* stmt = mysql_stmt_init(test.maxscales->conn_rwsplit[0]); + std::string query = "CALL p1((SELECT f1()), ?)"; + Bind bind; + + test.set_timeout(30); + + test.assert(mysql_stmt_prepare(stmt, query.c_str(), query.size()) == 0, + "Prepared statement failure: %s", mysql_stmt_error(stmt)); + test.assert(mysql_stmt_bind_param(stmt, &bind.bind) == 0, + "Bind failure: %s", mysql_stmt_error(stmt)); + test.assert(mysql_stmt_execute(stmt) == 0, + "Execute failure: %s", mysql_stmt_error(stmt)); + + mysql_stmt_close(stmt); + + test.assert(mysql_query(test.maxscales->conn_rwsplit[0], "SELECT 1") == 0, "Normal queries should work"); + test.maxscales->disconnect(); + + return test.global_result; +} diff --git a/query_classifier/qc_sqlite/qc_sqlite.cc b/query_classifier/qc_sqlite/qc_sqlite.cc index 713586cdc..af49d29d8 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.cc +++ b/query_classifier/qc_sqlite/qc_sqlite.cc @@ -1074,7 +1074,8 @@ public: if (pExpr->flags & EP_xIsSelect) { - update_field_infos_from_subselect(pAliases, pExpr->x.pSelect, pExclude); + ss_dassert(pAliases); + update_field_infos_from_subselect(*pAliases, pExpr->x.pSelect, pExclude); if (zName) @@ -1299,7 +1300,7 @@ public: while (pPrior) { - update_field_infos_from_subselect(&aliases, pPrior, pExclude, + update_field_infos_from_subselect(aliases, pPrior, pExclude, IGNORE_COMPOUND_SELECTS); pPrior = pPrior->pPrior; } @@ -1307,12 +1308,12 @@ public: } } - void update_field_infos_from_subselect(QcAliases* pAliases, + void update_field_infos_from_subselect(const QcAliases& existing_aliases, const Select* pSelect, const ExprList* pExclude, compound_approach_t compound_approach = ANALYZE_COMPOUND_SELECTS) { - QcAliases aliases(*pAliases); + QcAliases aliases(existing_aliases); update_field_infos_from_select(aliases, pSelect, pExclude, compound_approach); } @@ -1325,7 +1326,8 @@ public: if (pCte->pSelect) { - update_field_infos_from_subselect(pAliases, pCte->pSelect, NULL); + ss_dassert(pAliases); + update_field_infos_from_subselect(*pAliases, pCte->pSelect, NULL); } } } @@ -2064,7 +2066,8 @@ public: if (pExprList) { - update_field_infos_from_exprlist(NULL, pExprList, NULL); + QcAliases aliases; + update_field_infos_from_exprlist(&aliases, pExprList, NULL); } exposed_sqlite3SrcListDelete(pParse->db, pName); diff --git a/query_classifier/test/maxscale.test b/query_classifier/test/maxscale.test index a0d037df2..63fed3f4c 100644 --- a/query_classifier/test/maxscale.test +++ b/query_classifier/test/maxscale.test @@ -100,3 +100,6 @@ SELECT '2005-01-01' - INTERVAL 1 SECOND; # MXS-1730 SELECT id as engine FROM users WHERE id = 1; + +# MXS-1787 +CALL p1((SELECT f1()), ?); diff --git a/server/core/internal/trxboundaryparser.hh b/server/core/internal/trxboundaryparser.hh index 8dc3310db..3bfc00fd1 100644 --- a/server/core/internal/trxboundaryparser.hh +++ b/server/core/internal/trxboundaryparser.hh @@ -607,7 +607,7 @@ private: if (m_pI != m_pEnd) { - MXS_WARNING("Non-space data found after semi-colon: '%.*s'.", + MXS_INFO("Non-space data found after semi-colon: '%.*s'.", (int)(m_pEnd - m_pI), m_pI); } diff --git a/server/core/modutil.cc b/server/core/modutil.cc index 735227f61..d381dd340 100644 --- a/server/core/modutil.cc +++ b/server/core/modutil.cc @@ -1071,6 +1071,21 @@ int modutil_count_statements(GWBUF* buffer) return num; } +int modutil_count_packets(GWBUF* buffer) +{ + int packets = 0; + size_t offset = 0; + uint8_t len[3]; + + while (gwbuf_copy_data(buffer, offset, 3, len) == 3) + { + ++packets; + offset += gw_mysql_get_byte3(len) + MYSQL_HEADER_LEN; + } + + return packets; +} + /** * Initialize the PCRE2 patterns used when converting MySQL wildcards to PCRE syntax. */ diff --git a/server/core/queryclassifier.cc b/server/core/queryclassifier.cc index f3815b1a4..31f9516f3 100644 --- a/server/core/queryclassifier.cc +++ b/server/core/queryclassifier.cc @@ -941,7 +941,7 @@ uint32_t QueryClassifier::get_target_type(QueryClassifier::current_target_t curr route_target = get_route_target(*command, *type, buffer->hint); } } - else + else if (load_data_state() == QueryClassifier::LOAD_DATA_ACTIVE) { /** Empty packet signals end of LOAD DATA LOCAL INFILE, send it to master*/ set_load_data_state(QueryClassifier::LOAD_DATA_END); diff --git a/server/modules/protocol/MySQL/mariadbclient/setsqlmodeparser.hh b/server/modules/protocol/MySQL/mariadbclient/setsqlmodeparser.hh index 73f63e711..f0400584d 100644 --- a/server/modules/protocol/MySQL/mariadbclient/setsqlmodeparser.hh +++ b/server/modules/protocol/MySQL/mariadbclient/setsqlmodeparser.hh @@ -562,7 +562,7 @@ private: if (m_pI != m_pEnd) { - MXS_WARNING("Non-space data found after semi-colon: '%.*s'.", + MXS_INFO("Non-space data found after semi-colon: '%.*s'.", (int)(m_pEnd - m_pI), m_pI); } diff --git a/server/modules/protocol/MySQL/mysql_common.cc b/server/modules/protocol/MySQL/mysql_common.cc index 8c4afa35a..0ab66f652 100644 --- a/server/modules/protocol/MySQL/mysql_common.cc +++ b/server/modules/protocol/MySQL/mysql_common.cc @@ -1668,8 +1668,7 @@ bool mxs_mysql_command_will_respond(uint8_t cmd) { return cmd != MXS_COM_STMT_SEND_LONG_DATA && cmd != MXS_COM_QUIT && - cmd != MXS_COM_STMT_CLOSE && - cmd != MXS_COM_STMT_FETCH; + cmd != MXS_COM_STMT_CLOSE; } typedef std::vector< std::pair > TargetList; diff --git a/server/modules/routing/maxinfo/maxinfo.c b/server/modules/routing/maxinfo/maxinfo.c index bd1ac2bbe..4792ab6dd 100644 --- a/server/modules/routing/maxinfo/maxinfo.c +++ b/server/modules/routing/maxinfo/maxinfo.c @@ -154,6 +154,7 @@ createInstance(SERVICE *service, char **options) return NULL; } + inst->sessions = NULL; inst->service = service; spinlock_init(&inst->lock); diff --git a/server/modules/routing/readwritesplit/rwbackend.cc b/server/modules/routing/readwritesplit/rwbackend.cc index 4dad84b74..015f7acba 100644 --- a/server/modules/routing/readwritesplit/rwbackend.cc +++ b/server/modules/routing/readwritesplit/rwbackend.cc @@ -11,7 +11,9 @@ RWBackend::RWBackend(SERVER_REF* ref): mxs::Backend(ref), m_reply_state(REPLY_STATE_DONE), m_large_packet(false), - m_command(0) + m_command(0), + m_open_cursor(false), + m_expected_rows(0) { } @@ -21,7 +23,8 @@ RWBackend::~RWBackend() bool RWBackend::execute_session_command() { - bool expect_response = mxs_mysql_command_will_respond(next_session_command()->get_command()); + m_command = next_session_command()->get_command(); + bool expect_response = mxs_mysql_command_will_respond(m_command); bool rval = mxs::Backend::execute_session_command(); if (rval && expect_response) @@ -66,6 +69,28 @@ bool RWBackend::write(GWBUF* buffer, response_type type) /** Replace the client handle with the real PS handle */ uint8_t* ptr = GWBUF_DATA(buffer) + MYSQL_PS_ID_OFFSET; gw_mysql_set_byte4(ptr, it->second); + + if (cmd == MXS_COM_STMT_EXECUTE) + { + // Extract the flag byte after the statement ID + uint8_t flags = 0; + gwbuf_copy_data(buffer, MYSQL_PS_ID_OFFSET + MYSQL_PS_ID_SIZE, 1, &flags); + + // Any non-zero flag value means that we have an open cursor + m_open_cursor = flags != 0; + } + else if (cmd == MXS_COM_STMT_FETCH) + { + ss_dassert(m_open_cursor); + // Number of rows to fetch is a 4 byte integer after the ID + uint8_t buf[4]; + gwbuf_copy_data(buffer, MYSQL_PS_ID_OFFSET + MYSQL_PS_ID_SIZE, 4, buf); + m_expected_rows = gw_mysql_get_byte4(buf); + } + else + { + m_open_cursor = false; + } } } @@ -78,6 +103,13 @@ void RWBackend::close(close_type type) mxs::Backend::close(type); } +bool RWBackend::consume_fetched_rows(GWBUF* buffer) +{ + m_expected_rows -= modutil_count_packets(buffer); + ss_dassert(m_expected_rows >= 0); + return m_expected_rows == 0; +} + static inline bool have_next_packet(GWBUF* buffer) { uint32_t len = MYSQL_GET_PAYLOAD_LEN(GWBUF_DATA(buffer)) + MYSQL_HEADER_LEN; @@ -94,7 +126,26 @@ static inline bool have_next_packet(GWBUF* buffer) */ bool RWBackend::reply_is_complete(GWBUF *buffer) { - if (get_reply_state() == REPLY_STATE_START && + if (current_command() == MXS_COM_STMT_FETCH) + { + bool more = false; + modutil_state state = {is_large_packet()}; + int n_eof = modutil_count_signal_packets(buffer, 0, &more, &state); + set_large_packet(state.state); + + // If the server responded with an error, n_eof > 0 + if (n_eof > 0 || consume_fetched_rows(buffer)) + { + + set_reply_state(REPLY_STATE_DONE); + } + } + else if (current_command() == MXS_COM_STATISTICS) + { + // COM_STATISTICS returns a single string and thus requires special handling + set_reply_state(REPLY_STATE_DONE); + } + else if (get_reply_state() == REPLY_STATE_START && (!mxs_mysql_is_result_set(buffer) || GWBUF_IS_COLLECTED_RESULT(buffer))) { if (GWBUF_IS_COLLECTED_RESULT(buffer) || @@ -145,6 +196,12 @@ bool RWBackend::reply_is_complete(GWBUF *buffer) { /** Waiting for the EOF packet after the rows */ set_reply_state(REPLY_STATE_RSET_ROWS); + + if (cursor_is_open()) + { + MXS_INFO("Cursor successfully opened"); + set_reply_state(REPLY_STATE_DONE); + } } else { diff --git a/server/modules/routing/readwritesplit/rwbackend.hh b/server/modules/routing/readwritesplit/rwbackend.hh index ff891148b..2340c909e 100644 --- a/server/modules/routing/readwritesplit/rwbackend.hh +++ b/server/modules/routing/readwritesplit/rwbackend.hh @@ -66,6 +66,9 @@ public: bool write(GWBUF* buffer, response_type type = EXPECT_RESPONSE); void close(close_type type = CLOSE_NORMAL); + // For COM_STMT_FETCH processing + bool consume_fetched_rows(GWBUF* buffer); + inline void set_large_packet(bool value) { m_large_packet = value; @@ -81,6 +84,11 @@ public: return m_command; } + inline bool cursor_is_open() const + { + return m_open_cursor; + } + bool reply_is_complete(GWBUF *buffer); private: @@ -90,6 +98,8 @@ private: *calculation for result sets when the result * contains very large rows */ uint8_t m_command; + bool m_open_cursor; /**< Whether we have an open cursor */ + uint32_t m_expected_rows; /**< Number of rows a COM_STMT_FETCH is retrieving */ }; } diff --git a/server/modules/routing/readwritesplit/rwsplitsession.cc b/server/modules/routing/readwritesplit/rwsplitsession.cc index f9f0dd67f..3d18187b7 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.cc +++ b/server/modules/routing/readwritesplit/rwsplitsession.cc @@ -116,7 +116,6 @@ int32_t RWSplitSession::routeQuery(GWBUF* querybuf) if (m_query_queue == NULL && (m_expected_responses == 0 || - mxs_mysql_get_command(querybuf) == MXS_COM_STMT_FETCH || m_qc.load_data_state() == QueryClassifier::LOAD_DATA_ACTIVE || m_qc.large_query())) { @@ -376,6 +375,13 @@ void RWSplitSession::clientReply(GWBUF *writebuf, DCB *backend_dcb) SRWBackend& backend = get_backend_from_dcb(backend_dcb); + if (m_qc.load_data_state() == QueryClassifier::LOAD_DATA_ACTIVE && + mxs_mysql_is_err_packet(writebuf)) + { + // Server responded with an error to the LOAD DATA LOCAL INFILE + m_qc.set_load_data_state(QueryClassifier::LOAD_DATA_INACTIVE); + } + if ((writebuf = handle_causal_read_reply(writebuf, backend)) == NULL) { return; // Nothing to route, return