diff --git a/include/maxscale/protocol/mysql.h b/include/maxscale/protocol/mysql.h index 90795e36b..33386ec42 100644 --- a/include/maxscale/protocol/mysql.h +++ b/include/maxscale/protocol/mysql.h @@ -454,9 +454,22 @@ mxs_auth_state_t gw_send_backend_auth(DCB *dcb); /** Write an OK packet to a DCB */ int mxs_mysql_send_ok(DCB *dcb, int sequence, uint8_t affected_rows, const char* message); -/** Check for OK packet */ +/** + * @brief Check if the buffer contains an OK packet + * + * @param buffer Buffer containing a complete MySQL packet + * @return True if the buffer contains an OK packet + */ bool mxs_mysql_is_ok_packet(GWBUF *buffer); +/** + * @brief Check if the buffer contains an ERR packet + * + * @param buffer Buffer containing a complete MySQL packet + * @return True if the buffer contains an ERR packet + */ +bool mxs_mysql_is_err_packet(GWBUF *buffer); + /** * @brief Check if a buffer contains a result set * diff --git a/server/modules/protocol/MySQL/mysql_common.c b/server/modules/protocol/MySQL/mysql_common.c index ecf7fb684..e7803e9c9 100644 --- a/server/modules/protocol/MySQL/mysql_common.c +++ b/server/modules/protocol/MySQL/mysql_common.c @@ -1498,23 +1498,18 @@ bool gw_read_backend_handshake(DCB *dcb, GWBUF *buffer) return rval; } -/** - * @brief Check if the buffer contains an OK packet - * - * @param buffer Buffer containing a complete MySQL packet - * @return True if the buffer contains an OK packet - */ bool mxs_mysql_is_ok_packet(GWBUF *buffer) { - bool rval = false; - uint8_t cmd; + uint8_t cmd = 0xff; // Default should differ from the OK packet + gwbuf_copy_data(buffer, MYSQL_HEADER_LEN, 1, &cmd); + return cmd == MYSQL_REPLY_OK; +} - if (gwbuf_copy_data(buffer, MYSQL_HEADER_LEN, 1, &cmd) && cmd == MYSQL_REPLY_OK) - { - rval = true; - } - - return rval; +bool mxs_mysql_is_err_packet(GWBUF *buffer) +{ + uint8_t cmd = 0x00; // Default should differ from the ERR packet + gwbuf_copy_data(buffer, MYSQL_HEADER_LEN, 1, &cmd); + return cmd == MYSQL_REPLY_ERR; } bool mxs_mysql_is_result_set(GWBUF *buffer) diff --git a/server/modules/routing/readwritesplit/readwritesplit.cc b/server/modules/routing/readwritesplit/readwritesplit.cc index a1390a8fa..d6fad38dc 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.cc +++ b/server/modules/routing/readwritesplit/readwritesplit.cc @@ -1076,6 +1076,40 @@ static json_t* diagnostics_json(const MXS_ROUTER *instance) return rval; } +static void log_unexpected_response(DCB* dcb, GWBUF* buffer) +{ + if (mxs_mysql_is_err_packet(buffer)) + { + /** This should be the only valid case where the server sends a response + * without the client sending one first. MaxScale does not yet advertise + * the progress reporting flag so we don't need to handle it. */ + uint8_t* data = GWBUF_DATA(buffer); + size_t len = MYSQL_GET_PAYLOAD_LEN(data); + uint16_t errcode = MYSQL_GET_ERRCODE(data); + std::string errstr((char*)data + 7, (char*)data + 7 + len - 3); + + if (errcode == ER_CONNECTION_KILLED) + { + MXS_INFO("Connection from '%s'@'%s' to '%s' was killed", + dcb->session->client_dcb->user, + dcb->session->client_dcb->remote, + dcb->server->unique_name); + } + else + { + MXS_WARNING("Server '%s' sent an unexpected error: %hu, %s", + dcb->server->unique_name, errcode, errstr.c_str()); + } + } + else + { + MXS_ERROR("Unexpected internal state: received response 0x%02hhx from " + "server '%s' when no response was expected", + mxs_mysql_get_command(buffer), dcb->server->unique_name); + ss_dassert(false); + } +} + /** * @brief Client Reply routine * @@ -1102,9 +1136,18 @@ static void clientReply(MXS_ROUTER *instance, SRWBackend backend = get_backend_from_dcb(rses, backend_dcb); + if (backend->get_reply_state() == REPLY_STATE_DONE) + { + /** If we receive an unexpected response from the server, the internal + * logic cannot handle this situation. Routing the reply straight to + * the client should be the safest thing to do at this point. */ + log_unexpected_response(backend_dcb, writebuf); + MXS_SESSION_ROUTE_REPLY(backend_dcb->session, writebuf); + return; + } + /** Statement was successfully executed, free the stored statement */ session_clear_stmt(backend_dcb->session); - ss_dassert(backend->get_reply_state() != REPLY_STATE_DONE); if (reply_is_complete(backend, writebuf)) {