Description:
We have some daemon program which issue "show global status" and "show processlist" command periodically to capture some perf statistics of MySQL instance, when the instance have ten's thousands of connections, we see heavy contention of LOCK_status, and it may stuck the whole instance.
When processing "show global status" command, we see the following lock logic in calc_sum_of_all_status function
mysql_mutex_lock(&LOCK_status);
do_for_all_thd_copy(Add_status);
mysql_mutex_unlock(&LOCK_status);
The better way should be
do_for_all_thd_copy
mysql_mutex_lock(&LOCK_status);
Add_status(thd);
mysql_mutex_unlock(&LOCK_status);
How to repeat:
Benchmark with thousands of short connections, and run "show global status" and "show processlist" periodically.
Suggested fix:
diff -rc mysql-8.0.31/sql/mysqld.cc mysql-8.0.31-patched/sql/mysqld.cc
*** mysql-8.0.31/sql/mysqld.cc 2022-09-13 12:15:16.000000000 -0400
--- mysql-8.0.31-patched/sql/mysqld.cc 2022-12-08 13:11:32.507359809 -0500
***************
*** 11600,11609 ****
--- 11600,11611 ----
Reset_thd_status() = default;
void operator()(THD *thd) override {
/* Update the global status if not done so already. */
+ mysql_mutex_lock(&LOCK_status);
if (!thd->status_var_aggregated) {
add_to_status(&global_status_var, &thd->status_var);
}
reset_system_status_vars(&thd->status_var);
+ mysql_mutex_unlock(&LOCK_status);
}
};
***************
*** 11611,11621 ****
Reset global and session status variables.
*/
void refresh_status() {
- mysql_mutex_lock(&LOCK_status);
-
/* For all threads, add status to global status and then reset. */
Reset_thd_status reset_thd_status;
Global_THD_manager::get_instance()->do_for_all_thd_copy(&reset_thd_status);
#ifdef WITH_PERFSCHEMA_STORAGE_ENGINE
/* Reset aggregated status counters. */
reset_pfs_status_stats();
--- 11613,11623 ----
Reset global and session status variables.
*/
void refresh_status() {
/* For all threads, add status to global status and then reset. */
Reset_thd_status reset_thd_status;
Global_THD_manager::get_instance()->do_for_all_thd_copy(&reset_thd_status);
+
+ mysql_mutex_lock(&LOCK_status);
#ifdef WITH_PERFSCHEMA_STORAGE_ENGINE
/* Reset aggregated status counters. */
reset_pfs_status_stats();
diff -rc mysql-8.0.31/sql/sql_parse.cc mysql-8.0.31-patched/sql/sql_parse.cc
*** mysql-8.0.31/sql/sql_parse.cc 2022-09-13 12:15:16.000000000 -0400
--- mysql-8.0.31-patched/sql/sql_parse.cc 2022-12-08 13:12:42.808232095 -0500
***************
*** 2308,2316 ****
query_logger.general_log_print(thd, command, NullS);
thd->status_var.com_stat[SQLCOM_SHOW_STATUS]++;
- mysql_mutex_lock(&LOCK_status);
calc_sum_of_all_status(¤t_global_status_var);
- mysql_mutex_unlock(&LOCK_status);
if (!(uptime = (ulong)(thd->query_start_in_secs() - server_start_time)))
queries_per_second1000 = 0;
else
--- 2308,2314 ----
diff -rc mysql-8.0.31/sql/sql_show.cc mysql-8.0.31-patched/sql/sql_show.cc
*** mysql-8.0.31/sql/sql_show.cc 2022-09-13 12:15:16.000000000 -0400
--- mysql-8.0.31-patched/sql/sql_show.cc 2022-12-08 13:14:13.193353588 -0500
***************
*** 3618,3625 ****
--- 3618,3627 ----
public:
Add_status(System_status_var *value) : m_stat_var(value) {}
void operator()(THD *thd) override {
+ mysql_mutex_lock(&LOCK_status);
if (!thd->status_var_aggregated)
add_to_status(m_stat_var, &thd->status_var);
+ mysql_mutex_unlock(&LOCK_status);
}
private:
***************
*** 3629,3635 ****
void calc_sum_of_all_status(System_status_var *to) {
DBUG_TRACE;
- mysql_mutex_assert_owner(&LOCK_status);
/* Get global values as base. */
*to = global_status_var;
Add_status add_status(to);
--- 3631,3636 ----
Description: We have some daemon program which issue "show global status" and "show processlist" command periodically to capture some perf statistics of MySQL instance, when the instance have ten's thousands of connections, we see heavy contention of LOCK_status, and it may stuck the whole instance. When processing "show global status" command, we see the following lock logic in calc_sum_of_all_status function mysql_mutex_lock(&LOCK_status); do_for_all_thd_copy(Add_status); mysql_mutex_unlock(&LOCK_status); The better way should be do_for_all_thd_copy mysql_mutex_lock(&LOCK_status); Add_status(thd); mysql_mutex_unlock(&LOCK_status); How to repeat: Benchmark with thousands of short connections, and run "show global status" and "show processlist" periodically. Suggested fix: diff -rc mysql-8.0.31/sql/mysqld.cc mysql-8.0.31-patched/sql/mysqld.cc *** mysql-8.0.31/sql/mysqld.cc 2022-09-13 12:15:16.000000000 -0400 --- mysql-8.0.31-patched/sql/mysqld.cc 2022-12-08 13:11:32.507359809 -0500 *************** *** 11600,11609 **** --- 11600,11611 ---- Reset_thd_status() = default; void operator()(THD *thd) override { /* Update the global status if not done so already. */ + mysql_mutex_lock(&LOCK_status); if (!thd->status_var_aggregated) { add_to_status(&global_status_var, &thd->status_var); } reset_system_status_vars(&thd->status_var); + mysql_mutex_unlock(&LOCK_status); } }; *************** *** 11611,11621 **** Reset global and session status variables. */ void refresh_status() { - mysql_mutex_lock(&LOCK_status); - /* For all threads, add status to global status and then reset. */ Reset_thd_status reset_thd_status; Global_THD_manager::get_instance()->do_for_all_thd_copy(&reset_thd_status); #ifdef WITH_PERFSCHEMA_STORAGE_ENGINE /* Reset aggregated status counters. */ reset_pfs_status_stats(); --- 11613,11623 ---- Reset global and session status variables. */ void refresh_status() { /* For all threads, add status to global status and then reset. */ Reset_thd_status reset_thd_status; Global_THD_manager::get_instance()->do_for_all_thd_copy(&reset_thd_status); + + mysql_mutex_lock(&LOCK_status); #ifdef WITH_PERFSCHEMA_STORAGE_ENGINE /* Reset aggregated status counters. */ reset_pfs_status_stats(); diff -rc mysql-8.0.31/sql/sql_parse.cc mysql-8.0.31-patched/sql/sql_parse.cc *** mysql-8.0.31/sql/sql_parse.cc 2022-09-13 12:15:16.000000000 -0400 --- mysql-8.0.31-patched/sql/sql_parse.cc 2022-12-08 13:12:42.808232095 -0500 *************** *** 2308,2316 **** query_logger.general_log_print(thd, command, NullS); thd->status_var.com_stat[SQLCOM_SHOW_STATUS]++; - mysql_mutex_lock(&LOCK_status); calc_sum_of_all_status(¤t_global_status_var); - mysql_mutex_unlock(&LOCK_status); if (!(uptime = (ulong)(thd->query_start_in_secs() - server_start_time))) queries_per_second1000 = 0; else --- 2308,2314 ---- diff -rc mysql-8.0.31/sql/sql_show.cc mysql-8.0.31-patched/sql/sql_show.cc *** mysql-8.0.31/sql/sql_show.cc 2022-09-13 12:15:16.000000000 -0400 --- mysql-8.0.31-patched/sql/sql_show.cc 2022-12-08 13:14:13.193353588 -0500 *************** *** 3618,3625 **** --- 3618,3627 ---- public: Add_status(System_status_var *value) : m_stat_var(value) {} void operator()(THD *thd) override { + mysql_mutex_lock(&LOCK_status); if (!thd->status_var_aggregated) add_to_status(m_stat_var, &thd->status_var); + mysql_mutex_unlock(&LOCK_status); } private: *************** *** 3629,3635 **** void calc_sum_of_all_status(System_status_var *to) { DBUG_TRACE; - mysql_mutex_assert_owner(&LOCK_status); /* Get global values as base. */ *to = global_status_var; Add_status add_status(to); --- 3631,3636 ----