Perform thd_status aggregation using mdl mutex

Summary: We have seen that there is high contention and stalls during status aggregation. The mutex used over there does not work well with scheduler. Rather, we want a mutex which allows the waiters to yield the scheduler when waiting for the lock and get notified of the lock when available.

Differential Revision: D63607467

fbshipit-source-id: 7d5b7e06b344271619522f73aa2d196d0a19ef7a

mysql-test/r/mysqld--help-notwin.result

@@ -3270,6 +3270,10 @@ The following options may be given as the first argument:
  mutex where instrumented. If disabled, the regular mutex
  is used.
  (Defaults to on; use --skip-use-mdl-mutex to disable.)
+ --use-status-mdl-mutex 
+ Use thread pool friendly MDL lock instead of regular
+ mutex where instrumented for status variables. If
+ disabled, the regular mutex is used.
  -u, --user=name     Run mysqld daemon as user.
  --validate-config   Validate the server configuration specified by the user.
  --validate-schema-from-attributes 
@@ -4282,6 +4286,7 @@ update-table-create-timestamp-on-truncate FALSE
 upgrade AUTO
 use-fb-json-functions 
 use-mdl-mutex TRUE
+use-status-mdl-mutex FALSE
 validate-config FALSE
 validate-schema-from-attributes FALSE
 validate-user-plugins TRUE

mysql-test/suite/sys_vars/r/all_vars.result

@@ -273,6 +273,8 @@ update_binlog_pos_threshold
 update_table_create_timestamp_on_truncate
 update_table_create_timestamp_on_truncate
 use_secondary_engine
+use_status_mdl_mutex
+use_status_mdl_mutex
 wait_for_hlc_sleep_scaling_factor
 wait_for_hlc_sleep_scaling_factor
 wait_for_hlc_sleep_threshold_ms

mysql-test/t/all_persisted_variables.test

