Plugin System Refactor

The current plugin variable system has a few problems.

• Bookmarks hash
• Locking around global status counters
• Implementation breaks strict aliasing
• Implementation is done via a ludicrous set of macros

The positives of the system are that it is done in plain structs that the plugin loader system can read before initialization of the plugin is accomplished.

The solution to all of these problems, and an introduction of new functionality can be accomplished by a redesign of the sys_var and plugin_sys_var system.

There are two fundamental design shifts. The first is that rather than making a variable inside of code somewhere and then handing a sys_var a pointer to that variable and a way to read and/or update that variable, the sys_var itself will actually become the owner of the data and the calling code will, after instantiation, read and set the value of the variable via the system variable interface.

The second is that the implementation of the system variable interface will be pluggable, so that for some of the variable types alternate implementations could be added. (For some, such as ReadOnly, it probably doesn't make a hell of a lot of sense to make an alternate implementation)

There will be a single abstract SystemVariable interface. There will be seven different templated concrete types that implement the SystemVariable interface.

Class Overview

SystemVariable
{
  bool is_status_var();
  const std::string &getName() const;
}
 
SystemVariableImpl<T>: SystemVariable
{
  abstract void set(T);
  abstract T get(T);
  abstract void increment();
  abstract void increment(T);
  abstract void decrement();
  abstract void decrement(T);
  abstract void clear(); 
}

ReadOnly<T>

Few and far between. Global but cannot change, so protection is not needed. This would be a good type for things like server_version.

GlobalShared<T>

A variable that is global in scope, shared by threads, and to which changes are immediately seen. The default implmentation will either have mutex protection for the value (strings/objects) or use atomic<> (for int values)

SessionLocal<T>

A variable which is local to a session does not propagate its changes back to the global context. Initialized with the value of a GlobalShared or a ReadOnly. Can also be used to implement user @@ variables.

GlobalDeferred<T>

Global in scope, but changes are done first to a session copy, and then are collected into the global variable at session end.

SessionDeferred<T>

The per-session counterpart to the GlobalDeferred.

GlobalCounter<T>

Shared data, but doesn't actually effect the operation of other threads. This is different from GlobalShared because it might have a lockless or dirty implmentation, or perhaps one based on memcached or the like with no local storage at all. (Think threads_connected)

SessionCounter<T>

Proxy for global counter, but also performs all increment operations on a per-session copy. (Think com_select)

Usage

From a usage perspective, Globals are the easiest. There is only one copy of each given Global, which is created by the code that needs it. The registration of the variable is only useful for things like I_S or show commands that need to iterate over the list of values, but the list or mapping of Global Variables isn't involved in the setting of any particular variable- only the instance of the Global.

Session variables are actually factories, since they have to create a instance per session. There will therefore be a global registry of SessionVar Factories. The global registry will be passed to the session constructor, and in the Session then there will be a registry of created SessionVar objects which can be used by the session.

The Session will also contain a list of SessionDeferred object which will be populated by the SessionDeferredFactory objects. When the Session is reaped, at the end of an execution thread, the SessionDeferred registry will be read by the scheduler thread and the results will be appended to or will overwrite the GlobalDeferred values.

In this scheme, use of GlobalShared objects is clearly the most expensive thing.

Usage Example

From a usage flow, the system will look something like this:

using namespace drizzled::variables;
 
/* Create a variable using the pluggable storage system */
 
/* Arguments are: variable name, default/initial value */
GlobalShared<string> gearman_connect_string("connect_string", "localhost");
 
GlobalCounter<uint32_t> gearman_bytes_transferred("bytes_transferred", 0);
SessionCounter<uint32_t> gearman_session_bytes_transferred(gearman_bytes_transferred);
 
/* At this point, gearman_bytes_transferred will not show up in
   show variables. Let's fix that */
gearmanPlugin.registerGlobalVariable(gearman_bytes_transferred);
 
gearman_bytes_transferred.set(0);
gearman_bytes_transferred.increment(1024);

Implementation

Global variables are easy. A reference to the global variable is registered with the global variable list so that they may be queried. The original global variable may still be used directly, so there is no need to excessive hash lookups or locks.

Session variables are create on use - so there is no cost for not touching them.

To access a session variable, you'll do:

SystemVariable<uint32_t> &bytes_transferred= session->getSessionVar("gearman","bytes_transferred");
bytes_transferred.increment(1024);

Behind the scenese, getSessionVar does something like this:

Registry<SystemVariable *>::iterator var_iter= this->variable_registry.find(name);
if (var_iter == this->variable_registry.end())
{
  var_iter= sessionVarFactoryRegistry.find(name);
  if (var_iter == sessionVarFactoryRegistry.end())
    return NULL;
  SystemVariable *new_var= (*var_iter).createSessionVar();
  sessionVarRegistry.add(new_var);
  return new_var;
}
return *var_iter;

variable_registry exists per Session. sessionVarFactoryRegistry is the global list of available session var factories.

The reference returned fomr getSessionVar can continue to be used for the lifetime of the session, so it is not necessary to call getSessionVar every time you need to get something.

show session status; or show session variables; would each get the list of variable names from the sessionVarFactory, and then do session.getSessionVar() for each of them, so it would effectively seed the session list with all the variables.

The sessionVarFactory is created with a reference to the global variable it's releated to, so an implementation of SessionVariable can be copy on write.

One thing that hasn't been mentioned is the distinction between status variables and system variables that exists currently, which comes down to read-only or read-write variables. (except mysql makes it more complex with read-only system variables) Since those aren't useful concept within the server code itself, I'd like to just make that a property of a variable (either make it a read-only flag, or a status var flag) and then let show status or show variables process the list as it wants to.

With that in mind, every global variable that winds up being user settable should be of type GlobalShared and should also provide a SessionLocal of the same variable. It's possible that at some point a user-settable variable could be made Deferred. It is extremely unlikely that a user-settable variable would be a Counter.

Side benefits to this refactor

Most of the strict aliasing problems go away, since we get to remove the big set of Macro-based crazy struct casting in plugin.h

Outstanding problems

Registering variables as actual objects means the current scheme of being able to read a list from the plugin object without initializing the plugin becomes almost impossible. eday is working on a two-phase init mechanism that will remove some of the pain here. The other bits of pain (namely I'd like drizzled --help to work without starting innodb) may be solved by having two different init methods - but since --help has required starting innodb up until about 1 week ago, I don't think it's the worst thing if we lose it for just a bit.

Jay's Thoughts

I don't really see the need for a separate *Impl class...the following should accomplish what you want to do.

template<class T>
class SystemVariable
{
public:
  typedef T value_type;
  typedef T* pointer_type;
  typedef T& reference_type;
  typedef const T& const_reference;
private:
  value_type value;
  const std::string name;
public:
  SystemVariable(const std::string &in_name)
  :
    name(in_name)
  {}
  SystemVariable(const std::string &in_name, reference_type in_value)
  :
    name(in_name),
    value(in_value)
  {}
  ~SystemVariable()
  {}
  inline const std::string &getName() const
  {
    return name;
  }
  inline const_reference getValue() const
  {
    return value;
  }
};

Response - Monty

The separate class is required to have a non-templatized ABT that can be used in containers. That way, we can have a vector<SystemVariable>. I do think we need a getString() method added so that things that want to operate on the list and who don't really need the actual type that's stored (like show variables) but instead just need a display value can operate on the Variables easily.

Such as:

class SystemVariable
{
private:
  const std::string name;
  virtual const std::string getStringImpl() const= 0;
public
  SystemVariable(const std::string &in_name)
    : name(in_name)
  {}
  virtual ~SystemVariable()
  {}
  inline const std::string &getName() const
  {
    return name;
  }
  const std::string getString() const
  {
     return getStringImpl();
  }
}
 
template<class T>
class SystemVariableImpl : public SystemVariable
{
public:
  typedef T value_type;
  typedef T* pointer_type;
  typedef T& reference_type;
  typedef const T& const_reference;
private:
  value_type value;
public:
 
  SystemVariableImpl(const std::string &in_name, reference_type in_value)
  :
    SystemVariable(in_name),
    value(in_value)
  {}
  virtual ~SystemVariableImpl()
  {}
 
  inline const_reference getValue() const= 0;
  virtual void setValue(const_reference in_value)= 0;
};

Response from Jay

OK, yeah, that makes sense. I'm not a huge fan of the "Impl" naming convention, though. What about calling the base class AbstractSystemVariable or BaseSystemVariable and then having the implementation class template called SystemVariable?