Bug #20201006 spamming show processlist prevents old

connection threads from cleaning up.

Analysis
=========
Issue here is, delay in connection cleanup for which global
connection counter is already decremented, makes room for
new connections. Hence more than expected connections are
observed in the server.

Connections running statement "SHOW PROCESSLIST" or "SELECT
on INFORMATION_SCHEMA.PROCESSLIST" acquires mutex
LOCK_thd_remove for reading information of all the connections
in server. Connections in cleanup phase, acquires mutex to
remove thread from global thread list. Many such concurrent
connections increases contention on mutex LOCK_thd_remove.

In connection cleanup phase, connection count is decreased
first and then the thd is removed from global thd list. This
order makes new connection (above max_connections) possible
while existing connections removal is still pending because
of mutex LOCK_thd_remove being held by show processlist.

Fix:
=====
In connection clean phase, thd is removed from the global
thd list first and then global connection count is
decremented. Added code to wait for connection_count be
be zero during shutdown to ensure connection threads are
done with their task.

Author: Ajo Robert

mysql-test/r/connect_debug.result

@@ -0,0 +1,49 @@
+
+# -- Bug#20201006: Spamming show processlist prevents old connection
+# -- threads from cleaning up.
+SET @saved_max_connections = @@global.max_connections;
+SET GLOBAL max_connections = 2;
+
+# -- Check that we allow only max_connections + 1 connections here
+connect  con_1, localhost, root;
+connect  con_2, localhost, root;
+connect(localhost,root,,test,MYSQL_PORT,MYSQL_SOCK);
+connect  con_3, localhost, root;
+ERROR HY000: Too many connections
+
+# -- Ensure we have max_connections + 1 connections.
+SELECT count(*)= @@global.max_connections + 1 FROM information_schema.processlist;
+count(*)= @@global.max_connections + 1
+1
+
+# -- Take LOCK_thd_remove and close one connection then
+# attempt new one [should fail]...
+SET DEBUG_SYNC='fill_schema_processlist_after_copying_threads SIGNAL disconnect_connection WAIT_FOR continue';
+SELECT user FROM INFORMATION_SCHEMA.PROCESSLIST GROUP BY user;;
+connection default;
+SET DEBUG_SYNC='now WAIT_FOR disconnect_connection';
+disconnect con_1;
+connect(localhost,root,,test,MYSQL_PORT,MYSQL_SOCK);
+connect  con_3, localhost, root;
+ERROR HY000: Too many connections
+
+# -- Release the lock. Now new connection should go through
+SET DEBUG_SYNC='now SIGNAL continue';
+connection con_2;
+user
+root
+SET DEBUG_SYNC='RESET';
+
+# -- Waiting for connection to close...
+connect  con_3, localhost, root;
+
+# -- Closing connections...
+disconnect con_3;
+disconnect con_2;
+connection default;
+
+# -- Resetting variables...
+SET GLOBAL max_connections= @saved_max_connections;
+
+# -- End of Bug#20201006.
+

mysql-test/suite/perfschema/r/dml_setup_instruments.result

@@ -36,6 +36,7 @@ where name like 'Wait/Synch/Cond/sql/%'
 'wait/synch/cond/sql/DEBUG_SYNC::cond')
 order by name limit 10;
 NAME	ENABLED	TIMED
+wait/synch/cond/sql/COND_connection_count	YES	YES
 wait/synch/cond/sql/COND_flush_thread_cache	YES	YES
 wait/synch/cond/sql/COND_manager	YES	YES
 wait/synch/cond/sql/COND_queue_state	YES	YES
@@ -45,7 +46,6 @@ wait/synch/cond/sql/COND_thread_count	YES	YES
 wait/synch/cond/sql/Delayed_insert::cond	YES	YES
 wait/synch/cond/sql/Delayed_insert::cond_client	YES	YES
 wait/synch/cond/sql/Event_scheduler::COND_state	YES	YES
-wait/synch/cond/sql/Gtid_state	YES	YES
 select * from performance_schema.setup_instruments
 where name='Wait';
 select * from performance_schema.setup_instruments

mysql-test/t/connect_debug.test

