From 4208dff2e6d95e336eb61270a2fc2d087b9743e9 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Fri, 22 Feb 2019 14:01:20 +0200 Subject: [PATCH 1/7] MXS-2337 Schemarouter responds correctly to SHOW TABLES The router now handles the query similarly to a "SHOW TABLES FROM X" with the current db assigned to X. --- .../routing/schemarouter/schemaroutersession.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/server/modules/routing/schemarouter/schemaroutersession.cc b/server/modules/routing/schemarouter/schemaroutersession.cc index f17fcc9a3..c47ea32ab 100644 --- a/server/modules/routing/schemarouter/schemaroutersession.cc +++ b/server/modules/routing/schemarouter/schemaroutersession.cc @@ -1592,8 +1592,17 @@ bool SchemaRouterSession::send_tables(GWBUF* pPacket) if (database.empty()) { - MXS_FREE(query); - return false; + // Was not a "show tables from x". If a current database is selected, use that as target. + if (!m_current_db.empty()) + { + database = m_current_db; + } + else + { + // No current db, route the query to a server, likely getting "No database selected" + MXS_FREE(query); + return false; + } } ServerMap tablelist; From e7f739e95df49f8bd6ee8ff2eb812255803f064e Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Mon, 4 Mar 2019 18:16:56 +0200 Subject: [PATCH 2/7] MXS-2337 Fix sharding test There is now enought time between queries that the shard map is reconstructed. --- maxscale-system-test/cnf/maxscale.cnf.template.sharding | 1 + maxscale-system-test/sharding.cpp | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.sharding b/maxscale-system-test/cnf/maxscale.cnf.template.sharding index 9d8a99454..c21d3634b 100755 --- a/maxscale-system-test/cnf/maxscale.cnf.template.sharding +++ b/maxscale-system-test/cnf/maxscale.cnf.template.sharding @@ -17,6 +17,7 @@ user=maxskysql password=skysql auth_all_servers=1 ignore_databases_regex=.* +refresh_interval=5 [Sharding Listener] type=listener diff --git a/maxscale-system-test/sharding.cpp b/maxscale-system-test/sharding.cpp index c16c7eb11..19191f622 100644 --- a/maxscale-system-test/sharding.cpp +++ b/maxscale-system-test/sharding.cpp @@ -75,11 +75,11 @@ int main(int argc, char* argv[]) "GRANT SELECT,USAGE,CREATE ON shard_db.* TO 'user%d'@'%%'", i), "Query should succeed."); + execute_query(Test->repl->nodes[i], "FLUSH PRIVILEGES"); } Test->repl->close_connections(); Test->stop_timeout(); - sleep(10); MYSQL* conn; for (i = 0; i < Test->repl->N; i++) @@ -97,6 +97,7 @@ int main(int argc, char* argv[]) Test->add_result(execute_query(conn, "CREATE TABLE table%d (x1 int, fl int);", i), "Query should succeed."); } + sleep(6); // The router is configured to refresh the shard map if older than 5 seconds. for (i = 0; i < Test->repl->N; i++) { From a7be3c527c4b3fbe49bce480f4ebceed1a10d5f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 5 Mar 2019 10:47:56 +0200 Subject: [PATCH 3/7] Remove unnecessary memory allocations Given the fact that there exist only three possible categories, the map can be replaced with a static array that needs no memory allocations. Making this array thread-local allows it to be reused which places an upper limit on the number of memory allocations. --- .../readwritesplit/rwsplit_select_backends.cc | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/server/modules/routing/readwritesplit/rwsplit_select_backends.cc b/server/modules/routing/readwritesplit/rwsplit_select_backends.cc index 8403e8340..dd9591550 100644 --- a/server/modules/routing/readwritesplit/rwsplit_select_backends.cc +++ b/server/modules/routing/readwritesplit/rwsplit_select_backends.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -201,6 +202,11 @@ BackendSelectFunction get_backend_select_function(select_criteria_t sc) return backend_cmp_current_load; } +namespace +{ +// Buffers for sorting the servers, thread_local to avoid repeated memory allocations +thread_local std::array priority_map; +} /** * @brief Find the best slave candidate for routing reads. @@ -216,7 +222,6 @@ SRWBackendVector::iterator find_best_backend(SRWBackendVector& backends, bool masters_accepts_reads) { // Group backends by priority. The set of highest priority backends will then compete. - std::map priority_map; int best_priority {INT_MAX}; // low numbers are high priority for (auto& psBackend : backends) @@ -230,16 +235,16 @@ SRWBackendVector::iterator find_best_backend(SRWBackendVector& backends, { if (!is_busy) { - priority = 1; // highest priority, idle servers + priority = 0; // highest priority, idle servers } else { - priority = 13; // lowest priority, busy servers + priority = 2; // lowest priority, busy servers } } else { - priority = 2; // idle masters with masters_accept_reads==false + priority = 1; // idle masters with masters_accept_reads==false } priority_map[priority].push_back(psBackend); @@ -247,8 +252,14 @@ SRWBackendVector::iterator find_best_backend(SRWBackendVector& backends, } auto best = select(priority_map[best_priority]); + auto rval = std::find(backends.begin(), backends.end(), *best); - return std::find(backends.begin(), backends.end(), *best); + for (auto& a : priority_map) + { + a.clear(); + } + + return rval; } /** From b97976c4eeb97cd4e60f32bddf53a30350915ca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 5 Mar 2019 11:20:34 +0200 Subject: [PATCH 4/7] MXS-2323: Close stale connections Cleaning up and closing stale connections to servers in maintenance mode helps administrators see when a server is no longer in use. --- .../routing/readwritesplit/rwsplitsession.cc | 20 +++++++++++++++++++ .../routing/readwritesplit/rwsplitsession.hh | 1 + 2 files changed, 21 insertions(+) diff --git a/server/modules/routing/readwritesplit/rwsplitsession.cc b/server/modules/routing/readwritesplit/rwsplitsession.cc index 68e7b810a..76d53d293 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.cc +++ b/server/modules/routing/readwritesplit/rwsplitsession.cc @@ -563,6 +563,17 @@ static bool server_is_shutting_down(GWBUF* writebuf) return err == ER_SERVER_SHUTDOWN || err == ER_NORMAL_SHUTDOWN || err == ER_SHUTDOWN_COMPLETE; } +void RWSplitSession::close_stale_connections() +{ + for (auto& backend : m_backends) + { + if (backend->in_use() && !backend->can_connect()) + { + backend->close(); + } + } +} + void RWSplitSession::clientReply(GWBUF* writebuf, DCB* backend_dcb) { DCB* client_dcb = backend_dcb->session->client_dcb; @@ -721,6 +732,15 @@ void RWSplitSession::clientReply(GWBUF* writebuf, DCB* backend_dcb) m_can_replay_trx = true; } + if (m_expected_responses == 0) + { + /** + * Close stale connections to servers in maintenance. Done here to avoid closing the connections + * before all responses have been received. + */ + close_stale_connections(); + } + if (backend->in_use() && backend->has_session_commands()) { // Backend is still in use and has more session commands to execute diff --git a/server/modules/routing/readwritesplit/rwsplitsession.hh b/server/modules/routing/readwritesplit/rwsplitsession.hh index 242fee32f..8472b9deb 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.hh +++ b/server/modules/routing/readwritesplit/rwsplitsession.hh @@ -187,6 +187,7 @@ private: void continue_large_session_write(GWBUF* querybuf, uint32_t type); bool route_single_stmt(GWBUF* querybuf); bool route_stored_query(); + void close_stale_connections(); mxs::SRWBackend get_hinted_backend(char* name); mxs::SRWBackend get_slave_backend(int max_rlag); From 5b43940559bbb4f4081b1fb12efe0297610f8092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 5 Mar 2019 12:24:15 +0200 Subject: [PATCH 5/7] Track session state only when required The protocol should not track the session state as the parsing is quite expensive with the current code. This change is a workaround that enables the parsing only when required. A proper way to handle this would be to do all the response processing in one place thus avoiding the duplication of work. --- include/maxscale/buffer.h | 6 +++++- include/maxscale/protocol/mysql.h | 1 + .../modules/protocol/MySQL/mariadbbackend/mysql_backend.cc | 5 ++++- server/modules/protocol/MySQL/mysql_common.cc | 1 + server/modules/routing/readwritesplit/rwsplit_route_stmt.cc | 5 +++++ 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/include/maxscale/buffer.h b/include/maxscale/buffer.h index 3dafdb0b9..63dc2ce10 100644 --- a/include/maxscale/buffer.h +++ b/include/maxscale/buffer.h @@ -56,6 +56,7 @@ typedef enum GWBUF_TYPE_RESULT = (1 << 3), GWBUF_TYPE_REPLY_OK = (1 << 4), GWBUF_TYPE_REPLAYED = (1 << 5), + GWBUF_TYPE_TRACK_STATE = (1 << 6), } gwbuf_type_t; #define GWBUF_IS_TYPE_UNDEFINED(b) ((b)->gwbuf_type == 0) @@ -65,7 +66,10 @@ typedef enum #define GWBUF_IS_REPLY_OK(b) ((b)->gwbuf_type & GWBUF_TYPE_REPLY_OK) // True if the query is not initiated by the client but an internal replaying mechanism -#define GWBUF_IS_REPLAYED(b) ((b)->gwbuf_type & GWBUF_TYPE_REPLAYED) +#define GWBUF_IS_REPLAYED(b) ((b)->gwbuf_type & GWBUF_TYPE_REPLAYED) + +// Track session state change response +#define GWBUF_SHOULD_TRACK_STATE(b) ((b)->gwbuf_type & GWBUF_TYPE_TRACK_STATE) typedef enum { diff --git a/include/maxscale/protocol/mysql.h b/include/maxscale/protocol/mysql.h index 9c8ff0783..cc506a949 100644 --- a/include/maxscale/protocol/mysql.h +++ b/include/maxscale/protocol/mysql.h @@ -338,6 +338,7 @@ typedef struct GWBUF* stored_query; /*< Temporarily stored queries */ bool collect_result; /*< Collect the next result set as one buffer */ bool changing_user; + bool track_state; /*< Track session state */ uint32_t num_eof_packets; /*< Encountered eof packet number, used for check * packet type */ bool large_query; /*< Whether to ignore the command byte of the next diff --git a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc index 30fc9eeab..2236f7da8 100644 --- a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc +++ b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc @@ -447,6 +447,8 @@ static inline void prepare_for_write(DCB* dcb, GWBUF* buffer) { proto->collect_result = true; } + + proto->track_state = GWBUF_SHOULD_TRACK_STATE(buffer); } /******************************************************************************* @@ -836,7 +838,8 @@ static int gw_read_and_write(DCB* dcb) * The OK packets sent in response to COM_STMT_PREPARE are of a different * format so we need to detect and skip them. */ if (rcap_type_required(capabilities, RCAP_TYPE_SESSION_STATE_TRACKING) - && !expecting_ps_response(proto)) + && !expecting_ps_response(proto) + && proto->track_state) { mxs_mysql_get_session_track_info(tmp, proto); } diff --git a/server/modules/protocol/MySQL/mysql_common.cc b/server/modules/protocol/MySQL/mysql_common.cc index cdb8fd1e1..c575304f4 100644 --- a/server/modules/protocol/MySQL/mysql_common.cc +++ b/server/modules/protocol/MySQL/mysql_common.cc @@ -63,6 +63,7 @@ MySQLProtocol* mysql_protocol_init(DCB* dcb, int fd) p->changing_user = false; p->num_eof_packets = 0; p->large_query = false; + p->track_state = false; /*< Assign fd with protocol */ p->fd = fd; p->owner_dcb = dcb; diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc index 4f24c7e70..05e94e66f 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc @@ -280,6 +280,11 @@ bool RWSplitSession::route_single_stmt(GWBUF* querybuf) } else if (TARGET_IS_MASTER(route_target)) { + if (m_config.causal_reads) + { + gwbuf_set_type(querybuf, GWBUF_TYPE_TRACK_STATE); + } + succp = handle_master_is_target(&target); if (!succp && should_migrate_trx(target)) From 48d2f3bd84dd61a0823cef79eccc5f88d13c2196 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 7 Mar 2019 15:46:14 +0200 Subject: [PATCH 6/7] Correct REST-API-Tutorial --- Documentation/Tutorials/REST-API-Tutorial.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Documentation/Tutorials/REST-API-Tutorial.md b/Documentation/Tutorials/REST-API-Tutorial.md index 397b42f77..7eaa0d2f1 100644 --- a/Documentation/Tutorials/REST-API-Tutorial.md +++ b/Documentation/Tutorials/REST-API-Tutorial.md @@ -53,7 +53,7 @@ now secure and can be used across networks. ## Requesting Data **Note:** For the sake of brevity, the rest of this tutorial will omit the -TLS/SSL options for the `curl` command line. For more information, refer to the +TLS/SSL options from the `curl` command line. For more information, refer to the `curl` manpage. The most basic task to do with the REST API is to see whether MaxScale is up and @@ -272,8 +272,7 @@ execute the following command. curl -X PATCH -d @server1.txt 127.0.0.1:8989/v1/servers/server1 ``` -To verify that the data was updated correctly, request the updated created -object. +To verify that the data was updated correctly, request the updated object. ``` curl 127.0.0.1:8989/v1/servers/server1 From 267ec9ccccdabef15832089ca3e0503e30ada5e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 7 Mar 2019 16:02:49 +0200 Subject: [PATCH 7/7] Fix filter serialization Filters were serialized with commas as separators instead of pipes. --- server/core/service.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/service.cc b/server/core/service.cc index 4ca6afd80..e73dbd5b9 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -1850,7 +1850,7 @@ bool Service::dump_config(const char* filename) const for (const auto& f : m_filters) { dprintf(file, "%s%s", sep, f->name.c_str()); - sep = ","; + sep = "|"; } dprintf(file, "\n");