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.
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.