List:Commits« Previous MessageNext Message »
From:Alexander Nozdrin Date:March 12 2008 2:44pm
Subject:bk commit into 5.1 tree (anozdrin:1.2561) BUG#33507
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of anozdrin.  When anozdrin does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-03-12 17:44:40+03:00, anozdrin@quad. +5 -0
  Fix for Bug#33507: Event scheduler creates more threads
  than max_connections -- which results in user lockout.
  
  The problem was that the variable thread_count that contains
  the number of active threads was interpreted as a number of
  active connections.
  
  The fix is to introduce a new counter for active connections.

  mysql-test/r/connect.result@stripped, 2008-03-12 17:44:39+03:00, anozdrin@quad. +58 -0
    A test case for Bug#33507: Event scheduler creates more threads
    than max_connections -- which results in user lockout.

  mysql-test/t/connect.test@stripped, 2008-03-12 17:44:39+03:00, anozdrin@quad. +126 -1
    A test case for Bug#33507: Event scheduler creates more threads
    than max_connections -- which results in user lockout.

  sql/mysql_priv.h@stripped, 2008-03-12 17:44:39+03:00, anozdrin@quad. +2 -3
    1. Polishing: login_connection() and end_connection() need not
       to be public.
    
    2. Introduce connection_count -- a variable to contain the number
       of active connections. It is protected by LOCK_connection_count. 

  sql/mysqld.cc@stripped, 2008-03-12 17:44:39+03:00, anozdrin@quad. +38 -4
    Use connection_count to count active connections.

  sql/sql_connect.cc@stripped, 2008-03-12 17:44:39+03:00, anozdrin@quad. +7 -6
    1. Use connection_count to count active connections.
    2. Make login_connection(), end_connection() private for the module
    as they had to be.

