dbi plugin mysql dbi_result_next_row failed

[GeraldWodni]

• Version of collectd: collectd 5.7.1.git
• Operating system / distribution: Debian stretch
• SQL: 10.1.26-MariaDB-0+deb9u1

Expected behavior

Write query results into gauge

Actual behavior

system log:

Aug 9 18:14:56 xxx collectd[16951]: plugin_load: plugin "dbi" successfully loaded.
...
Aug  9 18:14:56 xxx collectd[16951]: dbi plugin: cdbi_read_database_query (dbxxx, db_status): dbi_result_next_row failed: dbi_conn_error failed with status -6.

This error repeats. Looking through the headers it seams to be an DBI_ERROR_BADIDX.
It seems that connecting as well as querying work fine, but for some reason attempting to read the next row fails. The query has been simplified My guess is the error occurs here:

collectd/src/dbi.c

Lines 608 to 615 in 065f3ea

	if (dbi_conn_error(db->connection, NULL) != 0) {
	char errbuf[1024];
	WARNING("dbi plugin: cdbi_read_database_query (%s, %s): "
	"dbi_result_next_row failed: %s.",
	db->name, udb_query_get_name(q),
	cdbi_strerror(db->connection, errbuf, sizeof(errbuf)));
	}
	break;

Steps to reproduce

collectd.conf (query has been simplified, different number of result lines tried, but even this yields an error):

<Plugin dbi>
        <Query "xxx_status">
                Statement "SELECT 'damons_ok' AS name, '11' as value"
                <Result>
                        Type "gauge"
                        InstancesFrom "name"
                        ValuesFrom "value"
                </Result>
        </Query>

        <Database "xxx_db">
                Driver "mysql"
                DriverOption "host" "localhost"
                DriverOption "username" "dbxxx"
                DriverOption "password" "xyz"
                DriverOption "dbname" "dbxxx"
                SelectDB "dbxxx"
                Query "xxx_status"
        </Database>
</Plugin>

[GeraldWodni]

Sorry, I think I wasn't patient enough.
While the error messages do look menacing, the results are indeed written. However, this warning should still not be shown.

I would suggest using dbi_result_has_next_row (preferred) or break the loop on DBI_ERROR_BADIDX without showing the warning. Otherwise this plugin spams the log.

Thanks for the great software btw, I really enjoy using collectd! ;)

[rpv-tomsk]

Hi!

Otherwise this plugin spams the log.

Agreed, this is very annoying issue. Personally, I encountered some unpleasant situations due to its presence. But I treat this as an issue in libdbi code, not Collectd. And, unfortunately, 'libdbi' is dead project by the fact - it has bugs and memleaks, some of them even fixed in project bugtracker, but no releases for more than 5 years.

I use 'dbi' plugin with 'mysql' and 'sqlite' drivers. The code of Collectd is the same ;-) and there is no dbi_result_next_row failed issue when 'sqlite' is in use (ouch, but there is memleak exists then...).
Due to this fact, I think this is a bug in 'dbi', but I haven't digged it deeply.

I would suggest using dbi_result_has_next_row

I think, unfortunately this highly probably will not work as we expect: dbi_result_next_row, used by Collectd, already does this check.

If you look into dbi code (I used libdbi-0.9 and the same version is in Stretch), then you can find following implementation in libdbi-0.9.0/src/dbi_result.c :

int dbi_result_next_row(dbi_result Result) {
  if (!RESULT) {
    _error_handler(/*RESULT->conn*/ NULL, DBI_ERROR_BADPTR);
    return 0;
  }

  _reset_conn_error(RESULT->conn);

  if (!dbi_result_has_next_row(Result)) {
/*     _error_handler(RESULT->conn, DBI_ERROR_BADIDX); */
    return 0;
  }
  return dbi_result_seek_row(Result, RESULT->currowidx+1);
}

As you can see, there is _reset_conn_error() + dbi_result_has_next_row(). So, dbi_conn_error should return 0 and no error should be reported by Collectd. But it is reported, and, I think, that is a 'small implementation detail' of 'mysql' dbi driver (note again, there is no such message when 'sqlite' driver is used).

[rpv-tomsk]

Ouch. That is clearly Debian issue.
Please see debian/patches/re-enable_call_to_error_handler.patch in Debian package sources, it uncomments the line, which is commented in dbi_result_next_row in upstream.

We have nothing to change in Collectd code.

[rpv-tomsk]

Ok.

1. Debian patch is based on 'fresh' commit in 'libdbi' repo.
2. Uncommented line in dbi_result_next_row matches same line in dbi_result_prev_row and matches documentation, so we can assume this is correct. (However, libdbi code has much other problems, such as endless duplicated checks and incorrect use of unsigned integers).
3. There is no issues in 'mysql' driver of 'libdbi', which are cause of 'error -6'.
4. We will add dbi_result_has_next_row into Collectd code (so all checks will be done three times for each row, but we have no other way)..

[nmamn]

Hi

any plan on releasing this fix into a new version?

Thanks

[rpv-tomsk]

This fix is already in master branch and that is not possible to release new version w/o this commit.