change method of running thread count

sql/event_scheduler.cc

@@ -170,7 +170,6 @@ bool post_init_event_thread(THD *thd) {
 
   Global_THD_manager *thd_manager = Global_THD_manager::get_instance();
   thd_manager->add_thd(thd);
-  thd_manager->inc_thread_running();
   return false;
 }
 
@@ -190,7 +189,6 @@ void deinit_event_thread(THD *thd) {
   DBUG_PRINT("exit", ("Event thread finishing"));
   thd->release_resources();
   thd_manager->remove_thd(thd);
-  thd_manager->dec_thread_running();
   delete thd;
 }
 

sql/mysqld_thd_manager.cc

@@ -315,6 +315,31 @@ THD *Global_THD_manager::find_thd(Find_thd_with_id *func) {
   return nullptr;
 }
 
+/**
+  This class implements callback for do_for_all_thd().
+  It counts the total number of running threads
+  from global thread list.
+*/
+class Count_thread_running : public Do_THD_Impl {
+ public:
+  Count_thread_running() : m_count(0) {}
+  virtual void operator()(THD *thd) {
+    if (thd->get_command() != COM_SLEEP) {
+      m_count++;
+    }
+  }
+  int get_count() { return m_count; }
+
+ private:
+  int m_count;
+};
+
+void Global_THD_manager::count_num_thread_running() {
+  Count_thread_running count_thread_running;
+  do_for_all_thd(&count_thread_running);
+  atomic_num_thread_running = count_thread_running.get_count();
+}
+
 void inc_thread_created() {
   Global_THD_manager::get_instance()->inc_thread_created();
 }

sql/mysqld_thd_manager.h