@@ -0,0 +1,80 @@
+
+# This test makes no sense with the embedded server
+--source include/not_embedded.inc
+
+--source include/have_debug_sync.inc
+
+# Save the initial number of concurrent sessions
+--source include/count_sessions.inc
+
+--echo
+--echo # -- Bug#20201006: Spamming show processlist prevents old connection
+--echo # -- threads from cleaning up.
+
+--enable_connect_log
+SET @saved_max_connections = @@global.max_connections;
+SET GLOBAL max_connections = 2;
+
+--echo
+--echo # -- Check that we allow only max_connections + 1 connections here
+--connect (con_1, localhost, root)
+--connect (con_2, localhost, root)
+--replace_result $MASTER_MYPORT MYSQL_PORT $MASTER_MYSOCK MYSQL_SOCK
+--error ER_CON_COUNT_ERROR
+--connect (con_3, localhost, root)
+
+--echo
+--echo # -- Ensure we have max_connections + 1 connections.
+SELECT count(*)= @@global.max_connections + 1 FROM information_schema.processlist;
+
+--echo
+--echo # -- Take LOCK_thd_remove and close one connection then
+--echo # attempt new one [should fail]...
+SET DEBUG_SYNC='fill_schema_processlist_after_copying_threads SIGNAL disconnect_connection WAIT_FOR continue';
+--send SELECT user FROM INFORMATION_SCHEMA.PROCESSLIST GROUP BY user;
+
+--connection default
+SET DEBUG_SYNC='now WAIT_FOR disconnect_connection';
+--disconnect con_1
+
+--replace_result $MASTER_MYPORT MYSQL_PORT $MASTER_MYSOCK MYSQL_SOCK
+--error ER_CON_COUNT_ERROR
+--connect (con_3, localhost, root)
+
+--echo
+--echo # -- Release the lock. Now new connection should go through
+SET DEBUG_SYNC='now SIGNAL continue';
+--connection con_2
+reap;
+
+SET DEBUG_SYNC='RESET';
+
+--echo
+--echo # -- Waiting for connection to close...
+let $wait_condition =
+  SELECT COUNT(*) = 2
+  FROM information_schema.processlist;
+--source include/wait_condition.inc
+
+--connect (con_3, localhost, root)
+
+--echo
+--echo # -- Closing connections...
+--disconnect con_3
+--disconnect con_2
+--source include/wait_until_disconnected.inc
+
+--connection default
+
+--echo
+--echo # -- Resetting variables...
+SET GLOBAL max_connections= @saved_max_connections;
+
+--disable_connect_log
+
+--echo
+--echo # -- End of Bug#20201006.
+--echo
+
+# Wait till all disconnects are completed
+--source include/wait_until_count_sessions.inc

sql/mysqld.cc

@@ -1265,6 +1265,7 @@ struct st_VioSSLFd *ssl_acceptor_fd;
   LOCK_connection_count.
 */
 uint connection_count= 0;
+mysql_cond_t COND_connection_count;
 
 /* Function declarations */
 
@@ -1544,6 +1545,17 @@ static void close_connections(void)
   }
   mysql_mutex_unlock(&LOCK_thread_count);
 
+  /*
+    Connection threads might take a little while to go down after removing from
+    global thread list. Give it some time.
+  */
+  mysql_mutex_lock(&LOCK_connection_count);
+  while (connection_count > 0)
+  {
+    mysql_cond_wait(&COND_connection_count, &LOCK_connection_count);
+  }
+  mysql_mutex_unlock(&LOCK_connection_count);
+
   close_active_mi();
   DBUG_PRINT("quit",("close_connections thread"));
   DBUG_VOID_RETURN;
@@ -2020,6 +2032,7 @@ static void clean_up_mutexes()
   mysql_cond_destroy(&COND_thread_cache);
   mysql_cond_destroy(&COND_flush_thread_cache);
   mysql_cond_destroy(&COND_manager);
+  mysql_cond_destroy(&COND_connection_count);
 }
 #endif /*EMBEDDED_LIBRARY*/
 
@@ -2642,6 +2655,8 @@ void dec_connection_count()
 {
   mysql_mutex_lock(&LOCK_connection_count);
   --connection_count;
+  if (connection_count == 0)
+    mysql_cond_signal(&COND_connection_count);
   mysql_mutex_unlock(&LOCK_connection_count);
 }
 
@@ -2749,8 +2764,8 @@ bool one_thread_per_connection_end(THD *thd, bool block_pthread)
   DBUG_PRINT("info", ("thd %p block_pthread %d", thd, (int) block_pthread));
 
   thd->release_resources();
-  dec_connection_count();
   remove_global_thread(thd);