diff -Nrup a/mysql-test/r/connect.result b/mysql-test/r/connect.result
--- a/mysql-test/r/connect.result	2006-12-09 06:27:52 +03:00
+++ b/mysql-test/r/connect.result	2008-03-12 17:44:39 +03:00
@@ -115,3 +115,61 @@ create temporary table t2(id integer not
 set @id := 1;
 delete from t1 where id like @id;
 drop table t1;
+# ------------------------------------------------------------------
+# -- End of 4.1 tests
+# ------------------------------------------------------------------
+
+# -- Bug#33507: Event scheduler creates more threads than max_connections
+# -- which results in user lockout.
+
+GRANT USAGE ON *.* TO mysqltest_u1@localhost;
+
+SET GLOBAL max_connections = 3;
+SET GLOBAL event_scheduler = ON;
+
+# -- Waiting for old connections to close...
+
+
+# -- Disconnecting default connection...
+
+# -- Check that we allow exactly three user connections, no matter how
+# -- many threads are running.
+
+# -- Connecting (1)...
+
+# -- Waiting for root connection to close...
+
+# -- Connecting (2)...
+# -- Connecting (3)...
+# -- Connecting (4)...
+ERROR 08004: Too many connections
+
+# -- Waiting for the last connection to close...
+
+# -- Check that we allow one extra SUPER-user connection.
+
+# -- Connecting super (1)...
+# -- Connecting super (2)...
+ERROR 00000: Too many connections
+
+SELECT user FROM information_schema.processlist ORDER BY id;
+user
+event_scheduler
+mysqltest_u1
+mysqltest_u1
+mysqltest_u1
+root
+
+# -- Resetting variables...
+SET GLOBAL max_connections = 151;
+SET GLOBAL event_scheduler = OFF;
+
+# -- That's it. Closing connections...
+
+# -- Restoring default connection...
+
+# -- End of Bug#33507.
+
+# ------------------------------------------------------------------
+# -- End of 5.1 tests
+# ------------------------------------------------------------------
diff -Nrup a/mysql-test/t/connect.test b/mysql-test/t/connect.test
--- a/mysql-test/t/connect.test	2006-10-04 18:33:24 +04:00
+++ b/mysql-test/t/connect.test	2008-03-12 17:44:39 +03:00
@@ -102,4 +102,129 @@ disconnect con7;
 connection default;
 drop table t1;
 
-# End of 4.1 tests
+--disconnect con1
+--disconnect con2
+--disconnect con3
+--disconnect con4
+--disconnect con5
+--disconnect con6
+--disconnect con10
+
+--echo # ------------------------------------------------------------------
+--echo # -- End of 4.1 tests
+--echo # ------------------------------------------------------------------
+
+--echo
+--echo # -- Bug#33507: Event scheduler creates more threads than max_connections
+--echo # -- which results in user lockout.
+--echo
+
+GRANT USAGE ON *.* TO mysqltest_u1@localhost;
+
+# NOTE: if the test case fails sporadically due to spurious connections,
+# consider disabling all users.
+
+--echo
+
+let $saved_max_connections = `SELECT @@global.max_connections`;
+
+SET GLOBAL max_connections = 3;
+SET GLOBAL event_scheduler = ON;
+
+--echo
+--echo # -- Waiting for old connections to close...
+let $wait_condition =
+  SELECT COUNT(*) = 1
+  FROM information_schema.processlist
+  WHERE db = 'test';
+--source include/wait_condition.inc
+
+--echo
+let $wait_condition =
+  SELECT COUNT(*) = 1
+  FROM information_schema.processlist
+  WHERE user = 'event_scheduler';
+--source include/wait_condition.inc
+--echo
+
+--echo # -- Disconnecting default connection...
+--disconnect default
+
+--echo
+--echo # -- Check that we allow exactly three user connections, no matter how
+--echo # -- many threads are running.
+--echo 
+
+--echo # -- Connecting (1)...
+--connect (con_1,localhost,mysqltest_u1)
+
+--echo
+--echo # -- Waiting for root connection to close...
+let $wait_condition =
+  SELECT COUNT(*) = 1
+  FROM information_schema.processlist
+  WHERE db = 'test';
+--source include/wait_condition.inc
+--echo
+
+--echo # -- Connecting (2)...
+--connect (con_2,localhost,mysqltest_u1)
+
+--echo # -- Connecting (3)...
+--connect (con_3,localhost,mysqltest_u1)
+
+--echo # -- Connecting (4)...
+--disable_query_log
+--error ER_CON_COUNT_ERROR
+--connect (con_4,localhost,mysqltest_u1)
+--enable_query_log
+
+--echo
+--echo # -- Waiting for the last connection to close...
+let $wait_condition =
+  SELECT COUNT(*) = 3
+  FROM information_schema.processlist
+  WHERE db = 'test';
+--source include/wait_condition.inc
+
+--echo
+--echo # -- Check that we allow one extra SUPER-user connection.
+--echo
+
+--echo # -- Connecting super (1)...
+--connect (con_super_1,localhost,root)
+
+--echo # -- Connecting super (2)...
+--disable_query_log
+--error ER_CON_COUNT_ERROR
+--connect (con_super_2,localhost,root)
+--enable_query_log
+
+--echo
+# Ensure that we have Event Scheduler thread, 3 ordinary user connections and
+# one extra super-user connection.
+SELECT user FROM information_schema.processlist ORDER BY id;
+
+--echo
+--echo # -- Resetting variables...
+
+--eval SET GLOBAL max_connections = $saved_max_connections
+SET GLOBAL event_scheduler = OFF;
+
+--echo
+--echo # -- That's it. Closing connections...
+--disconnect con_1
+--disconnect con_2
+--disconnect con_super_1
+
+--echo
+--echo # -- Restoring default connection...
+--connect (default,localhost,root,,test)
+
+--echo
+--echo # -- End of Bug#33507.
+--echo
+
+--echo # ------------------------------------------------------------------
+--echo # -- End of 5.1 tests
+--echo # ------------------------------------------------------------------
diff -Nrup a/sql/mysql_priv.h b/sql/mysql_priv.h
--- a/sql/mysql_priv.h	2008-02-26 19:26:40 +03:00
+++ b/sql/mysql_priv.h	2008-03-12 17:44:39 +03:00
@@ -974,8 +974,6 @@ void time_out_user_resource_limits(THD *
 void decrease_user_connections(USER_CONN *uc);
 void thd_init_client_charset(THD *thd, uint cs_number);
 bool setup_connection_thread_globals(THD *thd);
-bool login_connection(THD *thd);
-void end_connection(THD *thd);
 
 bool mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create, bool silent);
 bool mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create);