@@ -77,6 +77,7 @@ let $total_excluded_vars=`SELECT COUNT(*) FROM performance_schema.global_variabl
 'innodb_log_write_events',
 'innodb_sync_pool_size',
 'use_mdl_mutex',
+'use_status_mdl_mutex',
 'override_enable_raft_check',
 'performance_schema_esms_by_thread_by_event_name',
 'performance_schema_ews_by_thread_by_event_name',

sql/binlog.cc

@@ -13455,10 +13455,13 @@ bool show_raft_status(THD *thd) {
   std::vector<std::pair<std::string, std::string>> var_value_pairs;
   std::vector<std::pair<std::string, std::string>>::const_iterator itr;
 
-  mysql_mutex_lock(&LOCK_status);
-  int error = RUN_HOOK_STRICT(raft_replication, show_raft_status,
-                              (current_thd, &var_value_pairs));
-  mysql_mutex_unlock(&LOCK_status);
+  int error = 0;
+  {
+    MDL_mutex_guard guard(THD::get_mutex_thd_status_aggregation(), current_thd,
+                          &LOCK_status, use_status_mdl_mutex);
+    error = RUN_HOOK_STRICT(raft_replication, show_raft_status,
+                            (current_thd, &var_value_pairs));
+  }
   if (error) {
     errmsg = "Failure to run plugin hook";
     goto err;

sql/mdl.cc

@@ -5331,16 +5331,25 @@ void MDL_mutex::unlock(THD *thd, MDL_ticket *ticket) {
   Guard constructor locks the mutex.
 */
 MDL_mutex_guard::MDL_mutex_guard(MDL_mutex *mdl_mutex, THD *thd,
-                                 mysql_mutex_t *mutex)
-    : m_mdl_mutex(mdl_mutex), m_thd(thd), m_mutex(mutex) {
+                                 mysql_mutex_t *mutex,
+                                 bool allow_caller_to_use_mdl_mutex,
+                                 bool allow_bypass)
+    : m_mdl_mutex(mdl_mutex),
+      m_thd(thd),
+      m_mutex(mutex),
+      m_allow_bypass(allow_bypass) {
   assert(m_mdl_mutex);
 
   // If regular mutex is not passed in then ignore use_mdl_mutex flag.
-  if (use_mdl_mutex || !m_mutex) {
-    assert(m_thd);
-    m_ticket = m_mdl_mutex->lock(m_thd);
-  } else {
-    mysql_mutex_lock(m_mutex);
+  if (!allow_bypass) {
+    bool enable_usage = use_mdl_mutex && allow_caller_to_use_mdl_mutex;
+    if (enable_usage || !m_mutex) {
+      assert(m_thd);
+      m_ticket = m_mdl_mutex->lock(m_thd);
+    } else {
+      assert(m_mutex);
+      mysql_mutex_lock(m_mutex);
+    }
   }
 }
 
@@ -5352,7 +5361,7 @@ MDL_mutex_guard::~MDL_mutex_guard() {
   if (m_ticket) {
     assert(m_mdl_mutex);
     m_mdl_mutex->unlock(m_thd, m_ticket);
-  } else {
+  } else if (!m_allow_bypass) {
     assert(m_mutex);
     mysql_mutex_unlock(m_mutex);
   }

sql/mdl.h

@@ -1863,7 +1863,9 @@ class MDL_mutex {
 */
 class MDL_mutex_guard {
  public:
-  MDL_mutex_guard(MDL_mutex *mdl_mutex, THD *thd, mysql_mutex_t *mutex);
+  MDL_mutex_guard(MDL_mutex *mdl_mutex, THD *thd, mysql_mutex_t *mutex,
+                  bool allow_caller_to_use_mdl_mutex = true,
+                  bool allow_bypass = false);
   ~MDL_mutex_guard();
 
  private:
@@ -1878,6 +1880,9 @@ class MDL_mutex_guard {
 
   // Regular mutex to lock and unlock if MDL mutex is disabled.
   mysql_mutex_t *m_mutex{nullptr};
+
+  // Should the guard bypass the incoming request?
+  bool m_allow_bypass{false};
 };
 
 #endif

sql/mysqld.cc

@@ -1692,6 +1692,7 @@ bool enable_optimizer_cputime_with_wallclock = true;
 bool enable_pfs_global_select = false;
 bool enable_rocksdb_intrinsic_tmp_table = false;
 bool use_mdl_mutex = true;
+bool use_status_mdl_mutex = false;
 bool install_plugin_skip_registration = false;
 
 /**
@@ -6331,6 +6332,7 @@ static int init_thread_environment() {
 
   // Init THD static objects.
   THD::init_mutex_thd_security_ctx();
+  THD::init_mutex_thd_status_aggregation();
 
   return 0;
 }
@@ -13729,25 +13731,26 @@ class Reset_thd_status : public Do_THD_Impl {
 /**
   Reset global and session status variables.
 */
-void refresh_status() {
-  mysql_mutex_lock(&LOCK_status);
+void refresh_status(THD *thd) {
+  {
+    MDL_mutex_guard guard(THD::get_mutex_thd_status_aggregation(), thd,
+                          &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);
+    /* 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();
+    /* Reset aggregated status counters. */
+    reset_pfs_status_stats();
 #endif
 
-  /* Reset some global variables. */
-  reset_status_vars();
-
-  /* Reset the counters of all key caches (default and named). */
-  process_key_caches(reset_key_cache_counters);
-  flush_status_time = time((time_t *)nullptr);
-  mysql_mutex_unlock(&LOCK_status);
+    /* Reset some global variables. */
+    reset_status_vars();
 
+    /* Reset the counters of all key caches (default and named). */
+    process_key_caches(reset_key_cache_counters);
+    flush_status_time = time((time_t *)nullptr);
+  }
   /*
     Set max_used_connections to the number of currently open
     connections.  Do this out of LOCK_status to avoid deadlocks.

sql/mysqld.h

@@ -139,7 +139,7 @@ extern bool dynamic_plugins_are_initialized;
 
 bool signal_restart_server();
 void kill_mysql(void);
-void refresh_status();
+void refresh_status(THD *thd);
 void reset_status_by_thd();
 bool is_secure_file_path(const char *path);
 ulong sql_rnd_with_mutex();
@@ -456,6 +456,7 @@ extern std::vector<std::string> authentication_policy_list;
 extern bool opt_log_ddl;
 extern bool slave_high_priority_ddl;
 extern bool use_mdl_mutex;
+extern bool use_status_mdl_mutex;
 extern bool install_plugin_skip_registration;
 extern ulonglong slave_high_priority_lock_wait_timeout_nsec;
 extern double slave_high_priority_lock_wait_timeout_double;

sql/sql_class.cc

@@ -138,6 +138,7 @@ using std::unique_ptr;
 
 // Static objects.
 MDL_mutex THD::mutex_thd_security_ctx[THD::mutex_thd_security_ctx_partitions];
+MDL_mutex THD::mutex_thd_status_aggregation;
 
 /*
   The following is used to initialise Table_ident with a internal
@@ -913,6 +914,21 @@ MDL_mutex *THD::get_mutex_thd_security_ctx() {
   return &mutex_thd_security_ctx[partition];
 }
 
+/**
+ Initialize mutex_thd_status_aggregation.
+ */
+void THD::init_mutex_thd_status_aggregation() {
+  std::string thd_status_aggregation{"STATUS_AGGREGATION"};
+  mutex_thd_status_aggregation.init(thd_status_aggregation);
+}
+
+/**
+ Return mutex_thd_status_aggregation.
+ */
+MDL_mutex *THD::get_mutex_thd_status_aggregation() {
+  return &mutex_thd_status_aggregation;
+}
+
 void THD::copy_table_access_properties(THD *thd) {
   thread_stack = thd->thread_stack;
   variables.option_bits = thd->variables.option_bits & OPTION_BIN_LOG;
@@ -1386,10 +1402,12 @@ void THD::cleanup_connection(void) {
   m_thd_life_cycle_stage = enum_thd_life_cycle_stages::ACTIVE;
 
   /* Aggregate to global status now that cleanup is done. */
-  mysql_mutex_lock(&LOCK_status);
-  add_to_status(&global_status_var, &status_var);
-  reset_system_status_vars(&status_var);
-  mysql_mutex_unlock(&LOCK_status);
+  {
+    MDL_mutex_guard guard(get_mutex_thd_status_aggregation(), this,
+                          &LOCK_status, use_status_mdl_mutex);
+    add_to_status(&global_status_var, &status_var);
+    reset_system_status_vars(&status_var);
+  }
 
   propagate_pending_global_disk_usage();
 
@@ -1673,19 +1691,21 @@ void THD::release_resources() {
 
   reset_connection_certificate();
 
-  mysql_mutex_lock(&LOCK_status);
-  /* Add thread status to the global totals. */
-  add_to_status(&global_status_var, &status_var);
+  {
+    MDL_mutex_guard guard(get_mutex_thd_status_aggregation(), this,
+                          &LOCK_status, use_status_mdl_mutex);
+    /* Add thread status to the global totals. */
+    add_to_status(&global_status_var, &status_var);
 #ifdef HAVE_PSI_THREAD_INTERFACE
-  /* Aggregate thread status into the Performance Schema. */
-  if (m_psi != nullptr) {
-    PSI_THREAD_CALL(aggregate_thread_status)(m_psi);
-  }
+    /* Aggregate thread status into the Performance Schema. */
+    if (m_psi != nullptr) {
+      PSI_THREAD_CALL(aggregate_thread_status)(m_psi);
+    }
 #endif /* HAVE_PSI_THREAD_INTERFACE */
-  /* Ensure that the thread status is not re-aggregated to the global totals. */
-  status_var_aggregated = true;
-
-  mysql_mutex_unlock(&LOCK_status);
+    /* Ensure that the thread status is not re-aggregated to the global totals.
+     */
+    status_var_aggregated = true;
+  }
 
   propagate_pending_global_disk_usage();
 

sql/sql_class.h

@@ -1258,10 +1258,15 @@ class THD : public MDL_context_owner,
   static constexpr int mutex_thd_security_ctx_partitions = 8;
   static MDL_mutex mutex_thd_security_ctx[mutex_thd_security_ctx_partitions];
 
+  static MDL_mutex mutex_thd_status_aggregation;
+
  public:
   static void init_mutex_thd_security_ctx();
   MDL_mutex *get_mutex_thd_security_ctx();
 
+  static void init_mutex_thd_status_aggregation();
+  static MDL_mutex *get_mutex_thd_status_aggregation();
+
   /**
     Protects THD::db_read_only_hash.
   */

sql/sql_parse.cc

@@ -2904,9 +2904,11 @@ bool dispatch_command(THD *thd, const COM_DATA *com_data,
 
       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(&current_global_status_var);
-      mysql_mutex_unlock(&LOCK_status);
+      {
+        MDL_mutex_guard guard(THD::get_mutex_thd_status_aggregation(), thd,
+                              &LOCK_status, use_status_mdl_mutex);
+        calc_sum_of_all_status(&current_global_status_var);
+      }
       if (!(uptime = (ulong)(thd->query_start_in_secs() - server_start_time)))
         queries_per_second1000 = 0;
       else

sql/sql_reload.cc

@@ -344,7 +344,7 @@ bool handle_reload_request(THD *thd, unsigned long options, Table_ref *tables,
     }
   }
   if (options & REFRESH_HOSTS) hostname_cache_refresh();
-  if (thd && (options & REFRESH_STATUS)) refresh_status();
+  if (thd && (options & REFRESH_STATUS)) refresh_status(thd);
   if (options & REFRESH_THREADS)
     Per_thread_connection_handler::kill_blocked_pthreads();
   if (options & REFRESH_MASTER) {