+  dec_connection_count();
   if (kill_blocked_pthreads_flag)
   {
     // Do not block if we are about to shut down
@@ -4241,6 +4256,7 @@ static int init_thread_environment()
   mysql_rwlock_init(key_rwlock_LOCK_sys_init_slave, &LOCK_sys_init_slave);
   mysql_rwlock_init(key_rwlock_LOCK_grant, &LOCK_grant);
   mysql_cond_init(key_COND_thread_count, &COND_thread_count, NULL);
+  mysql_cond_init(key_COND_connection_count, &COND_connection_count, NULL);
   mysql_cond_init(key_COND_thread_cache, &COND_thread_cache, NULL);
   mysql_cond_init(key_COND_flush_thread_cache, &COND_flush_thread_cache, NULL);
   mysql_cond_init(key_COND_manager, &COND_manager, NULL);
@@ -6077,6 +6093,8 @@ void create_thread_to_handle_connection(THD *thd)
 
       mysql_mutex_lock(&LOCK_connection_count);
       --connection_count;
+      if (connection_count == 0)
+        mysql_cond_signal(&COND_connection_count);
       mysql_mutex_unlock(&LOCK_connection_count);
 
       statistic_increment(aborted_connects,&LOCK_status);
@@ -9569,7 +9587,8 @@ PSI_cond_key key_BINLOG_update_cond,
   key_relay_log_info_sleep_cond, key_cond_slave_parallel_pend_jobs,
   key_cond_slave_parallel_worker,
   key_TABLE_SHARE_cond, key_user_level_lock_cond,
-  key_COND_thread_count, key_COND_thread_cache, key_COND_flush_thread_cache;
+  key_COND_thread_count, key_COND_thread_cache, key_COND_flush_thread_cache,
+  key_COND_connection_count;
 PSI_cond_key key_RELAYLOG_update_cond;
 PSI_cond_key key_BINLOG_COND_done;
 PSI_cond_key key_RELAYLOG_COND_done;
@@ -9615,7 +9634,8 @@ static PSI_cond_info all_server_conds[]=
   { &key_COND_thread_count, "COND_thread_count", PSI_FLAG_GLOBAL},
   { &key_COND_thread_cache, "COND_thread_cache", PSI_FLAG_GLOBAL},
   { &key_COND_flush_thread_cache, "COND_flush_thread_cache", PSI_FLAG_GLOBAL},
-  { &key_gtid_ensure_index_cond, "Gtid_state", PSI_FLAG_GLOBAL}
+  { &key_gtid_ensure_index_cond, "Gtid_state", PSI_FLAG_GLOBAL},
+  { &key_COND_connection_count, "COND_connection_count", PSI_FLAG_GLOBAL}
 };
 
 PSI_thread_key key_thread_bootstrap, key_thread_delayed_insert,

sql/mysqld.h

@@ -383,7 +383,8 @@ extern PSI_cond_key key_BINLOG_update_cond,
   key_relay_log_info_sleep_cond, key_cond_slave_parallel_pend_jobs,
   key_cond_slave_parallel_worker,
   key_TABLE_SHARE_cond, key_user_level_lock_cond,
-  key_COND_thread_count, key_COND_thread_cache, key_COND_flush_thread_cache;
+  key_COND_thread_count, key_COND_thread_cache, key_COND_flush_thread_cache,
+  key_COND_connection_count;
 extern PSI_cond_key key_BINLOG_COND_done;
 extern PSI_cond_key key_RELAYLOG_COND_done;
 extern PSI_cond_key key_RELAYLOG_update_cond;

sql/scheduler.cc

@@ -34,8 +34,8 @@
 static bool no_threads_end(THD *thd, bool put_in_cache)
 {
   thd_release_resources(thd);
-  dec_connection_count();
   remove_global_thread(thd);
+  dec_connection_count();
   // THD is an incomplete type here, so use destroy_thd() to delete it.
   destroy_thd(thd);
 

sql/sql_show.cc

@@ -2222,6 +2222,8 @@ int fill_schema_processlist(THD* thd, TABLE_LIST* tables, Item* cond)
     mysql_mutex_lock(&LOCK_thd_remove);
     copy_global_thread_list(&global_thread_list_copy);
 
+    DEBUG_SYNC(thd,"fill_schema_processlist_after_copying_threads");
+
     Thread_iterator it= global_thread_list_copy.begin();
     Thread_iterator end= global_thread_list_copy.end();
     for (; it != end; ++it)