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