@@ -148,20 +148,15 @@ class Global_THD_manager {
   void remove_thd(THD *thd);
 
   /**
-    Retrieves thread running statistic variable.
-    @return int Returns the total number of threads currently running
+    Count thread running statistic variable.
   */
-  int get_num_thread_running() const { return atomic_num_thread_running; }
+  void count_num_thread_running();
 
   /**
-    Increments thread running statistic variable.
-  */
-  void inc_thread_running() { atomic_num_thread_running++; }
-
-  /**
-    Decrements thread running statistic variable.
+    Retrieves thread running statistic variable.
+    @return int Returns the total number of threads currently running
   */
-  void dec_thread_running() { atomic_num_thread_running--; }
+  int get_num_thread_running() const { return atomic_num_thread_running; }
 
   /**
     Retrieves thread created statistic variable.

sql/sql_parse.cc

@@ -1547,7 +1547,6 @@ bool dispatch_command(THD *thd, const COM_DATA *com_data,
   }
   thd->set_query_id(next_query_id());
   thd->reset_rewritten_query();
-  thd_manager->inc_thread_running();
 
   if (!(server_command_flags[command] & CF_SKIP_QUESTIONS))
     thd->status_var.questions++;
@@ -2195,8 +2194,6 @@ bool dispatch_command(THD *thd, const COM_DATA *com_data,
   /* Prevent rewritten query from getting "stuck" in SHOW PROCESSLIST. */
   thd->reset_rewritten_query();
 
-  thd_manager->dec_thread_running();
-
   /* Freeing the memroot will leave the THD::work_part_info invalid. */
   thd->work_part_info = nullptr;
 

storage/perfschema/pfs_variable.cc

@@ -1156,6 +1156,17 @@ int PFS_status_variable_cache::do_materialize_global(void) {
                                         false, /* threads */
                                         true,  /* THDs */
                                         &visitor);
+
+  /*
+    Because of the reason described in 
+    PFS_status_variable_cache::do_materialize_all(THD *unsafe_thd),
+    PFS_status_variable_cache::do_materialize_session(THD *unsafe_thd) and
+    PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread),
+    count_num_thread_running() cannot put together with
+    get_num_thread_running(), so count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
   /*
     Build the status variable cache using the SHOW_VAR array as a reference.
     Use the status totals collected from all threads.
@@ -1199,6 +1210,22 @@ int PFS_status_variable_cache::do_materialize_all(THD *unsafe_thd) {
     init_show_var_array(OPT_SESSION, false);
   }
 
+  /*
+    count_num_thread_running() counts the total number of running threads
+    from global thread list, using LOCK_thd_list to protect sharded
+    global thread list. In lock_order_dependencies.txt, the lock order
+    is that LOCK_thd_list must be locked before LOCK_thd_data. In this
+    function, LOCK_thd_data is already locked in get_THD(), Then manifest()
+    will call get_num_thread_running(). If get_num_thread_running() counts
+    and returns the num, the lock order will be incorrect, which may
+    lead to dead lock. To prevent this situation, get_num_thread_running()
+    is split into two part, one is still called get_num_thread_running()
+    which returns the num, the other is called count_num_thread_running()
+    which counts the num and should be called before get_THD() and
+    get_num_thread_running(). So count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
   /* Get and lock a validated THD from the thread manager. */
   if ((m_safe_thd = get_THD(unsafe_thd)) != nullptr) {
     /*
@@ -1248,6 +1275,22 @@ int PFS_status_variable_cache::do_materialize_session(THD *unsafe_thd) {
     init_show_var_array(OPT_SESSION, true);
   }
 
+  /*
+    count_num_thread_running() counts the total number of running threads
+    from global thread list, using LOCK_thd_list to protect sharded
+    global thread list. In lock_order_dependencies.txt, the lock order
+    is that LOCK_thd_list must be locked before LOCK_thd_data. In this
+    function, LOCK_thd_data is already locked in get_THD(), Then manifest()
+    will call get_num_thread_running(). If get_num_thread_running() counts
+    and returns the num, the lock order will be incorrect, which may
+    lead to dead lock. To prevent this situation, get_num_thread_running()
+    is split into two part, one is still called get_num_thread_running()
+    which returns the num, the other is called count_num_thread_running()
+    which counts the num and should be called before get_THD() and
+    get_num_thread_running(). So count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
   /* Get and lock a validated THD from the thread manager. */
   if ((m_safe_thd = get_THD(unsafe_thd)) != nullptr) {
     /*
@@ -1291,6 +1334,22 @@ int PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread) {
   /* The SHOW_VAR array must be initialized externally. */
   DBUG_ASSERT(m_initialized);
 
+  /*
+    count_num_thread_running() counts the total number of running threads
+    from global thread list, using LOCK_thd_list to protect sharded
+    global thread list. In lock_order_dependencies.txt, the lock order
+    is that LOCK_thd_list must be locked before LOCK_thd_data. In this
+    function, LOCK_thd_data is already locked in get_THD(), Then manifest()
+    will call get_num_thread_running(). If get_num_thread_running() counts
+    and returns the num, the lock order will be incorrect, which may
+    lead to dead lock. To prevent this situation, get_num_thread_running()
+    is split into two part, one is still called get_num_thread_running()
+    which returns the num, the other is called count_num_thread_running()
+    which counts the num and should be called before get_THD() and
+    get_num_thread_running(). So count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
   /* Get and lock a validated THD from the thread manager. */
   if ((m_safe_thd = get_THD(pfs_thread)) != nullptr) {
     /*
@@ -1342,6 +1401,16 @@ int PFS_status_variable_cache::do_materialize_client(PFS_client *pfs_client) {
   */
   m_sum_client_status(pfs_client, &status_totals);
 
+  /*
+    Because of the reason described in 
+    PFS_status_variable_cache::do_materialize_all(THD *unsafe_thd),
+    PFS_status_variable_cache::do_materialize_session(THD *unsafe_thd) and
+    PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread),
+    count_num_thread_running() cannot put together with
+    get_num_thread_running(), so count_num_thread_running() is put here.
+  */
+  Global_THD_manager::get_instance()->count_num_thread_running();
+
   /*
     Build the status variable cache using the SHOW_VAR array as a reference and
     the status totals collected from threads associated with this client.

unittest/gunit/thd_manager-t.cc

@@ -82,14 +82,6 @@ TEST_F(ThreadManagerTest, AddRemoveTHDWithGuard) {
   EXPECT_EQ(0U, thd_manager->get_thd_count());
 }
 
-TEST_F(ThreadManagerTest, IncDecThreadRunning) {
-  EXPECT_EQ(0, thd_manager->get_num_thread_running());
-  thd_manager->inc_thread_running();
-  EXPECT_EQ(1, thd_manager->get_num_thread_running());
-  thd_manager->dec_thread_running();
-  EXPECT_EQ(0, thd_manager->get_num_thread_running());
-}
-
 TEST_F(ThreadManagerTest, IncThreadCreated) {
   EXPECT_EQ(0U, thd_manager->get_num_thread_created());
   thd_manager->inc_thread_created();