MDEV-38179 sysvars tracker: lock only when needed

mariadb-YuchenPei · mariadb-YuchenPei · commit da6654903948 · 2026-04-20T13:22:56.000+10:00
Lock LOCK_plugin only when the variable is a plugin variable.
Lock LOCK_global_system_variables only when the variable is a
global-only variable or a plugin variable.

Also fix lockless session value for Sys_var_multi_source_ulonglong.

This saves 54 calls to pthread_mutex_lock when constructing the
initial OK packet.

The added testcases do not show the lock savings. They are there to
improve coverage for the MDEV-31609 feature, in light of bugs like
MDEV-39207 and MDEV-39217.
diff --git a/include/mysql/plugin.h b/include/mysql/plugin.h
@@ -200,7 +200,7 @@ enum enum_mysql_show_type
 
 enum enum_var_type
 {
-  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL
+  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK
 };
 
 struct st_mysql_show_var {
diff --git a/include/mysql/plugin_audit.h.pp b/include/mysql/plugin_audit.h.pp
@@ -540,7 +540,7 @@
 };
 enum enum_var_type
 {
-  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL
+  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK
 };
 struct st_mysql_show_var {
   const char *name;
diff --git a/include/mysql/plugin_auth.h.pp b/include/mysql/plugin_auth.h.pp
@@ -540,7 +540,7 @@
 };
 enum enum_var_type
 {
-  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL
+  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK
 };
 struct st_mysql_show_var {
   const char *name;
diff --git a/include/mysql/plugin_data_type.h.pp b/include/mysql/plugin_data_type.h.pp
@@ -540,7 +540,7 @@
 };
 enum enum_var_type
 {
-  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL
+  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK
 };
 struct st_mysql_show_var {
   const char *name;
diff --git a/include/mysql/plugin_encryption.h.pp b/include/mysql/plugin_encryption.h.pp
@@ -540,7 +540,7 @@
 };
 enum enum_var_type
 {
-  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL
+  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK
 };
 struct st_mysql_show_var {
   const char *name;
diff --git a/include/mysql/plugin_ftparser.h.pp b/include/mysql/plugin_ftparser.h.pp
@@ -540,7 +540,7 @@
 };
 enum enum_var_type
 {
-  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL
+  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK
 };
 struct st_mysql_show_var {
   const char *name;
diff --git a/include/mysql/plugin_function.h.pp b/include/mysql/plugin_function.h.pp
@@ -540,7 +540,7 @@
 };
 enum enum_var_type
 {
-  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL
+  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK
 };
 struct st_mysql_show_var {
   const char *name;
diff --git a/include/mysql/plugin_password_validation.h.pp b/include/mysql/plugin_password_validation.h.pp
@@ -540,7 +540,7 @@
 };
 enum enum_var_type
 {
-  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL
+  SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK
 };
 struct st_mysql_show_var {
   const char *name;
diff --git a/mysql-test/suite/sys_vars/r/session_track_system_variables_basic.result b/mysql-test/suite/sys_vars/r/session_track_system_variables_basic.result
@@ -202,3 +202,59 @@ connect foo,localhost,root;
 connection default;
 disconnect foo;
 # End of tests 11.5
+#
+# MDEV-38179 regression in t_connect
+#
+# myisam_block_size is a global only plugin var and is not
+# added to the session_track_system_variables.
+# example_int_var is a session plugin var and is included.
+install soname 'ha_example.so';
+set global session_track_system_variables='sql_slave_skip_counter,character_set_client,example_int_var';
+select @@global.session_track_system_variables;
+@@global.session_track_system_variables
+character_set_client,example_int_var,sql_slave_skip_counter
+connect foo,localhost,root;
+-- Tracker : SESSION_TRACK_SYSTEM_VARIABLES
+-- character_set_client: latin1
+-- example_int_var: 0
+-- sql_slave_skip_counter: 0
+
+select @@session.session_track_system_variables;
+@@session.session_track_system_variables
+character_set_client,example_int_var,sql_slave_skip_counter
+connection default;
+disconnect foo;
+set global session_track_system_variables='sql_slave_skip_counter,myisam_block_size,character_set_client,example_int_var';
+select @@global.session_track_system_variables;
+@@global.session_track_system_variables
+character_set_client,example_int_var,sql_slave_skip_counter
+connect foo,localhost,root;
+-- Tracker : SESSION_TRACK_SYSTEM_VARIABLES
+-- character_set_client: latin1
+-- example_int_var: 0
+-- sql_slave_skip_counter: 0
+
+select @@session.session_track_system_variables;
+@@session.session_track_system_variables
+character_set_client,example_int_var,sql_slave_skip_counter
+connection default;
+disconnect foo;
+set global session_track_system_variables='sql_slave_skip_counter,myisam_block_size,max_connections,character_set_client,example_int_var';
+select @@global.session_track_system_variables;
+@@global.session_track_system_variables
+character_set_client,example_int_var,max_connections,sql_slave_skip_counter
+connect foo,localhost,root;
+-- Tracker : SESSION_TRACK_SYSTEM_VARIABLES
+-- character_set_client: latin1
+-- example_int_var: 0
+-- max_connections: 151
+-- sql_slave_skip_counter: 0
+
+select @@session.session_track_system_variables;
+@@session.session_track_system_variables
+character_set_client,example_int_var,max_connections,sql_slave_skip_counter
+connection default;
+SET global session_track_system_variables = @global_saved_tmp;
+disconnect foo;
+uninstall soname 'ha_example.so';
+# End of tests 11.8
diff --git a/mysql-test/suite/sys_vars/t/session_track_system_variables_basic.test b/mysql-test/suite/sys_vars/t/session_track_system_variables_basic.test
@@ -173,3 +173,45 @@ connection default;
 disconnect foo;
 
 --echo # End of tests 11.5
+
+--echo #
+--echo # MDEV-38179 regression in t_connect
+--echo #
+
+--echo # myisam_block_size is a global only plugin var and is not
+--echo # added to the session_track_system_variables.
+--echo # example_int_var is a session plugin var and is included.
+install soname 'ha_example.so';
+set global session_track_system_variables='sql_slave_skip_counter,character_set_client,example_int_var';
+select @@global.session_track_system_variables;
+enable_session_track_info;
+
+connect foo,localhost,root;
+select @@session.session_track_system_variables;
+
+connection default;
+disconnect foo;
+
+set global session_track_system_variables='sql_slave_skip_counter,myisam_block_size,character_set_client,example_int_var';
+select @@global.session_track_system_variables;
+enable_session_track_info;
+
+connect foo,localhost,root;
+select @@session.session_track_system_variables;
+
+connection default;
+disconnect foo;
+
+set global session_track_system_variables='sql_slave_skip_counter,myisam_block_size,max_connections,character_set_client,example_int_var';
+select @@global.session_track_system_variables;
+enable_session_track_info;
+
+connect foo,localhost,root;
+select @@session.session_track_system_variables;
+
+connection default;
+
+SET global session_track_system_variables = @global_saved_tmp;
+disconnect foo;
+uninstall soname 'ha_example.so';
+--echo # End of tests 11.8
diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc
@@ -97,6 +97,22 @@ bool Session_sysvars_tracker::vars_list::insert(const sys_var *svar)
   node->m_svar= (sys_var *)svar;
   node->test_load= node->m_svar->test_load;
   node->m_changed= false;
+  /*
+    No need to lock LOCK_plugin for non-plugin variables.
+  */
+  node->skip_plugin_lock= !node->m_svar->cast_pluginvar();
+  /*
+    No need to lock LOCK_global_system_variables for session or
+    session/global variables. Note that
+
+    1. Plugin global-only variables are not included in the session
+       sysvar tracker even if specified by the user
+    2. Plugin session variables require LOCK_global_system_variables
+       because of sync_dynamic_session_variables
+  */
+  node->skip_global_lock= (node->skip_plugin_lock &&
+                           node->m_svar->scope() == sys_var::SESSION);
+
   if (my_hash_insert(&m_registered_sysvars, (uchar *) node))
   {
     my_free(node);
@@ -299,6 +315,7 @@ bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf,
   for (ulong i= 0; i < m_registered_sysvars.records; i++)
   {
     sysvar_node_st *node= at(i);
+    /* Global plugin variables are not included */
     if (*node->test_load)
       names[idx++]= &node->m_svar->name;
   }
@@ -445,30 +462,34 @@ bool Session_sysvars_tracker::vars_list::store(THD *thd, String *buf)
     SHOW_VAR show;
     CHARSET_INFO *charset;
     size_t val_length, length;
-    mysql_mutex_lock(&LOCK_global_system_variables);
-    mysql_mutex_lock(&LOCK_plugin);
+    if (!node->skip_global_lock)
+      mysql_mutex_lock(&LOCK_global_system_variables);
+    if (!node->skip_plugin_lock)
+      mysql_mutex_lock(&LOCK_plugin);
+    /* Happens if the corresponding plugin has been unloaded */
     if (!*node->test_load)
     {
+      DBUG_ASSERT(!node->skip_plugin_lock);
       mysql_mutex_unlock(&LOCK_plugin);
       mysql_mutex_unlock(&LOCK_global_system_variables);
       continue;
     }
     sys_var *svar= node->m_svar;
-    bool is_plugin= svar->cast_pluginvar();
-    if (!is_plugin)
-      mysql_mutex_unlock(&LOCK_plugin);
 
     /* As its always system variable. */
     show.type= SHOW_SYS;
     show.name= svar->name.str;
     show.value= (char *) svar;
 
-    const char *value= get_one_variable(thd, &show, OPT_SESSION, SHOW_SYS, NULL,
+    const enum enum_var_type vtype=
+      node->skip_global_lock ? SHOW_OPT_SESSION_NO_LOCK : OPT_SESSION;
+    const char *value= get_one_variable(thd, &show, vtype, SHOW_SYS, NULL,
                                         &charset, val_buf, &val_length);
 
-    if (is_plugin)
+    if (!node->skip_plugin_lock)
       mysql_mutex_unlock(&LOCK_plugin);
-    mysql_mutex_unlock(&LOCK_global_system_variables);
+    if (!node->skip_global_lock)
+      mysql_mutex_unlock(&LOCK_global_system_variables);
 
     length= net_length_size(svar->name.length) +
       svar->name.length +
diff --git a/sql/session_tracker.h b/sql/session_tracker.h
@@ -128,6 +128,8 @@ class Session_sysvars_tracker: public State_tracker
     sys_var *m_svar;
     bool *test_load;
     bool m_changed;
+    bool skip_plugin_lock;
+    bool skip_global_lock;
   };
 
   class vars_list
diff --git a/sql/set_var.cc b/sql/set_var.cc
@@ -241,6 +241,11 @@ const uchar *sys_var::session_value_ptr(THD *thd, const LEX_CSTRING *base) const
   return session_var_ptr(thd);
 }
 
+const uchar *sys_var::session_no_lock_value_ptr(THD *thd, const LEX_CSTRING *base) const
+{
+  return session_value_ptr(thd, base);
+}
+
 const uchar *sys_var::global_value_ptr(THD *thd, const LEX_CSTRING *base) const
 {
   return global_var_ptr();
@@ -284,6 +289,8 @@ const uchar *sys_var::value_ptr(THD *thd, enum_var_type type,
     AutoRLock lock(guard);
     return global_value_ptr(thd, base);
   }
+  else if (type == SHOW_OPT_SESSION_NO_LOCK)
+    return session_no_lock_value_ptr(thd, base);
   else
     return session_value_ptr(thd, base);
 }
@@ -806,6 +813,7 @@ int set_var::check(THD *thd)
   switch (type) {
   case SHOW_OPT_DEFAULT:
   case SHOW_OPT_SESSION:
+  case SHOW_OPT_SESSION_NO_LOCK:
     DBUG_ASSERT(var->scope() != sys_var::GLOBAL);
     if (var->on_check_access_session(thd))
       return -1;
diff --git a/sql/set_var.h b/sql/set_var.h
@@ -58,6 +58,13 @@ class sys_var: protected Value_source // for double_from_string_with_check
 public:
   sys_var *next;
   LEX_CSTRING name;
+
+  /*
+    - For non-plugin system variables: always true (static_test_load)
+    - For plugin global var: always false (static_unload)
+    - For plugin session var: true on plugin init, false on plugin
+      deinit
+ */
   bool *test_load;
   enum flag_enum { GLOBAL, SESSION, ONLY_SESSION, SCOPE_MASK=1023,
                    READONLY=1024, ALLOCATED=2048, PARSE_EARLY=4096,
@@ -238,6 +245,7 @@ class sys_var: protected Value_source // for double_from_string_with_check
     int for SHOW_INT, longlong for SHOW_LONGLONG, etc).
   */
   virtual const uchar *session_value_ptr(THD *thd, const LEX_CSTRING *base) const;
+  virtual const uchar *session_no_lock_value_ptr(THD *thd, const LEX_CSTRING *base) const;
   virtual const uchar *global_value_ptr(THD *thd, const LEX_CSTRING *base) const;
 
   /**
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
@@ -5959,18 +5959,20 @@ Sys_slave_net_timeout(
 */
 
 ulonglong Sys_var_multi_source_ulonglong::
-get_master_info_ulonglong_value(THD *thd) const
+get_master_info_ulonglong_value(THD *thd, bool with_lock) const
 {
   Master_info *mi;
   ulonglong res= 0;                                  // Default value
-  mysql_mutex_unlock(&LOCK_global_system_variables);
+  if (with_lock)
+    mysql_mutex_unlock(&LOCK_global_system_variables);
   if ((mi= get_master_info(&thd->variables.default_master_connection,
                            Sql_condition::WARN_LEVEL_WARN)))
   {
     res= (mi->*mi_accessor_func)();
     mi->release();
   }
-  mysql_mutex_lock(&LOCK_global_system_variables);
+  if (with_lock)
+    mysql_mutex_lock(&LOCK_global_system_variables);
   return res;
 }
   
diff --git a/sql/sys_vars.inl b/sql/sys_vars.inl
@@ -2571,19 +2571,27 @@ public:
     /* Use value given in variable declaration */
     global_save_default(thd, var);
   }
-  const uchar *session_value_ptr(THD *thd, const LEX_CSTRING *base) const override
+  uchar *value_ptr_internal(THD *thd, const LEX_CSTRING *base, bool with_lock) const
   {
     ulonglong *tmp, res;
     tmp= (ulonglong*) (((uchar*)&(thd->variables)) + offset);
-    res= get_master_info_ulonglong_value(thd);
+    res= get_master_info_ulonglong_value(thd, with_lock);
     *tmp= res;
     return (uchar*) tmp;
   }
+  const uchar *session_value_ptr(THD *thd, const LEX_CSTRING *base) const override
+  {
+    return value_ptr_internal(thd, base, true);
+  }
   const uchar *global_value_ptr(THD *thd, const LEX_CSTRING *base) const override
   {
-    return session_value_ptr(thd, base);
+    return value_ptr_internal(thd, base, true);
+  }
+  const uchar *session_no_lock_value_ptr(THD *thd, const LEX_CSTRING *base) const override
+  {
+    return value_ptr_internal(thd, base, false);
   }
-  ulonglong get_master_info_ulonglong_value(THD *thd) const;
+  ulonglong get_master_info_ulonglong_value(THD *thd, bool with_lock) const;
   bool update_variable(THD *thd, Master_info *mi)
   {
     return update_multi_source_variable_func(this, thd, mi);

Original file line number	Diff line number	Diff line change
`@@ -200,7 +200,7 @@ enum enum_mysql_show_type`
`200`	`200`
`201`	`201`	`enum enum_var_type`
`202`	`202`	`{`
`203`		`- SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL`
	`203`	`+ SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK`
`204`	`204`	`};`
`205`	`205`
`206`	`206`	`struct st_mysql_show_var {`
Original file line number	Diff line number	Diff line change
`@@ -540,7 +540,7 @@`
`540`	`540`	`};`
`541`	`541`	`enum enum_var_type`
`542`	`542`	`{`
`543`		`- SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL`
	`543`	`+ SHOW_OPT_DEFAULT= 0, SHOW_OPT_SESSION, SHOW_OPT_GLOBAL, SHOW_OPT_SESSION_NO_LOCK`
`544`	`544`	`};`
`545`	`545`	`struct st_mysql_show_var {`
`546`	`546`	`const char *name;`