[collectd] [PATCH] mysql plugin: add support for multipledatabases

[Sebastian Harl]

Hi Doug and Florian,

On Wed, Mar 18, 2009 at 12:51:24PM +0100, Florian Forster wrote:
> On Tue, Mar 17, 2009 at 04:51:53PM -0400, Doug MacEachern wrote:
> > db->database was NULL, which mysql_real_connect doesn't seem to mind?
> > +		db->database = strdup (db->instance);
> 
> I didn't test without a `Database' setting. I *thought* it was optional,
> though..

According to the MySQL (5.0) reference manual, specifying a database is
optional when using mysql_real_connect(). If the respective parameter is
NULL, MySQL does not set a default database after connecting (which is
perfectly fine, since the "SHOW * STATUS" queries are database
independent).

Doug, what was your motivation to not make that optional? Later on,
there's the following check (in mysql_config()):

  if (db->database == NULL)
  {
      ERROR ("mysql plugin: No `Database' configured");
      status = -1;
  }

... which is executed independent of legacy mode. This is a backward-
incompatible change, since, previously, the "Database" option has not
been mandatory.

I guess, your motivation was something like ensuring a consistent
behavior. If the argument of a <Database> block is used as instance only
(instead of specifying the name of a database as well), the behavior
would be somewhat strange (and inconsistent with the other database
plugins). What do you think about the following:

  * Let the argument of a <Database> block be optional.

  * If there is an argument, use it as database name (to be consistent
    with the other plugins).

  * Introduce a new option "Instance" - if that's not specified, use the
    database name as plugin instance (unless in legacy mode). (This
    option would make sense in the postgresql and dbi plugins as well.)

  * Either a database name or "Instance" would have to be specified when
    not in legacy mode to ensure that we end up having a plugin
    instance.

  * Possibly mark the "Database" _option_ inside a <Database> _block_ as
    deprecated (or not allowed at all).

Did I miss anything? What do you think? Imho, this would be the most
straight-forward and consistent solution.

Cheers,
Sebastian

-- 
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety.         -- Benjamin Franklin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20090320/fa2d9825/attachment.pgp