@@ -1895,6 +1893,7 @@ extern bool opt_disable_networking, opt_
 extern my_bool opt_character_set_client_handshake;
 extern bool volatile abort_loop, shutdown_in_progress;
 extern uint volatile thread_count, thread_running, global_read_lock;
+extern uint connection_count;
 extern my_bool opt_sql_bin_update, opt_safe_user_create, opt_no_mix_types;
 extern my_bool opt_safe_show_db, opt_local_infile, opt_myisam_use_mmap;
 extern my_bool opt_slave_compressed_protocol, use_temp_pool;
@@ -1933,7 +1932,7 @@ extern pthread_mutex_t LOCK_mysql_create
        LOCK_slave_list, LOCK_active_mi, LOCK_manager, LOCK_global_read_lock,
        LOCK_global_system_variables, LOCK_user_conn,
        LOCK_prepared_stmt_count,
-       LOCK_bytes_sent, LOCK_bytes_received;
+       LOCK_bytes_sent, LOCK_bytes_received, LOCK_connection_count;
 #ifdef HAVE_OPENSSL
 extern pthread_mutex_t LOCK_des_key_file;
 #endif
diff -Nrup a/sql/mysqld.cc b/sql/mysqld.cc
--- a/sql/mysqld.cc	2008-02-26 19:26:40 +03:00
+++ b/sql/mysqld.cc	2008-03-12 17:44:39 +03:00
@@ -584,7 +584,8 @@ pthread_mutex_t LOCK_mysql_create_db, LO
 		LOCK_delayed_insert, LOCK_delayed_status, LOCK_delayed_create,
 		LOCK_crypt, LOCK_bytes_sent, LOCK_bytes_received,
 	        LOCK_global_system_variables,
-		LOCK_user_conn, LOCK_slave_list, LOCK_active_mi;
+		LOCK_user_conn, LOCK_slave_list, LOCK_active_mi,
+                LOCK_connection_count;
 /**
   The below lock protects access to two global server variables:
   max_prepared_stmt_count and prepared_stmt_count. These variables
@@ -720,6 +721,11 @@ char *des_key_file;
 struct st_VioSSLFd *ssl_acceptor_fd;
 #endif /* HAVE_OPENSSL */
 
+/**
+  Number of currently active user connections. The variable is protected by
+  LOCK_connection_count.
+*/
+uint connection_count= 0;
 
 /* Function declarations */
 
@@ -1341,6 +1347,7 @@ static void clean_up_mutexes()
   (void) pthread_mutex_destroy(&LOCK_bytes_sent);
   (void) pthread_mutex_destroy(&LOCK_bytes_received);
   (void) pthread_mutex_destroy(&LOCK_user_conn);
+  (void) pthread_mutex_destroy(&LOCK_connection_count);
   Events::destroy_mutexes();
 #ifdef HAVE_OPENSSL
   (void) pthread_mutex_destroy(&LOCK_des_key_file);
@@ -1783,6 +1790,11 @@ void unlink_thd(THD *thd)
   DBUG_ENTER("unlink_thd");
   DBUG_PRINT("enter", ("thd: 0x%lx", (long) thd));
   thd->cleanup();
+
+  pthread_mutex_lock(&LOCK_connection_count);
+  --connection_count;
+  pthread_mutex_unlock(&LOCK_connection_count);
+
   (void) pthread_mutex_lock(&LOCK_thread_count);
   thread_count--;
   delete thd;
@@ -3453,6 +3465,7 @@ static int init_thread_environment()
   (void) pthread_mutex_init(&LOCK_global_read_lock, MY_MUTEX_INIT_FAST);
   (void) pthread_mutex_init(&LOCK_prepared_stmt_count, MY_MUTEX_INIT_FAST);
   (void) pthread_mutex_init(&LOCK_uuid_generator, MY_MUTEX_INIT_FAST);
