Adieu find_sys_var_ex()

Only take LOCK_plugin for plugin system variables.

Reverted optimisation that was originally done for session tracker: it
makes much less sense now. Specifically only if connections would want to
track plugin session variables changes and these changes would actually
happen frequently. If this ever becomes an issue, there're much better
ways to optimise this workload.

Part of MDEV-14984 - regression in connect performance

Author: svoj

sql/session_tracker.cc

@@ -135,13 +135,6 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd,
   token= var_list.str;
 
   track_all= false;
-  /*
-    If Lock to the plugin mutex is not acquired here itself, it results
-    in having to acquire it multiple times in find_sys_var_ex for each
-    token value. Hence the mutex is handled here to avoid a performance
-    overhead.
-  */
-  mysql_mutex_lock(&LOCK_plugin);
   for (;;)
   {
     sys_var *svar;
@@ -165,11 +158,10 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd,
     {
       track_all= true;
     }
-    else if ((svar=
-              find_sys_var_ex(thd, var.str, var.length, throw_error, true)))
+    else if ((svar= find_sys_var(thd, var.str, var.length, throw_error)))
     {
       if (insert(svar) == TRUE)
-        goto error;
+        return true;
     }
     else if (throw_error && thd)
     {
@@ -179,20 +171,14 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd,
                           "be ignored.", (int)var.length, token);
     }
     else
-      goto error;
+      return true;
 
     if (lasts)
       token= lasts + 1;
     else
       break;
   }
-  mysql_mutex_unlock(&LOCK_plugin);
-
   return false;
-
-error:
-  mysql_mutex_unlock(&LOCK_plugin);
-  return true;
 }
 
 
@@ -211,14 +197,6 @@ bool sysvartrack_validate_value(THD *thd, const char *str, size_t len)
 
   token= var_list.str;
 
-  /*
-    If Lock to the plugin mutex is not acquired here itself, it results
-    in having to acquire it multiple times in find_sys_var_ex for each
-    token value. Hence the mutex is handled here to avoid a performance
-    overhead.
-  */
-  if (!thd)
-    mysql_mutex_lock(&LOCK_plugin);
   for (;;)
   {
     LEX_CSTRING var;
@@ -237,22 +215,14 @@ bool sysvartrack_validate_value(THD *thd, const char *str, size_t len)
     /* Remove leading/trailing whitespace. */
     trim_whitespace(system_charset_info, &var);
 
-    if(!strcmp(var.str, "*") &&
-       !find_sys_var_ex(thd, var.str, var.length, false, true))
-    {
-      if (!thd)
-        mysql_mutex_unlock(&LOCK_plugin);
+    if (!strcmp(var.str, "*") && !find_sys_var(thd, var.str, var.length))
       return true;
-    }
 
     if (lasts)
       token= lasts + 1;
     else
       break;
   }
-  if (!thd)
-    mysql_mutex_unlock(&LOCK_plugin);
-
   return false;
 }
 

sql/set_var.h

@@ -398,7 +398,8 @@ extern SHOW_COMP_OPTION have_openssl;
 SHOW_VAR* enumerate_sys_vars(THD *thd, bool sorted, enum enum_var_type type);
 int fill_sysvars(THD *thd, TABLE_LIST *tables, COND *cond);
 
-sys_var *find_sys_var(THD *thd, const char *str, size_t length=0);
+sys_var *find_sys_var(THD *thd, const char *str, size_t length= 0,
+                      bool throw_error= false);
 int sql_set_variables(THD *thd, List<set_var_base> *var_list, bool free);
 
 #define SYSVAR_AUTOSIZE(VAR,VAL)                        \

sql/sql_lex.cc

@@ -7254,8 +7254,7 @@ bool LEX::set_system_variable(THD *thd, enum_var_type var_type,
 {
   sys_var *tmp;
   if (unlikely(check_reserved_words(name1)) ||
-      unlikely(!(tmp= find_sys_var_ex(thd, name2->str, name2->length, true,
-                                      false))))
+      unlikely(!(tmp= find_sys_var(thd, name2->str, name2->length, true))))
   {
     my_error(ER_UNKNOWN_STRUCTURED_VARIABLE, MYF(0),
              (int) name1->length, name1->str);
@@ -7649,7 +7648,7 @@ int set_statement_var_if_exists(THD *thd, const char *var_name,
     my_error(ER_SP_BADSTATEMENT, MYF(0), "[NO]WAIT");
     return 1;
   }
-  if ((sysvar= find_sys_var_ex(thd, var_name, var_name_length, true, false)))
+  if ((sysvar= find_sys_var(thd, var_name, var_name_length, true)))
   {
     Item *item= new (thd->mem_root) Item_uint(thd, value);
     set_var *var= new (thd->mem_root) set_var(thd, OPT_SESSION, sysvar,

sql/sql_plugin.cc

@@ -2822,37 +2822,25 @@ static void update_func_double(THD *thd, struct st_mysql_sys_var *var,
   System Variables support
 ****************************************************************************/
 
-sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length,
-                         bool throw_error, bool locked)
+sys_var *find_sys_var(THD *thd, const char *str, size_t length,
+                      bool throw_error)
 {
   sys_var *var;
-  sys_var_pluginvar *pi= NULL;
-  plugin_ref plugin;
-  DBUG_ENTER("find_sys_var_ex");
+  sys_var_pluginvar *pi;
+  DBUG_ENTER("find_sys_var");
   DBUG_PRINT("enter", ("var '%.*s'", (int)length, str));
 
-  if (!locked)
-    mysql_mutex_lock(&LOCK_plugin);
   mysql_prlock_rdlock(&LOCK_system_variables_hash);
   if ((var= intern_find_sys_var(str, length)) &&
       (pi= var->cast_pluginvar()))
   {
-    mysql_prlock_unlock(&LOCK_system_variables_hash);
-    LEX *lex= thd ? thd->lex : 0;
-    if (!(plugin= intern_plugin_lock(lex, plugin_int_to_ref(pi->plugin))))
+    mysql_mutex_lock(&LOCK_plugin);
+    if (!intern_plugin_lock(thd ? thd->lex : 0, plugin_int_to_ref(pi->plugin),
+                            PLUGIN_IS_READY))
       var= NULL; /* failed to lock it, it must be uninstalling */
-    else
-    if (!(plugin_state(plugin) & PLUGIN_IS_READY))
-    {
-      /* initialization not completed */
-      var= NULL;
-      intern_plugin_unlock(lex, plugin);
-    }
-  }
-  else
-    mysql_prlock_unlock(&LOCK_system_variables_hash);
-  if (!locked)
     mysql_mutex_unlock(&LOCK_plugin);
+  }
+  mysql_prlock_unlock(&LOCK_system_variables_hash);
 
   if (unlikely(!throw_error && !var))
     my_error(ER_UNKNOWN_SYSTEM_VARIABLE, MYF(0),
@@ -2861,11 +2849,6 @@ sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length,
 }
 
 
-sys_var *find_sys_var(THD *thd, const char *str, size_t length)
-{
-  return find_sys_var_ex(thd, str, length, false, false);
-}
-
 /*
   called by register_var, construct_options and test_plugin_options.
   Returns the 'bookmark' for the named variable.

sql/sql_plugin.h

@@ -196,9 +196,6 @@ extern void sync_dynamic_session_variables(THD* thd, bool global_lock);
 extern bool plugin_dl_foreach(THD *thd, const LEX_CSTRING *dl,
                               plugin_foreach_func *func, void *arg);
 
-sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length,
-                         bool throw_error, bool locked);
-
 extern void sync_dynamic_session_variables(THD* thd, bool global_lock);
 #endif