Hi Octo, Hi all,
Thanks for your time. It will permit to me to improve this plugin and
my
knowledge of C.
Le 22.12.2011 01:06, Florian Forster a écrit :
Hi Cyril,
thank you very much for your patches! I'm very sorry, but I can't
give
the plugin the thorough review it deserves right now. But maybe you
find
the following comments helpful.
On Tue, Dec 20, 2011 at 03:03:43PM +0200, Cyril Feraudet wrote:
- with_mysql_libs=`$with_mysql_config --libs 2>/dev/null`
+ with_mysql_libs=`$with_mysql_config --libs_r 2>/dev/null`
This looks like a bug fix. Would it make sense to add this change to
the
4.10 branch? If so, would you be willing to send a separate patch for
this?
You are right, I will submit a patch soon. write_mysql is often called
than
mysql plugin and non thread safe library make collectd crashing
quickly.
+CREATE TABLE `data` ( + `id` bigint(20) NOT NULL auto_increment, +
`timestamp` double NOT NULL, + `host_id` int(11) NOT NULL, +
`plugin_id` int(11) NOT NULL, + `plugin_instance` varchar(255)
default
NULL, + `type_id` int(11) NOT NULL, + `typeinstance` varchar(255)
default NULL, + `dataset_id` int(11) NOT NULL, + `value` double NOT
NULL,
One of the reasons why I didn't write a "write_dbi" or "write_mysql"
plugin before is because I was uncertain about the schema to use. Why
did you chose to have a separate table for host, plugin,
plugin_instance, type, and type_instance? I currently think that just
having an "identifier" table with five columns would be a better
trade-
off between normalization and complexity.
The idea was to save space in `data` table and improve querying in it.
For now, I've 800MB `data` table (MEMORY type) just for the last value
of
each metrics (4207 servers / 462,395 metrics) using REPLACE instead of
INSERT
statement. I think that SQL join are faster on numeric id than varchar.
But all of this things are questionable and dependent of the final use.
I think about a customizable SQL statement in plugin configuration.
ATE TABLE `dataset` (
Why do you store datasets in the database (in addition to the
types.db
file)? Having the definition in more than one place is bound to
create
inconsistencies at some point
This table is for readonly purpose and to make SQL query independent of
number
of metrics in a type. It also contain type added on the way by a
plugin.
diff --git a/src/write_mysql.c b/src/w
/blockquote> typedef struct host_s host_t; +struct host_s { + char
name[DATA_MAX_NAME_LEN]; + int id; + host_t *next_host; +};
You will be doing a lot of lookups in this cache(es). I think using a
binary search tree (an implementation if provided in
src/utils_avltree.h) would improve performance here.
Thanks for the tips, Il will do that.
int port =
tatic int write_mysql_config (const char *key, const char *value) {
[...] + } else if (strcasecmp ("Port", key) == 0) { + port = value;
You
a char* to an int. This won't produce what you expect. Use the
function
service_name_to_port_number() from src/common.h. +static int wri
Your'e right I will fix it.
t (void) { + conn = mysql_init(NULL); + if (mysql_real_connect(conn,
host, user, passwd, database, port, NULL, 0) == NULL) {
The conn
be (re-)established in the write() callback, when it is needed.
Otherwise you will never recover from connection failures. + if
(!mysql_th
Your'e right, I just issued it. I will fix it too.
{ + ERROR("write_mysql plugin: mysqlclient Thread Safe OFF");
I wasn't
s function. Good to know, thanks! +static int add_host_id (
me) { [...] + len = ssnprintf (query, sizeof (query), "SELECT id
FROM
host WHERE name = '%s'", hostname);
I think we should
statement with bound arguments (or whatever the terminology). This
way
(a) MySQL doesn't have to parse and optimize the query each time we
need
to insert a host and (b) quoting is done for us by the mysql library.
+static int add_host_id (char *
I will looking for. I never used it before.
[...] + pthread_mutex_lock(&mutex); [...] + if (row =
mysql_fetch_row(result)) { + id = atoi(row[0]); +
mysql_free_result(result); + pthread_mutex_unlock(&mutex); [...] +
pthread_mutex_unlock(&mutex); + return id; +}
Mutex is released twice
+static int add_plugin_id (char
I'll fix that.
{ +static int add_type_id (char *typename) {
A *lot* of duplicate code
ite a function like: int add_string (const char *table, const char
*str);
/* returns id on success, less than zero on failure. */ +static int
write_mysql_write(con
t *ds, const value_list_t *vl, + user_data_t __attribute__((unused))
*user_data) { [...] + len = ssnprintf (tmpquery, sizeof (tmpquery),
"INSERT INTO data " +
"(timestamp,host_id,plugin_id,plugin_instance,type_id,typeinstance,dataset_id,value)"
+ "VALUES (%.3f,%d,%d,'%s',%d,'%s',%d,%%lf)", CDTIME_T_TO_DOUBLE
(vl->time), host_id, + plugin_id, vl->plugin_instance, type_id,
vl->type_instance, dataset_id );
You really should prepare this statement. Parsing this several
thousand
times per second will be a major performance problem.
+if (dso->type == DS_TYPE_GAUGE) {
+ len = ssnprintf (query, sizeof (query), tmpquery,
vl->values[i].gauge);
+} else {
+ if (rates == NULL) {
+ rates = uc_get_rate (ds, vl);
+ }
+ if (isnan(rates[i])) { continue; }
+ len = ssnprintf (query, sizeof (query), tmpquery, rates[i]);
+}
+//INFO("toto: %d", toto);
+pthread_mutex_lock(&mutex);
+DEBUG("write_mysql plugin: %s", query);
+mysql_real_query(conn, query, len);
+pthread_mutex_unlock(&mutex);
So you're inserting each data source in a
element? How can a client distinguish between the different data
sources?
At least, you should add a "ds_index" column or so. Thanks again and
best
There are dataset_id and type_id in `dataset` for that. Here a query
sample to get
all metric in a type in right order :
select data.timestamp AS date, dataset.name AS dataset_name, data.value
AS value
from data, host, plugin, type, dataset
where host.id = data.host_id
and plugin.id = data.plugin_id
and type.id = data.type_id
and data.dataset_id = dataset.id
and host.name = 'eqds3pcold001'
and plugin.name = 'interface'
and data.plugin_instance = 'eth0'
and type.name = 'if_octets'
In my case, write_mysql is used to make, for example, sum of cpu used
on several thousand
of server and re-inject it in collectd to graph it.
I've made a more flexible than Ganglia application based on Collectd at
work. My compagny
allow me to share all my work on collectd in GPL like licence. I will
Release a beta of
my unamed application soon a possible.
Regards,
Cyril Feraudet
_______________________________________________
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd
