From e5d7fc967ede53407a65bfde3faec3181e35f19f Mon Sep 17 00:00:00 2001 From: Elena Stepanova Date: Wed, 4 Jan 2017 13:03:30 +0200 Subject: [PATCH] MDEV-10100 main.pool_of_threads fails sporadically in buildbot Backport the fix to 5.5, because it fails there too The patch fixes two test failures: - on slow builders, sometimes a connection attempt which should fail due to the exceeded number of thread_pool_max_threads actually succeeds; - on even slow builders, MTR sometimes cannot establish the initial connection, and check-testcase fails prior to the test start The problem with check-testcase was caused by connect-timeout=2 which was set for all clients in the test config file. On slow builders it might be not enough. There is no way to override it for the pre-test check, so it needed to be substantially increased or removed. The other problem was caused by a race condition between sleeps that the test performs in existing connections and the connect timeout for the connection attempt which was expected to fail. If sleeps finished before the connect-timeout was exceeded, it would allow the connection to succeed. To solve each problem without making the other one worse, connect-timeout should be configured dynamically during the test. Due to the nature of the test (all connections must be busy at the moment when we need to change the timeout, and cannot execute SET GLOBAL ...), it needs to be done independently from the server. The solution: - recognize 'connect_timeout' as a connection option in mysqltest's "connect" command; - remove connect-timeout from the test configuration file; - use the new connect_timeout option for those connections which are expected to fail; - re-arrange the test flow to allow running a huge SLEEP without affecting the test execution time (because it would be interrupted after the main test flow is finished). The test is still subject to false negatives, e.g. if the connection fails due to timeout rather than due to the exceeded number of allowed threads, or if the connection on extra port succeeds due to a race condition and not because the special logic for the extra port. But those false negatives have always been possible there on slow builders, they should not be critical because faster builders should catch such failures if they appear. Conflicts: client/mysqltest.cc mysql-test/r/pool_of_threads.result mysql-test/t/pool_of_threads.test --- client/mysqltest.cc | 8 +++++ mysql-test/r/pool_of_threads.result | 21 +++++++------ mysql-test/t/pool_of_threads.cnf | 3 -- mysql-test/t/pool_of_threads.test | 46 ++++++++++++++++------------- 4 files changed, 44 insertions(+), 34 deletions(-) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index 5daa0e72270..d9fa9bc0bef 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -5900,6 +5900,7 @@ void do_connect(struct st_command *command) my_bool con_ssl= 0, con_compress= 0; my_bool con_pipe= 0; my_bool con_shm __attribute__ ((unused))= 0; + int connect_timeout= 0; struct st_connection* con_slot; static DYNAMIC_STRING ds_connection_name; @@ -5996,6 +5997,9 @@ void do_connect(struct st_command *command) con_pipe= 1; else if (length == 3 && !strncmp(con_options, "SHM", 3)) con_shm= 1; + else if (strncasecmp(con_options, "connect_timeout=", + sizeof("connect_timeout=")-1) == 0) + connect_timeout= atoi(con_options + sizeof("connect_timeout=")-1); else die("Illegal option to connect: %.*s", (int) (end - con_options), con_options); @@ -6066,6 +6070,10 @@ void do_connect(struct st_command *command) if (opt_protocol) mysql_options(con_slot->mysql, MYSQL_OPT_PROTOCOL, (char*) &opt_protocol); + if (connect_timeout) + mysql_options(con_slot->mysql, MYSQL_OPT_CONNECT_TIMEOUT, + (char*)&connect_timeout); + #ifdef HAVE_SMEM if (con_shm) { diff --git a/mysql-test/r/pool_of_threads.result b/mysql-test/r/pool_of_threads.result index 7acb45121d6..f153ebc48f4 100644 --- a/mysql-test/r/pool_of_threads.result +++ b/mysql-test/r/pool_of_threads.result @@ -2157,23 +2157,22 @@ Warnings: Warning 1052 Column 'kundentyp' in group statement is ambiguous drop table t1; SET optimizer_switch=@save_optimizer_switch; -SELECT sleep(5); -SELECT sleep(5); +SELECT sleep(50); +SELECT sleep(50); # -- Success: more than --thread_pool_max_threads normal connections not possible -sleep(5) -0 -sleep(5) -0 -SELECT sleep(5); -SELECT sleep(5); SELECT 'Connection on extra port ok'; Connection on extra port ok Connection on extra port ok +SELECT sleep(5.5); SELECT 'Connection on extra port 2 ok'; Connection on extra port 2 ok Connection on extra port 2 ok # -- Success: more than --extra-max-connections + 1 normal connections not possible -sleep(5) -0 -sleep(5) +KILL QUERY ; +KILL QUERY ; +sleep(50) +1 +sleep(50) +1 +sleep(5.5) 0 diff --git a/mysql-test/t/pool_of_threads.cnf b/mysql-test/t/pool_of_threads.cnf index c03e1da6450..f6651c878de 100644 --- a/mysql-test/t/pool_of_threads.cnf +++ b/mysql-test/t/pool_of_threads.cnf @@ -7,8 +7,5 @@ loose-thread_pool_max_threads= 2 extra-port= @ENV.MASTER_EXTRA_PORT extra-max-connections=1 -[client] -connect-timeout= 2 - [ENV] MASTER_EXTRA_PORT= @OPT.port diff --git a/mysql-test/t/pool_of_threads.test b/mysql-test/t/pool_of_threads.test index 4600128ff43..f13a096985c 100644 --- a/mysql-test/t/pool_of_threads.test +++ b/mysql-test/t/pool_of_threads.test @@ -15,20 +15,26 @@ SET optimizer_switch=@save_optimizer_switch; # connections on the extra port. # First set two connections running, and check that extra connection -# on normal port fails due to--thread-pool-max_threads=2 +# on normal port fails due to --thread-pool-max-threads=2. +# We can afford using a really long sleep, because we won't wait +# till it ends, we'll interrupt it as soon as we don't need it anymore + connection default; -send SELECT sleep(5); +--let $con1_id= `SELECT CONNECTION_ID()` + +send SELECT sleep(50); --sleep 1 connect(con2,localhost,root,,); -connection con2; -send SELECT sleep(5); +--let $con2_id= `SELECT CONNECTION_ID()` + +send SELECT sleep(50); --sleep 0.5 --disable_abort_on_error --disable_result_log --disable_query_log -connect(con3,localhost,root,,); +connect(con3,localhost,root,,,,,connect_timeout=2); --enable_query_log --enable_result_log --enable_abort_on_error @@ -42,24 +48,15 @@ if ($error) --echo # -- Success: more than --thread_pool_max_threads normal connections not possible } -connection default; ---reap -connection con2; ---reap - -# Now try again, but this time use the extra port to successfully connect. - -connection default; -send SELECT sleep(5); - -connection con2; -send SELECT sleep(5); ---sleep 1 - connect(extracon,127.0.0.1,root,,test,$MASTER_EXTRA_PORT,); connection extracon; SELECT 'Connection on extra port ok'; +# Here, sleep just for slightly longer than 5 sec to trigger MDEV-4566 +# (abort in interruptible wait connection check). +send SELECT sleep(5.5); + + connect(extracon2,127.0.0.1,root,,test,$MASTER_EXTRA_PORT,); connection extracon2; SELECT 'Connection on extra port 2 ok'; @@ -67,7 +64,7 @@ SELECT 'Connection on extra port 2 ok'; --disable_abort_on_error --disable_result_log --disable_query_log -connect(extracon3,127.0.0.1,root,,test,$MASTER_EXTRA_PORT,); +connect(extracon3,127.0.0.1,root,,test,$MASTER_EXTRA_PORT,,connect_timeout=2); --enable_query_log --enable_result_log --enable_abort_on_error @@ -81,7 +78,16 @@ if ($error) --echo # -- Success: more than --extra-max-connections + 1 normal connections not possible } +connection extracon2; +--replace_result $con1_id +eval KILL QUERY $con1_id; +--replace_result $con2_id +eval KILL QUERY $con2_id; + connection default; --reap connection con2; --reap + +connection extracon; +--reap