From 7e48941a044b5931ea308eb8542d2869fafc00c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 14 Sep 2017 01:00:12 +0300 Subject: [PATCH 01/16] MXS-1412: Process response buffers individually By processing each buffer individually, the need to iterate over the whole resultset is removed. Profiling showed that most of the time was spent navigating the linked list of buffers when an offset into the whole resultset was used instead of an offset to the individual response buffer. --- server/modules/filter/maxrows/maxrows.c | 37 ++++++++++++------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/server/modules/filter/maxrows/maxrows.c b/server/modules/filter/maxrows/maxrows.c index 5fa0e9aaa..a13b6c238 100644 --- a/server/modules/filter/maxrows/maxrows.c +++ b/server/modules/filter/maxrows/maxrows.c @@ -211,7 +211,7 @@ static void maxrows_session_data_free(MAXROWS_SESSION_DATA *data); static int handle_expecting_fields(MAXROWS_SESSION_DATA *csdata); static int handle_expecting_nothing(MAXROWS_SESSION_DATA *csdata); static int handle_expecting_response(MAXROWS_SESSION_DATA *csdata); -static int handle_rows(MAXROWS_SESSION_DATA *csdata); +static int handle_rows(MAXROWS_SESSION_DATA *csdata, GWBUF* buffer, size_t extra_offset); static int handle_ignoring_response(MAXROWS_SESSION_DATA *csdata); static bool process_params(char **options, MXS_CONFIG_PARAMETER *params, @@ -458,7 +458,7 @@ static int clientReply(MXS_FILTER *instance, break; case MAXROWS_EXPECTING_ROWS: - rv = handle_rows(csdata); + rv = handle_rows(csdata, data, 0); break; case MAXROWS_IGNORING_RESPONSE: @@ -608,7 +608,7 @@ static int handle_expecting_fields(MAXROWS_SESSION_DATA *csdata) } csdata->state = MAXROWS_EXPECTING_ROWS; - rv = handle_rows(csdata); + rv = handle_rows(csdata, csdata->res.data,csdata->res.offset); break; default: // Field information. @@ -767,28 +767,29 @@ static int handle_expecting_response(MAXROWS_SESSION_DATA *csdata) * * @param csdata The maxrows session data. */ -static int handle_rows(MAXROWS_SESSION_DATA *csdata) +static int handle_rows(MAXROWS_SESSION_DATA *csdata, GWBUF* buffer, size_t extra_offset) { ss_dassert(csdata->state == MAXROWS_EXPECTING_ROWS); ss_dassert(csdata->res.data); int rv = 1; bool insufficient = false; - size_t buflen = csdata->res.length; + size_t offset = extra_offset; + size_t buflen = gwbuf_length(buffer); - while (!insufficient && (buflen - csdata->res.offset >= MYSQL_HEADER_LEN)) + while (!insufficient && (buflen - offset >= MYSQL_HEADER_LEN)) { bool pending_large_data = csdata->large_packet; // header array holds a full EOF packet uint8_t header[MYSQL_EOF_PACKET_LEN]; - gwbuf_copy_data(csdata->res.data, - csdata->res.offset, + gwbuf_copy_data(buffer, + offset, MYSQL_EOF_PACKET_LEN, header); size_t packetlen = MYSQL_HEADER_LEN + MYSQL_GET_PAYLOAD_LEN(header); - if (csdata->res.offset + packetlen <= buflen) + if (offset + packetlen <= buflen) { /* Check for large packet packet terminator: * min is 4 bytes "0x0 0x0 0x0 0xseq_no and @@ -800,10 +801,8 @@ static int handle_rows(MAXROWS_SESSION_DATA *csdata) packetlen < MYSQL_EOF_PACKET_LEN)) { // Update offset, number of rows and break - csdata->res.offset += packetlen; + offset += packetlen; csdata->res.n_rows++; - - ss_dassert(csdata->res.offset == buflen); break; } @@ -817,9 +816,7 @@ static int handle_rows(MAXROWS_SESSION_DATA *csdata) // Mark the beginning of a large packet receiving csdata->large_packet = true; // Just update offset and break - csdata->res.offset += packetlen; - - ss_dassert(csdata->res.offset == buflen); + offset += packetlen; break; } else @@ -834,8 +831,7 @@ static int handle_rows(MAXROWS_SESSION_DATA *csdata) switch (command) { case 0xff: // ERR packet after the rows. - csdata->res.offset += packetlen; - ss_dassert(csdata->res.offset == buflen); + offset += packetlen; // This is the end of resultset: set big packet var to false csdata->large_packet = false; @@ -874,8 +870,7 @@ static int handle_rows(MAXROWS_SESSION_DATA *csdata) * NOTE: not supported right now */ case 0xfe: // EOF, the one after the rows. - csdata->res.offset += packetlen; - ss_dassert(csdata->res.offset == buflen); + offset += packetlen; /* EOF could be the last packet in the transmission: * check first whether SERVER_MORE_RESULTS_EXIST flag is set. @@ -940,7 +935,7 @@ static int handle_rows(MAXROWS_SESSION_DATA *csdata) case 0xfb: // NULL default: // length-encoded-string - csdata->res.offset += packetlen; + offset += packetlen; // Increase res.n_rows counter while not receiving large packets if (!csdata->large_packet) { @@ -972,6 +967,8 @@ static int handle_rows(MAXROWS_SESSION_DATA *csdata) } } + csdata->res.offset += offset; + return rv; } From 8905c3aa3437b8c8a9081bf9a285315ded5bd6c2 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 14 Sep 2017 15:33:39 +0200 Subject: [PATCH 02/16] MXS-1411: fix error message and log priority MXS-1411: fix error message and log priority --- server/modules/filter/maxrows/maxrows.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/server/modules/filter/maxrows/maxrows.c b/server/modules/filter/maxrows/maxrows.c index a13b6c238..06b1fd594 100644 --- a/server/modules/filter/maxrows/maxrows.c +++ b/server/modules/filter/maxrows/maxrows.c @@ -637,8 +637,27 @@ static int handle_expecting_nothing(MAXROWS_SESSION_DATA *csdata) { ss_dassert(csdata->state == MAXROWS_EXPECTING_NOTHING); ss_dassert(csdata->res.data); - MXS_ERROR("Received data from the backend although we were expecting nothing."); - ss_dassert(!true); + unsigned long msg_size = gwbuf_length(csdata->res.data); + + if ((int)MYSQL_GET_COMMAND(GWBUF_DATA(csdata->res.data)) == 0xff) + { + /** + * Error text message is after: + * MYSQL_HEADER_LEN offset + status flag (1) + error code (2) + + * 6 bytes message status = MYSQL_HEADER_LEN + 9 + */ + MXS_INFO("Error packet received from backend " + "(possibly a server shut down ?): [%s].", + GWBUF_DATA(csdata->res.data) + MYSQL_HEADER_LEN + 9); + } + else + { + MXS_WARNING("Received data from the backend although " + "filter is expecting nothing. " + "Packet size is %lu bytes long.", + msg_size); + ss_dassert(!true); + } return send_upstream(csdata); } From 9dd7f2174cd93ec98154cd8ac631631b5308a041 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Fri, 15 Sep 2017 12:05:28 +0200 Subject: [PATCH 03/16] MXS-1412: while discarding a result set don't buffer any data. MXS-1412: while discarding a result set don't buffer any data: this avoids to store useless data. Additionally the colum definitions buffer is used instead of the offset value. --- server/modules/filter/maxrows/maxrows.c | 69 ++++++++++++++++--------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/server/modules/filter/maxrows/maxrows.c b/server/modules/filter/maxrows/maxrows.c index 06b1fd594..7c82f42e9 100644 --- a/server/modules/filter/maxrows/maxrows.c +++ b/server/modules/filter/maxrows/maxrows.c @@ -185,8 +185,8 @@ typedef struct maxrows_response_state size_t n_fields; /**< How many fields we have received, <= n_totalfields. */ size_t n_rows; /**< How many rows we have received. */ size_t offset; /**< Where we are in the response buffer. */ - size_t rows_offset; /**< Offset to first row in result set */ size_t length; /**< Buffer size. */ + GWBUF* column_defs; /**< Buffer with result set columns definitions */ } MAXROWS_RESPONSE_STATE; static void maxrows_response_state_reset(MAXROWS_RESPONSE_STATE *state); @@ -415,8 +415,18 @@ static int clientReply(MXS_FILTER *instance, if (csdata->res.data) { - gwbuf_append(csdata->res.data, data); - csdata->res.length += gwbuf_length(data); + if (csdata->discard_resultset && + csdata->state == MAXROWS_EXPECTING_ROWS) + { + gwbuf_free(csdata->res.data); + csdata->res.data = data; + csdata->res.length = gwbuf_length(data); + } + else + { + gwbuf_append(csdata->res.data, data); + csdata->res.length += gwbuf_length(data); + } } else { @@ -495,7 +505,6 @@ static void diagnostics(MXS_FILTER *instance, MXS_FILTER_SESSION *sdata, DCB *dc dcb_printf(dcb, "Maxrows filter is working\n"); } - /** * Capability routine. * @@ -520,7 +529,7 @@ static void maxrows_response_state_reset(MAXROWS_RESPONSE_STATE *state) state->n_fields = 0; state->n_rows = 0; state->offset = 0; - state->rows_offset = 0; + state->column_defs = NULL; } /** @@ -599,16 +608,18 @@ static int handle_expecting_fields(MAXROWS_SESSION_DATA *csdata) case 0xfe: // EOF, the one after the fields. csdata->res.offset += packetlen; - /* Now set the offset to the first resultset - * this could be used for empty response handler + /** + * Set the buffer with column definitions. + * This will be used only by the empty response handler. */ - if (!csdata->res.rows_offset) + if (!csdata->res.column_defs && + csdata->instance->config.m_return == MAXROWS_RETURN_EMPTY) { - csdata->res.rows_offset = csdata->res.offset; + csdata->res.column_defs = gwbuf_clone(csdata->res.data); } csdata->state = MAXROWS_EXPECTING_ROWS; - rv = handle_rows(csdata, csdata->res.data,csdata->res.offset); + rv = handle_rows(csdata, csdata->res.data, csdata->res.offset); break; default: // Field information. @@ -641,11 +652,6 @@ static int handle_expecting_nothing(MAXROWS_SESSION_DATA *csdata) if ((int)MYSQL_GET_COMMAND(GWBUF_DATA(csdata->res.data)) == 0xff) { - /** - * Error text message is after: - * MYSQL_HEADER_LEN offset + status flag (1) + error code (2) + - * 6 bytes message status = MYSQL_HEADER_LEN + 9 - */ MXS_INFO("Error packet received from backend " "(possibly a server shut down ?): [%s].", GWBUF_DATA(csdata->res.data) + MYSQL_HEADER_LEN + 9); @@ -1016,12 +1022,20 @@ static int send_upstream(MAXROWS_SESSION_DATA *csdata) ss_dassert(csdata->res.data != NULL); /* Free a saved SQL not freed by send_error_upstream() */ - if (csdata->input_sql) + if (csdata->instance->config.m_return == MAXROWS_RETURN_ERR) { gwbuf_free(csdata->input_sql); csdata->input_sql = NULL; } + /* Free a saved columndefs not freed by send_eof_upstream() */ + if (csdata->instance->config.m_return == MAXROWS_RETURN_EMPTY) + { + gwbuf_free(csdata->res.column_defs); + csdata->res.column_defs = NULL; + } + + /* Send data to client */ int rv = csdata->up.clientReply(csdata->up.instance, csdata->up.session, csdata->res.data); @@ -1046,22 +1060,24 @@ static int send_eof_upstream(MAXROWS_SESSION_DATA *csdata) /* Sequence byte is #3 */ uint8_t eof[MYSQL_EOF_PACKET_LEN] = {05, 00, 00, 01, 0xfe, 00, 00, 02, 00}; GWBUF *new_pkt = NULL; + + ss_dassert(csdata->res.data != NULL); + ss_dassert(csdata->res.column_defs != NULL); + /** * The offset to server reply pointing to * next byte after column definitions EOF * of the first result set. */ - size_t offset = csdata->res.rows_offset; - - ss_dassert(csdata->res.data != NULL); + size_t offset = gwbuf_length(csdata->res.column_defs); /* Data to send + added EOF */ uint8_t *new_result = MXS_MALLOC(offset + MYSQL_EOF_PACKET_LEN); if (new_result) { - /* Get contiguous data from beginning to specified offset */ - gwbuf_copy_data(csdata->res.data, 0, offset, new_result); + /* Get contiguous data from saved columns defintions buffer */ + gwbuf_copy_data(csdata->res.column_defs, 0, offset, new_result); /* Increment sequence number for the EOF being added for empty resultset: * last one if found in EOF terminating column def @@ -1094,9 +1110,12 @@ static int send_eof_upstream(MAXROWS_SESSION_DATA *csdata) rv = 0; } - /* Free full input buffer */ + /* Free all data buffers */ gwbuf_free(csdata->res.data); + gwbuf_free(csdata->res.column_defs); + csdata->res.data = NULL; + csdata->res.column_defs = NULL; return rv; } @@ -1134,6 +1153,8 @@ static int send_ok_upstream(MAXROWS_SESSION_DATA *csdata) int rv = csdata->up.clientReply(csdata->up.instance, csdata->up.session, packet); + + /* Free server result buffer */ gwbuf_free(csdata->res.data); csdata->res.data = NULL; @@ -1215,10 +1236,10 @@ static int send_error_upstream(MAXROWS_SESSION_DATA *csdata) /* Free server result buffer */ gwbuf_free(csdata->res.data); + csdata->res.data = NULL; + /* Free input_sql buffer */ gwbuf_free(csdata->input_sql); - - csdata->res.data = NULL; csdata->input_sql = NULL; return rv; From fb12e4c0aa5291bcaca4587e4f4acaa42b0adbfb Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Fri, 15 Sep 2017 15:30:39 +0200 Subject: [PATCH 04/16] MXS-1411: additional fix to error message MXS-1411: additional fix to error message --- server/modules/filter/maxrows/maxrows.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/modules/filter/maxrows/maxrows.c b/server/modules/filter/maxrows/maxrows.c index 7c82f42e9..c4ac347bb 100644 --- a/server/modules/filter/maxrows/maxrows.c +++ b/server/modules/filter/maxrows/maxrows.c @@ -652,8 +652,14 @@ static int handle_expecting_nothing(MAXROWS_SESSION_DATA *csdata) if ((int)MYSQL_GET_COMMAND(GWBUF_DATA(csdata->res.data)) == 0xff) { + /** + * Error text message is after: + * MYSQL_HEADER_LEN offset + status flag (1) + error code (2) + + * 6 bytes message status = MYSQL_HEADER_LEN + 9 + */ MXS_INFO("Error packet received from backend " - "(possibly a server shut down ?): [%s].", + "(possibly a server shut down ?): [%.*s].", + (int)msg_size - (MYSQL_HEADER_LEN + 9), GWBUF_DATA(csdata->res.data) + MYSQL_HEADER_LEN + 9); } else From d1b742eaa5dd7d9b7d8f2ff39d62bad6eb1337bd Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 15 Sep 2017 17:06:35 +0300 Subject: [PATCH 05/16] Log details if cache received unexpected packet from server --- .../filter/cache/cachefiltersession.cc | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/server/modules/filter/cache/cachefiltersession.cc b/server/modules/filter/cache/cachefiltersession.cc index f537b7abe..f75b15e40 100644 --- a/server/modules/filter/cache/cachefiltersession.cc +++ b/server/modules/filter/cache/cachefiltersession.cc @@ -515,8 +515,28 @@ int CacheFilterSession::handle_expecting_nothing() { ss_dassert(m_state == CACHE_EXPECTING_NOTHING); ss_dassert(m_res.pData); - MXS_ERROR("Received data from the backend althoug we were expecting nothing."); - ss_dassert(!true); + unsigned long msg_size = gwbuf_length(m_res.pData); + + if ((int)MYSQL_GET_COMMAND(GWBUF_DATA(m_res.pData)) == 0xff) + { + /** + * Error text message is after: + * MYSQL_HEADER_LEN offset + status flag (1) + error code (2) + + * 6 bytes message status = MYSQL_HEADER_LEN + 9 + */ + MXS_INFO("Error packet received from backend " + "(possibly a server shut down ?): [%.*s].", + (int)msg_size - (MYSQL_HEADER_LEN + 9), + GWBUF_DATA(m_res.pData) + MYSQL_HEADER_LEN + 9); + } + else + { + MXS_WARNING("Received data from the backend although " + "filter is expecting nothing. " + "Packet size is %lu bytes long.", + msg_size); + ss_dassert(!true); + } return send_upstream(); } From 3fc498e4cc2056af3196b04fc88d302dc014aaaf Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 15 Sep 2017 17:14:47 +0300 Subject: [PATCH 06/16] Update version number to 2.1.8 --- VERSION21.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION21.cmake b/VERSION21.cmake index 43a3b21b1..41a573525 100644 --- a/VERSION21.cmake +++ b/VERSION21.cmake @@ -5,7 +5,7 @@ set(MAXSCALE_VERSION_MAJOR "2" CACHE STRING "Major version") set(MAXSCALE_VERSION_MINOR "1" CACHE STRING "Minor version") -set(MAXSCALE_VERSION_PATCH "7" CACHE STRING "Patch version") +set(MAXSCALE_VERSION_PATCH "8" CACHE STRING "Patch version") # This should only be incremented if a package is rebuilt set(MAXSCALE_BUILD_NUMBER 1 CACHE STRING "Release number") From 2d02cc997373db0169ca5243ba4ccb8e8e6f2f26 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 15 Sep 2017 17:23:20 +0300 Subject: [PATCH 07/16] Update ChangeLog, Upgrading and Release Notes --- Documentation/Changelog.md | 1 + .../MaxScale-2.1.8-Release-Notes.md | 46 +++++++++++++++++++ .../Upgrading/Upgrading-To-MaxScale-2.1.md | 1 + 3 files changed, 48 insertions(+) create mode 100644 Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md diff --git a/Documentation/Changelog.md b/Documentation/Changelog.md index 9a08f24e6..4a5d6f8a2 100644 --- a/Documentation/Changelog.md +++ b/Documentation/Changelog.md @@ -21,6 +21,7 @@ * MaxScale now supports IPv6 For more details, please refer to: +* [MariaDB MaxScale 2.1.8 Release Notes](Release-Notes/MaxScale-2.1.8-Release-Notes.md) * [MariaDB MaxScale 2.1.7 Release Notes](Release-Notes/MaxScale-2.1.7-Release-Notes.md) * [MariaDB MaxScale 2.1.6 Release Notes](Release-Notes/MaxScale-2.1.6-Release-Notes.md) * [MariaDB MaxScale 2.1.5 Release Notes](Release-Notes/MaxScale-2.1.5-Release-Notes.md) diff --git a/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md new file mode 100644 index 000000000..21fa975be --- /dev/null +++ b/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md @@ -0,0 +1,46 @@ +# MariaDB MaxScale 2.1.8 Release Notes + +Release 2.1.8 is a GA release. + +This document describes the changes in release 2.1.8, when compared to +release [2.1.7](MaxScale-2.1.7-Release-Notes.md). + +If you are upgrading from release 2.0, please also read the following +release notes: +[2.1.7](./MaxScale-2.1.7-Release-Notes.md) +[2.1.6](./MaxScale-2.1.6-Release-Notes.md) +[2.1.5](./MaxScale-2.1.5-Release-Notes.md) +[2.1.4](./MaxScale-2.1.4-Release-Notes.md) +[2.1.3](./MaxScale-2.1.3-Release-Notes.md) +[2.1.2](./MaxScale-2.1.2-Release-Notes.md) +[2.1.1](./MaxScale-2.1.1-Release-Notes.md) +[2.1.0](./MaxScale-2.1.0-Release-Notes.md) + +For any problems you encounter, please consider submitting a bug +report at [Jira](https://jira.mariadb.org). + +## Bug fixes + +[Here is a list of bugs fixed in MaxScale 2.1.8.](https://jira.mariadb.org/issues/?jql=project%20%3D%20MXS%20AND%20issuetype%20%3D%20Bug%20AND%20status%20%3D%20Closed%20AND%20fixVersion%20%3D%202.1.8) + +* (MXS-1421)[https://jira.mariadb.org/browse/MXS-1421] Even though limit is reached, maxrows continues to buffer resultset. +* (MXS-1414)[https://jira.mariadb.org/browse/MXS-1414] About Presistent Connection Mysql Gone away +* (MXS-1412)[https://jira.mariadb.org/browse/MXS-1412] Performance issue with MaxRows filter +* (MXS-1411)[https://jira.mariadb.org/browse/MXS-1411] error : (46) [maxrows] Received data from the backend although we were expecting nothing. +* (MXS-1400)[https://jira.mariadb.org/browse/MXS-1400] Crash with OpenSSL 1.1 +* (MXS-1396)[https://jira.mariadb.org/browse/MXS-1396] Persistent connections hang with Percona Server 5.6.37-82.2-log + +## Packaging + +RPM and Debian packages are provided for the Linux distributions supported +by MariaDB Enterprise. + +Packages can be downloaded [here](https://mariadb.com/resources/downloads). + +## Source Code + +The source code of MaxScale is tagged at GitHub with a tag, which is identical +with the version of MaxScale. For instance, the tag of version X.Y.Z of MaxScale +is maxscale-X.Y.Z. + +The source code is available [here](https://github.com/mariadb-corporation/MaxScale). diff --git a/Documentation/Upgrading/Upgrading-To-MaxScale-2.1.md b/Documentation/Upgrading/Upgrading-To-MaxScale-2.1.md index 0b5665f1b..8bdbeedb3 100644 --- a/Documentation/Upgrading/Upgrading-To-MaxScale-2.1.md +++ b/Documentation/Upgrading/Upgrading-To-MaxScale-2.1.md @@ -7,6 +7,7 @@ For more information about MariaDB MaxScale 2.1, please refer to the [ChangeLog](../Changelog.md). For a complete list of changes in MaxScale 2.1, refer to the +[MaxScale 2.1.8 Release Notes](../Release-Notes/MaxScale-2.1.8-Release-Notes.md). [MaxScale 2.1.7 Release Notes](../Release-Notes/MaxScale-2.1.7-Release-Notes.md). [MaxScale 2.1.6 Release Notes](../Release-Notes/MaxScale-2.1.6-Release-Notes.md). [MaxScale 2.1.5 Release Notes](../Release-Notes/MaxScale-2.1.5-Release-Notes.md). From 9267f8ad70b782bdfed10fe43d57d94a5e2f59e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Sat, 16 Sep 2017 07:27:32 +0300 Subject: [PATCH 08/16] MXS-1418: Keep connections open if server is removed The removal of a server from a service is intended to affect only new sessions. Added a test that checks that the connections are kept open even if the server is removed from the service. --- maxscale-system-test/CMakeLists.txt | 4 + maxscale-system-test/mxs1418.cpp | 73 +++++++++++++++++++ .../routing/readconnroute/readconnroute.c | 5 -- .../readwritesplit/rwsplit_route_stmt.c | 5 +- 4 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 maxscale-system-test/mxs1418.cpp diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 96caa88e6..6ff24e028 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -484,6 +484,10 @@ add_test_executable(mxs1123.cpp mxs1123 mxs1123 LABELS maxscale REPL_BACKEND) # https://jira.mariadb.org/browse/MXS-1319 add_test_executable(mxs1319.cpp mxs1319 replication LABELS MySQLAuth REPL_BACKEND) +# MXS-1418: Removing a server from a service breaks the connection +# https://jira.mariadb.org/browse/MXS-1418 +add_test_executable(mxs1418.cpp mxs1418 replication LABELS maxscale REPL_BACKEND) + # 'namedserverfilter' test add_test_executable(namedserverfilter.cpp namedserverfilter namedserverfilter LABELS namedserverfilter LIGHT REPL_BACKEND) diff --git a/maxscale-system-test/mxs1418.cpp b/maxscale-system-test/mxs1418.cpp new file mode 100644 index 000000000..9bd11efe6 --- /dev/null +++ b/maxscale-system-test/mxs1418.cpp @@ -0,0 +1,73 @@ +/** + * @file Check that removing a server from a service doesn't break active connections + */ + +#include "testconnections.h" + +static volatile bool running = true; + +void* thr(void* data) +{ + TestConnections* test = (TestConnections*)data; + + while (running && test->global_result == 0) + { + test->set_timeout(60); + if (test->try_query(test->conn_rwsplit, "SELECT 1")) + { + test->tprintf("Failed to select via readwritesplit"); + } + if (test->try_query(test->conn_master, "SELECT 1")) + { + test->tprintf("Failed to select via readconnroute master"); + } + if (test->try_query(test->conn_slave, "SELECT 1")) + { + test->tprintf("Failed to select via readconnroute slave"); + } + } + + test->stop_timeout(); + + return NULL; +} + +int main(int argc, char *argv[]) +{ + TestConnections test(argc, argv); + test.connect_maxscale(); + + test.tprintf("Connect to MaxScale and continuously execute queries"); + pthread_t thread; + pthread_create(&thread, NULL, thr, &test); + sleep(5); + + test.tprintf("Remove all servers from all services"); + + for (int i = 3; i > -1; i--) + { + test.ssh_maxscale(true, "maxadmin remove server server%d \"RW Split Router\"", i); + test.ssh_maxscale(true, "maxadmin remove server server%d \"Read Connection Router Slave\"", i); + test.ssh_maxscale(true, "maxadmin remove server server%d \"Read Connection Router Master\"", i); + } + + sleep(5); + + test.tprintf("Stop queries and close the connections"); + running = false; + pthread_join(thread, NULL); + test.close_maxscale_connections(); + + test.tprintf("Add all servers to all services"); + + for (int i = 3; i > -1; i--) + { + test.ssh_maxscale(true, "maxadmin add server server%d \"RW Split Router\"", i); + test.ssh_maxscale(true, "maxadmin add server server%d \"Read Connection Router Slave\"", i); + test.ssh_maxscale(true, "maxadmin add server server%d \"Read Connection Router Master\"", i); + } + + test.check_maxscale_alive(); + + return test.global_result; +} diff --git a/server/modules/routing/readconnroute/readconnroute.c b/server/modules/routing/readconnroute/readconnroute.c index 9c0cd68da..c996dc6c4 100644 --- a/server/modules/routing/readconnroute/readconnroute.c +++ b/server/modules/routing/readconnroute/readconnroute.c @@ -522,10 +522,6 @@ static void log_closed_session(mysql_server_cmd_t mysql_command, bool is_closed, { sprintf(msg, "Server '%s' is down.", ref->server->unique_name); } - else if (!SERVER_REF_IS_ACTIVE(ref)) - { - sprintf(msg, "Server '%s' was removed from the service.", ref->server->unique_name); - } else if (SERVER_IN_MAINT(ref->server)) { sprintf(msg, "Server '%s' is in maintenance.", ref->server->unique_name); @@ -579,7 +575,6 @@ routeQuery(MXS_ROUTER *instance, MXS_ROUTER_SESSION *router_session, GWBUF *queu } if (rses_is_closed || backend_dcb == NULL || - !SERVER_REF_IS_ACTIVE(router_cli_ses->backend) || !SERVER_IS_RUNNING(router_cli_ses->backend->server)) { log_closed_session(mysql_command, rses_is_closed, router_cli_ses->backend); diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.c b/server/modules/routing/readwritesplit/rwsplit_route_stmt.c index 98277e048..b8352215e 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.c +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.c @@ -502,7 +502,6 @@ bool rwsplit_get_dcb(DCB **p_dcb, ROUTER_CLIENT_SES *rses, backend_type_t btype, * server, or master. */ if (BREF_IS_IN_USE((&backend_ref[i])) && - SERVER_REF_IS_ACTIVE(b) && (strncasecmp(name, b->server->unique_name, PATH_MAX) == 0) && (SERVER_IS_SLAVE(&server) || SERVER_IS_RELAY_SERVER(&server) || SERVER_IS_MASTER(&server))) @@ -537,7 +536,7 @@ bool rwsplit_get_dcb(DCB **p_dcb, ROUTER_CLIENT_SES *rses, backend_type_t btype, * Unused backend or backend which is not master nor * slave can't be used */ - if (!BREF_IS_IN_USE(&backend_ref[i]) || !SERVER_REF_IS_ACTIVE(b) || + if (!BREF_IS_IN_USE(&backend_ref[i]) || (!SERVER_IS_MASTER(&server) && !SERVER_IS_SLAVE(&server))) { continue; @@ -627,7 +626,7 @@ bool rwsplit_get_dcb(DCB **p_dcb, ROUTER_CLIENT_SES *rses, backend_type_t btype, */ if (btype == BE_MASTER) { - if (master_bref && SERVER_REF_IS_ACTIVE(master_bref->ref)) + if (master_bref) { /** It is possible for the server status to change at any point in time * so copying it locally will make possible error messages From d003983e5ac43e313a2a07c6b059daf1c7c83053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Sat, 16 Sep 2017 07:44:25 +0300 Subject: [PATCH 09/16] MXS-1409: Improve MaxAdmin error messages If both socket and network options are listed, a clear error message is printed. The usage is also split into two lines to make it clear that the options should be used separately. --- client/maxadmin.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/client/maxadmin.c b/client/maxadmin.c index 42333959d..d7e4856cd 100644 --- a/client/maxadmin.c +++ b/client/maxadmin.c @@ -200,6 +200,7 @@ main(int argc, char **argv) { // Both unix socket path and at least of the internet socket // options have been provided. + printf("\nError: Both socket and network options are provided\n\n"); DoUsage(argv[0]); exit(EXIT_FAILURE); } @@ -812,8 +813,8 @@ DoUsage(const char *progname) { PrintVersion(progname); printf("The MaxScale administrative and monitor client.\n\n"); - printf("Usage: %s [(-S socket)|([-u user] [-p password] [-h hostname] [-P port])]" - "[ | ]\n\n", progname); + printf("Usage: %s [-S socket] \n", progname); + printf(" %s [-u user] [-p password] [-h hostname] [-P port] \n\n", progname); printf(" -S|--socket=... The UNIX domain socket to connect to, The default is\n"); printf(" %s\n", MAXADMIN_DEFAULT_SOCKET); printf(" -u|--user=... The user name to use for the connection, default\n"); From a4bad5ffd2b17dbd701c393e632873ecb353ff24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Sep 2017 13:41:24 +0300 Subject: [PATCH 10/16] Use shorter timeouts in mxs1323_retry_read The configuration for mxs1323_retry_read now uses shorter timeouts for monitors. This should help the monitors detect the server failures before the result of the SELECT returns. also increased the time the query sleeps before returning. --- maxscale-system-test/cnf/maxscale.cnf.template.mxs1323 | 6 +++++- maxscale-system-test/mxs1323_retry_read.cpp | 8 +++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.mxs1323 b/maxscale-system-test/cnf/maxscale.cnf.template.mxs1323 index f5fff9976..031dbf9a1 100644 --- a/maxscale-system-test/cnf/maxscale.cnf.template.mxs1323 +++ b/maxscale-system-test/cnf/maxscale.cnf.template.mxs1323 @@ -1,6 +1,8 @@ [maxscale] threads=###threads### log_info=1 +auth_read_timeout=1 +auth_connect_timeout=1 [MySQL Monitor] type=monitor @@ -9,7 +11,9 @@ module=mysqlmon servers= server1,server2 user=maxskysql passwd= skysql -monitor_interval=500 +monitor_interval=1000 +backend_read_timeout=1 +backend_connect_timeout=1 [RW Split Router] type=service diff --git a/maxscale-system-test/mxs1323_retry_read.cpp b/maxscale-system-test/mxs1323_retry_read.cpp index 7d17f337f..3bd64f621 100644 --- a/maxscale-system-test/mxs1323_retry_read.cpp +++ b/maxscale-system-test/mxs1323_retry_read.cpp @@ -18,7 +18,7 @@ std::string do_query(TestConnections& test) { MYSQL* conn = test.open_rwsplit_connection(); - const char* query = "SELECT SLEEP(10), @@server_id"; + const char* query = "SELECT SLEEP(15), @@server_id"; char output[512] = ""; find_field(conn, query, "@@server_id", output); @@ -38,11 +38,13 @@ int main(int argc, char *argv[]) test.repl->close_connections(); test.set_timeout(60); - test.add_result(do_query(test) != slave, "The slave should respond to the first query"); + std::string res = do_query(test); + test.add_result(res != slave, "The slave should respond to the first query: %s", res.c_str()); pthread_t thr; pthread_create(&thr, NULL, async_block, &test); - test.add_result(do_query(test) != master, "The master should respond to the second query"); + res = do_query(test); + test.add_result(res != master, "The master should respond to the second query: %s", res.c_str()); pthread_join(thr, NULL); test.repl->unblock_node(1); From d168493ddf792e58ab6731fbc504bf5d1c455c1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Sep 2017 20:32:28 +0300 Subject: [PATCH 11/16] Fix maxrows offset calculation The code that handles the resultset rows added the extra offset given as a parameter into the total offset when it should've be ignored. --- server/modules/filter/maxrows/maxrows.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/filter/maxrows/maxrows.c b/server/modules/filter/maxrows/maxrows.c index c4ac347bb..633cf5088 100644 --- a/server/modules/filter/maxrows/maxrows.c +++ b/server/modules/filter/maxrows/maxrows.c @@ -998,7 +998,7 @@ static int handle_rows(MAXROWS_SESSION_DATA *csdata, GWBUF* buffer, size_t extra } } - csdata->res.offset += offset; + csdata->res.offset += offset - extra_offset; return rv; } From d72375ebb5f8f9af623d606db17025f07d40000b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Sep 2017 20:51:52 +0300 Subject: [PATCH 12/16] Reset resultset offset when discarding response The offset into the resultset buffer needs to be reset before each processed packet if the resultset is being discarded. --- server/modules/filter/maxrows/maxrows.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/modules/filter/maxrows/maxrows.c b/server/modules/filter/maxrows/maxrows.c index 633cf5088..598a3f5ce 100644 --- a/server/modules/filter/maxrows/maxrows.c +++ b/server/modules/filter/maxrows/maxrows.c @@ -421,6 +421,7 @@ static int clientReply(MXS_FILTER *instance, gwbuf_free(csdata->res.data); csdata->res.data = data; csdata->res.length = gwbuf_length(data); + csdata->res.offset = 0; } else { From aaa60d37e64230ea7ef4c6ec80ced75866f0e754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Sep 2017 22:03:30 +0300 Subject: [PATCH 13/16] Add missing parameter documentation to maxrows Documented new parameters. --- server/modules/filter/maxrows/maxrows.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/modules/filter/maxrows/maxrows.c b/server/modules/filter/maxrows/maxrows.c index 598a3f5ce..d1e745dec 100644 --- a/server/modules/filter/maxrows/maxrows.c +++ b/server/modules/filter/maxrows/maxrows.c @@ -795,9 +795,13 @@ static int handle_expecting_response(MAXROWS_SESSION_DATA *csdata) } /** - * Called when resultset rows are handled. + * Called when resultset rows are handled * - * @param csdata The maxrows session data. + * @param csdata The maxrows session data + * @param buffer The buffer containing the packet + * @param extra_offset Offset into @c buffer where the packet is stored + * + * @return The return value of the upstream component */ static int handle_rows(MAXROWS_SESSION_DATA *csdata, GWBUF* buffer, size_t extra_offset) { From e8a9a393ae9ecdf1be839d12b935fa8258f5d774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Sep 2017 22:55:05 +0300 Subject: [PATCH 14/16] Fix bug and issue list scripts The scripts now properly handle quoted CSV values with the help of a Perl script. --- Documentation/list_fixed_bugs.sh | 2 +- Documentation/list_fixed_issues.sh | 2 +- Documentation/process.pl | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100755 Documentation/process.pl diff --git a/Documentation/list_fixed_bugs.sh b/Documentation/list_fixed_bugs.sh index ac39dc1aa..8be27538f 100755 --- a/Documentation/list_fixed_bugs.sh +++ b/Documentation/list_fixed_bugs.sh @@ -8,4 +8,4 @@ then fi version=$1 -curl -s "https://jira.mariadb.org/sr/jira.issueviews:searchrequest-csv-current-fields/temp/SearchRequest.csv?jqlQuery=project+%3D+MXS+AND+issuetype+%3D+Bug+AND+status+%3D+Closed+AND+fixVersion+%3D+$version"|cut -f 1,2 -d ,|tail -n+2|sed 's/\(.*\),\(.*\)/* (\2)[https:\/\/jira.mariadb.org\/browse\/\2] \1/' +curl -s "https://jira.mariadb.org/sr/jira.issueviews:searchrequest-csv-current-fields/temp/SearchRequest.csv?jqlQuery=project+%3D+MXS+AND+issuetype+%3D+Bug+AND+status+%3D+Closed+AND+fixVersion+%3D+$version" | process.pl diff --git a/Documentation/list_fixed_issues.sh b/Documentation/list_fixed_issues.sh index e40aebf87..19707a3c4 100755 --- a/Documentation/list_fixed_issues.sh +++ b/Documentation/list_fixed_issues.sh @@ -8,4 +8,4 @@ then fi version=$1 -curl -s "https://jira.mariadb.org/sr/jira.issueviews:searchrequest-csv-current-fields/temp/SearchRequest.csv?jqlQuery=project+%3D+MXS+AND+issuetype+%21%3D+Bug+AND+status+%3D+Closed+AND+fixVersion+%3D+$version"|cut -f 1,2 -d ,|tail -n+2|sed 's/\(.*\),\(.*\)/* (\2)[https:\/\/jira.mariadb.org\/browse\/\2] \1/' +curl -s "https://jira.mariadb.org/sr/jira.issueviews:searchrequest-csv-current-fields/temp/SearchRequest.csv?jqlQuery=project+%3D+MXS+AND+issuetype+%21%3D+Bug+AND+status+%3D+Closed+AND+fixVersion+%3D+$version"|process.pl diff --git a/Documentation/process.pl b/Documentation/process.pl new file mode 100755 index 000000000..f325b96de --- /dev/null +++ b/Documentation/process.pl @@ -0,0 +1,20 @@ +#!/bin/perl + +# Discard the CSV headers +<>; + +while (<>) +{ + # Replace commas that are inside double quotes + s/("[^"]*),([^"]*")/$1$2/g; + + # Replace the double quotes themselves + s/"([^"]*)"/$1/g; + + # Split the line and grab the issue number and description + my @parts = split(/,/); + my $issue = @parts[1]; + my $desc = @parts[0]; + + print "* ($issue)[https://jira.mariadb.org/browse/$issue] $desc\n"; +} From 15c16e378be439a42a92172f2cfc9eceb4e095e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Sep 2017 23:12:01 +0300 Subject: [PATCH 15/16] Add fixed bugs to 2.1.8 release notes Added MXS-1418 and MXS-1409 to release notes. --- Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md index 21fa975be..6084c535f 100644 --- a/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md @@ -24,9 +24,11 @@ report at [Jira](https://jira.mariadb.org). [Here is a list of bugs fixed in MaxScale 2.1.8.](https://jira.mariadb.org/issues/?jql=project%20%3D%20MXS%20AND%20issuetype%20%3D%20Bug%20AND%20status%20%3D%20Closed%20AND%20fixVersion%20%3D%202.1.8) * (MXS-1421)[https://jira.mariadb.org/browse/MXS-1421] Even though limit is reached, maxrows continues to buffer resultset. +* (MXS-1418)[https://jira.mariadb.org/browse/MXS-1418] remove server does not drain node * (MXS-1414)[https://jira.mariadb.org/browse/MXS-1414] About Presistent Connection Mysql Gone away * (MXS-1412)[https://jira.mariadb.org/browse/MXS-1412] Performance issue with MaxRows filter * (MXS-1411)[https://jira.mariadb.org/browse/MXS-1411] error : (46) [maxrows] Received data from the backend although we were expecting nothing. +* (MXS-1409)[https://jira.mariadb.org/browse/MXS-1409] maxadmin socket with port results in help * (MXS-1400)[https://jira.mariadb.org/browse/MXS-1400] Crash with OpenSSL 1.1 * (MXS-1396)[https://jira.mariadb.org/browse/MXS-1396] Persistent connections hang with Percona Server 5.6.37-82.2-log From 8ff0c0ad6ddf4a3c44703c1687d330799a8ef03c Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Wed, 20 Sep 2017 10:01:27 +0300 Subject: [PATCH 16/16] Update release date --- Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md index 6084c535f..167b0ec3e 100644 --- a/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-2.1.8-Release-Notes.md @@ -1,4 +1,4 @@ -# MariaDB MaxScale 2.1.8 Release Notes +# MariaDB MaxScale 2.1.8 Release Notes -- 2017-09-20 Release 2.1.8 is a GA release.