mysql-proxy-discuss team mailing list archive

[Kay Röpke <Kay.Roepke@xxxxxxx>]

Hi!

(Nick, I cc'ed you on this because it likely directly affects funnel.  
Probably in a trivial way, but nonetheless.)

Looking at bug#46048 <http://bugs.mysql.com/46048> I realized that our  
option handling code is a bit suboptimal.
While in general it works just fine, and fixing the bug can be done  
with minimal effort, it's a bit cluttered.
BTW, this bug is simply that we can't figure out if an INT option has  
been set on the command line or via an assignment (most likely  
assigning the default value) in code when parsing the defaults file.
For strings and string arrays it works, doubles (which we don't use  
currently) would be even worse to handle, since you can't rely on a  
double fitting into a pointer.

To expand on the current implementation a bit:
Currently we base everything on GOptionEntry, which does not know  
about default values.
What's more, large chunks of main_cmdline deals with options, making  
the code a bit harder to follow than necessary, especially figuring  
out at what points config variables can change their values.
The plugins are a bit cleaner in this regard, since they will  
generally initialize their variables in their init function, set the  
GOptionEntries in get_options and do something based on the actual  
values in apply_config.

Nevertheless, it would be nice to abstract out the entire options vs  
keyfiles vs default values process, but this will likely mean breaking  
binary compatibility with older plugins (which means you wouldn't be  
able to load them into a newer chassis). That's fine, really, since  
most of the time no one does that anyway (at least I don't know of  
anyone who does).

Here's my proposal in generic terms (haven't fully spec'ed it out):
Wrap all chassis config variables in a struct, makes it easy to see  
which ones are influenced by the outside world.
Rather than using GOptionEntry directly, wrap it in a struct as well,  
alongside a union holding the different possible default values.
Construction of an argument "object" would mean a function call with  
pretty much what GOptionEntry contains, either followed by a function  
call to set the default if there is any, or via some pointer trickery  
in the construction function directly (i.e. pass in void* and deref/ 
cast to the correct type based on the G_OPTION_ARG enum).

To get the actual array of GOptionEntry structs, you'd call a function  
once you registered all the options. Optionally you could request a  
GOptionGroup directly, haven't made up my mind on that one yet.
Using option groups directly have the added benefit of allowing to  
pass in a user_data, which would contain the default values.
Ideally I'd like to hide all the complexity that allowing the defaults  
file brings with a post parse hook, also provided by the option  
groups. This would be a function in the chassis that deals with  
figuring out the defaults file implications.

The neat thing would be that almost all of the complexity of dealing  
with options would be hidden, making the available options, their  
storage location and their default values painfully obvious in the  
code. And it hopefully cuts down on the size of main_cmdline - I  
always nearly get lost in there, it's too long (which also address  
part of one of the review comments we received).

Thoughts, opinions and critique to me please :)

cheers,
-k
--
Kay Roepke
Software Engineer, MySQL Enterprise Tools

Sun Microsystems GmbH    Sonnenallee 1, DE-85551 Kirchheim-Heimstetten
Geschaeftsfuehrer:    Thomas Schroeder,  Wolfang Engels,  Wolf Frenkel
Vorsitz d. Aufs.rat.: Martin Haering                    HRB MUC 161028