+  (void) pthread_mutex_init(&LOCK_connection_count, MY_MUTEX_INIT_FAST);
 #ifdef HAVE_OPENSSL
   (void) pthread_mutex_init(&LOCK_des_key_file,MY_MUTEX_INIT_FAST);
 #ifndef HAVE_YASSL
@@ -4700,6 +4713,11 @@ void create_thread_to_handle_connection(
       thread_count--;
       thd->killed= THD::KILL_CONNECTION;			// Safety
       (void) pthread_mutex_unlock(&LOCK_thread_count);
+
+      pthread_mutex_lock(&LOCK_connection_count);
+      --connection_count;
+      pthread_mutex_unlock(&LOCK_connection_count);
+
       statistic_increment(aborted_connects,&LOCK_status);
       /* Can't use my_error() since store_globals has not been called. */
       my_snprintf(error_message_buff, sizeof(error_message_buff),
@@ -4739,15 +4757,31 @@ static void create_new_thread(THD *thd)
   if (protocol_version > 9)
     net->return_errno=1;
 
-  /* don't allow too many connections */
-  if (thread_count - delayed_insert_threads >= max_connections+1 || abort_loop)
+  /*
+    Don't allow too many connections. We roughly check here that we allow
+    only (max_connections + 1) connections.
+  */
+
+  pthread_mutex_lock(&LOCK_connection_count);
+
+  if (connection_count >= max_connections + 1 || abort_loop)
   {
+    pthread_mutex_unlock(&LOCK_connection_count);
+
     DBUG_PRINT("error",("Too many connections"));
     close_connection(thd, ER_CON_COUNT_ERROR, 1);
     delete thd;
     DBUG_VOID_RETURN;
   }
+
+  ++connection_count;
+
+  pthread_mutex_unlock(&LOCK_connection_count);
+
+  /* Start a new thread to handle connection. */
+
   pthread_mutex_lock(&LOCK_thread_count);
+
   /*
     The initialization of thread_id is done in create_embedded_thd() for
     the embedded library.
@@ -4755,13 +4789,13 @@ static void create_new_thread(THD *thd)
   */
   thd->thread_id= thd->variables.pseudo_thread_id= thread_id++;
 
-  /* Start a new thread to handle connection */
   thread_count++;
 
   if (thread_count - delayed_insert_threads > max_used_connections)
     max_used_connections= thread_count - delayed_insert_threads;
 
   thread_scheduler.add_connection(thd);
+
   DBUG_VOID_RETURN;
 }
 #endif /* EMBEDDED_LIBRARY */
diff -Nrup a/sql/sql_connect.cc b/sql/sql_connect.cc
--- a/sql/sql_connect.cc	2008-02-26 19:26:41 +03:00
+++ b/sql/sql_connect.cc	2008-03-12 17:44:39 +03:00
@@ -402,10 +402,11 @@ check_user(THD *thd, enum enum_server_co
 
       if (check_count)
       {
-        VOID(pthread_mutex_lock(&LOCK_thread_count));
-        bool count_ok= thread_count <= max_connections + delayed_insert_threads
-                       || (thd->main_security_ctx.master_access & SUPER_ACL);
-        VOID(pthread_mutex_unlock(&LOCK_thread_count));
+        pthread_mutex_lock(&LOCK_connection_count);
+        bool count_ok= connection_count <= max_connections ||
+                       (thd->main_security_ctx.master_access & SUPER_ACL);
+        VOID(pthread_mutex_unlock(&LOCK_connection_count));
+
         if (!count_ok)
         {                                         // too many connections
           my_error(ER_CON_COUNT_ERROR, MYF(0));
@@ -930,7 +931,7 @@ bool setup_connection_thread_globals(THD
 */
 
 
-bool login_connection(THD *thd)
+static bool login_connection(THD *thd)
 {
   NET *net= &thd->net;
   int error;
@@ -968,7 +969,7 @@ bool login_connection(THD *thd)
     This mainly updates status variables
 */
 
-void end_connection(THD *thd)
+static void end_connection(THD *thd)
 {
   NET *net= &thd->net;
   plugin_thdvar_cleanup(thd);
Thread
bk commit into 5.1 tree (anozdrin:1.2561) BUG#33507Alexander Nozdrin12 Mar