From 3a1c2119fb0c9d7dcdbba39ab786335da763f25c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 17 Apr 2018 14:57:54 +0300 Subject: [PATCH] MXS-1804: Fix hanging of large session commands Large session commands weren't properly handled which caused the router to think that the trailing end of a multi-packet query was actually a new query. This cannot be confidently solved in 2.2 which is why the router session is now closed the moment a large session command is noticed. --- Documentation/About/Limitations.md | 3 ++ maxscale-system-test/CMakeLists.txt | 4 ++ maxscale-system-test/mxs1804_long_ps_hang.cpp | 41 +++++++++++++++++++ .../readwritesplit/rwsplit_route_stmt.cc | 31 ++++++++------ 4 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 maxscale-system-test/mxs1804_long_ps_hang.cpp diff --git a/Documentation/About/Limitations.md b/Documentation/About/Limitations.md index fff8ceaf5..fc8fbfd70 100644 --- a/Documentation/About/Limitations.md +++ b/Documentation/About/Limitations.md @@ -217,6 +217,9 @@ SELECT ..INTO variable|OUTFILE|DUMPFILE SET autocommit=1|0 ``` +Session commands that are 2²⁴ - 1 bytes or longer are not supported and +cause the session to be closed. + 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 diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 23b6708fb..02586c926 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -648,6 +648,10 @@ add_test_executable(mxs1786_statistics.cpp mxs1786_statistics replication LABELS # https://jira.mariadb.org/browse/MXS-1787 add_test_executable(mxs1787_call_ps.cpp mxs1787_call_ps replication LABELS readwritesplit REPL_BACKEND) +# MXS-1804: request 16M-1 stmt_prepare command packet connect hang +# https://jira.mariadb.org/browse/MXS-1804 +add_test_executable(mxs1804_long_ps_hang.cpp mxs1804_long_ps_hang replication LABELS readwritesplit REPL_BACKEND) + # MXS-1808: Crash with mysql_stmt_send_long_data # https://jira.mariadb.org/browse/MXS-1808 add_test_executable(mxs1808_long_data.cpp mxs1808_long_data replication LABELS readwritesplit REPL_BACKEND) diff --git a/maxscale-system-test/mxs1804_long_ps_hang.cpp b/maxscale-system-test/mxs1804_long_ps_hang.cpp new file mode 100644 index 000000000..2479bee33 --- /dev/null +++ b/maxscale-system-test/mxs1804_long_ps_hang.cpp @@ -0,0 +1,41 @@ +/** + * MXS-1804: request 16M-1 stmt_prepare command packet connect hang + * + * https://jira.mariadb.org/browse/MXS-1804 + */ + +#include "testconnections.h" + +int sql_str_size(int sqlsize) +{ + char prefx[] = "select ''"; + return sqlsize - strlen(prefx) - 1; +} + +void gen_select_sqlstr(char *sqlstr, unsigned int strsize, int sqlsize) +{ + strcpy(sqlstr, "select '"); + memset(sqlstr + strlen("select '"), 'f', strsize); + sqlstr[sqlsize - 2] = '\''; + sqlstr[sqlsize - 1] = '\0'; +} + +int main(int argc, char** argv) +{ + TestConnections test(argc, argv); + int sqlsize = 16777215; + int strsize = sql_str_size(sqlsize); + + char* sqlstr = (char *)malloc(sqlsize); + gen_select_sqlstr(sqlstr, strsize, sqlsize); + + test.set_timeout(30); + test.maxscales->connect(); + + MYSQL_STMT* stmt = mysql_stmt_init(test.maxscales->conn_rwsplit[0]); + test.assert(mysql_stmt_prepare(stmt, sqlstr, strlen(sqlstr)) != 0, "Prepare should fail in 2.2 but not hang", + mysql_stmt_error(stmt)); + mysql_stmt_close(stmt); + + return test.global_result; +} diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc index f9d943fbc..a9ad6c16e 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc @@ -290,6 +290,18 @@ bool route_single_stmt(RWSplit *inst, RWSplitSession *rses, GWBUF *querybuf, con return succp; } +static inline bool is_large_query(GWBUF* buf) +{ + uint32_t buflen = gwbuf_length(buf); + + // The buffer should contain at most (2^24 - 1) + 4 bytes ... + ss_dassert(buflen <= MYSQL_HEADER_LEN + GW_MYSQL_MAX_PACKET_LEN); + // ... and the payload should be buflen - 4 bytes + ss_dassert(MYSQL_GET_PAYLOAD_LEN(GWBUF_DATA(buf)) == buflen - MYSQL_HEADER_LEN); + + return buflen == MYSQL_HEADER_LEN + GW_MYSQL_MAX_PACKET_LEN; +} + /** * Execute in backends used by current router session. * Save session variable commands to router session property @@ -313,6 +325,13 @@ bool route_single_stmt(RWSplit *inst, RWSplitSession *rses, GWBUF *querybuf, con bool route_session_write(RWSplitSession *rses, GWBUF *querybuf, uint8_t command, uint32_t type) { + if (is_large_query(querybuf)) + { + MXS_ERROR("Session command is too large, session cannot continue. " + "Large session commands are not supported in 2.2."); + return false; + } + /** The SessionCommand takes ownership of the buffer */ uint64_t id = rses->sescmd_count++; mxs::SSessionCommand sescmd(new mxs::SessionCommand(querybuf, id)); @@ -1120,18 +1139,6 @@ static inline bool query_creates_reply(uint8_t cmd) cmd != MXS_COM_STMT_CLOSE; } -static inline bool is_large_query(GWBUF* buf) -{ - uint32_t buflen = gwbuf_length(buf); - - // The buffer should contain at most (2^24 - 1) + 4 bytes ... - ss_dassert(buflen <= MYSQL_HEADER_LEN + GW_MYSQL_MAX_PACKET_LEN); - // ... and the payload should be buflen - 4 bytes - ss_dassert(MYSQL_GET_PAYLOAD_LEN(GWBUF_DATA(buf)) == buflen - MYSQL_HEADER_LEN); - - return buflen == MYSQL_HEADER_LEN + GW_MYSQL_MAX_PACKET_LEN; -} - /** * @brief Handle writing to a target server *