Bug #34907 Handlers cannot replace the plugin variable update func for thread vars
Submitted: 27 Feb 2008 21:23 Modified: 20 Oct 2009 7:22
Reporter: Bill Mitchell Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server Severity:S3 (Non-critical)
Version:5.1.22 OS:Any
Assigned to: CPU Architecture:Any
Triage: Triaged: D5 (Feature request)

[27 Feb 2008 21:23] Bill Mitchell
Description:
(1) In order for the handler to process a change in a plug-in variable value, it would be nice to be able to define a handler method for the variable update_func to action the change in the variable value immediately, or to set a flag that the value has changed so that a handler thread can process the change at a convenient point.  Although this works for system variables, it doesn't work for thread local variables because the handler update_func doesn't know whether it is the global value or the thread local value that is being updated.  

In sys_var_pluginvar::set_default() and sys_var_pluginvar::update(), it knows which variable is being updated by its incoming parameter type.  But this is not passed to the update function.   So the update function doesn't know which is being updated, it just has a pointer to where the new value should be stored.  

(2) There is a related issue when writing a handler method to update a variable value of type str (PLUGIN_VAR_STR) that you might want to fix at the same time.  If you look at the default implementation of update_func_str() below, it accesses the flags in the st_mysql_sys_var struct to determine if the variable needs allocation.  Much as the handler might like to copy and use this code intact to perform the mechanics of updating the value, var->flags is not accessible to the handler because the st_mysql_sys_var struct definition is not included in plugin.h.  So, today, the handler just has to know which variables need string reallocation and which don't, and define different update functions for each.  

static void update_func_str(THD *thd, struct st_mysql_sys_var *var,
                             void *tgt, void *save)
{
  char *old= *(char **) tgt;
  *(char **)tgt= *(char **) save;
  if (var->flags & PLUGIN_VAR_MEMALLOC)
  {
    *(char **)tgt= my_strdup(*(char **) save, MYF(0));
    my_free(old, MYF(0));
  }
}

How to repeat:
In a handler, declare a thread local variable of type string, e.g.:

static const char default_log_filename[] = "log.trace";

static MYSQL_THDVAR_STR(log_filename, PLUGIN_VAR_OPCMDARG | PLUGIN_VAR_MEMALLOC,
  "The name of the logfile. ",
  NULL, ExampleHandlerton::update_log_filename, default_log_filename);

In the handler's update_log_filename() method, it is impossible to differentiate which variable value is being modified.  

Suggested fix:
My suggestion is that, following the convention used by the THDVAR macro where a zero for the thread value indicates that the global value should be fetched, sys_var_pluginvar::set_default() and sys_var_pluginvar::update() should pass a zero thread pointer when updating the global value.  

The section of code:

  if (!(plugin_var->flags & PLUGIN_VAR_THDLOCAL) || var->type == OPT_GLOBAL)
  {
    /* variable we are updating has global scope, so we unlock after updating */
    plugin_var->update(thd, plugin_var, tgt, &var->save_result);
    pthread_mutex_unlock(&LOCK_global_system_variables);
  }
  else
  {
    pthread_mutex_unlock(&LOCK_global_system_variables);
    plugin_var->update(thd, plugin_var, tgt, &var->save_result);
  }
should read:

  if (!(plugin_var->flags & PLUGIN_VAR_THDLOCAL) || var->type == OPT_GLOBAL)
  {
    /* variable we are updating has global scope, so we unlock after updating */
    plugin_var->update(0, plugin_var, tgt, &var->save_result);
    pthread_mutex_unlock(&LOCK_global_system_variables);
  }
  else
  {
    pthread_mutex_unlock(&LOCK_global_system_variables);
    plugin_var->update(thd, plugin_var, tgt, &var->save_result);
  }

With this change, the handler can look at the thread pointer to determine which value is being changed, and perform the appropriate logic.  

The above code fragment is extracted from 5.1.23, as by inspection it has the same issue that I debugged in 5.1.22.
[27 Mar 2008 14:36] Susanne Ebrecht
Many thanks for writing a bug report.

Please try with newer version. MySQL 5.1.23.
[27 Apr 2008 23:00] Bugs System
No feedback was provided for this bug for over a month, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".
[4 Aug 2008 4:48] Valeriy Kravchuk
Please, try to repeat with a newer version, 5.1.26, and inform about the results.
[4 Sep 2008 23:00] Bugs System
No feedback was provided for this bug for over a month, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".
[20 Oct 2009 6:32] Yuan WANG
Hello, I have tried it with 5.1.33 and got the following error:

src/ha_ntse.cpp:315: error: invalid use of incomplete type 'struct st_mysql_sys_var'
../../include/mysql/plugin.h:148: error: forward declaration of 'struct st_mysql_sys_var'
[20 Oct 2009 6:59] Valeriy Kravchuk
5.1.33 is also ages old already. Please, try with 5.1.39 and inform about the results.