[dev.icinga.com #12322] Exceptions might be better than exit in IDO

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/12322

Created by ip1981 on 2016-08-06 13:13:56 +00:00

Assignee: (none)
Status: New
Target Version: (none)
Last Update: 2016-08-08 14:26:38 +00:00 (in Redmine)

Icinga Version: 2.4.10
Backport?: Not yet backported
Include in Changelog: 1

---

I noticed a kind of race on new installation, when icinga and mysql database are get initialized in parallel:

Credentials are valid (icinga can connect to the database), tables exist,
but version info is not yet inserted (is it done at the end of the SQL file).

Throwing an exception might be better choice just like it's done for failed connection,
to avoid restarts

From https://github.com/Icinga/icinga2/blob/master/lib/db\_ido\_mysql/idomysqlconnection.cpp:

    String dbVersionName = "idoutils";
    result = Query("SELECT version FROM " + GetTablePrefix() + "dbversion WHERE name='" + Escape(dbVersionName) + "'");

    row = FetchRow(result);

    if (!row) {
        mysql_close(&m_Connection);
        SetConnected(false);

        Log(LogCritical, "IdoMysqlConnection", "Schema does not provide any valid version! Verify your schema installation.");

        Application::Exit(EXIT_FAILURE);
    }

[icinga-migration]

Updated by mfriedrich on 2016-08-08 14:16:55 +00:00

Upgrading the database schema should involve a daemon stop for icinga2. I'm not exactly sure if exceptions instead of exit() would entirely solve the issue - what if the user never updates the schema on package upgrade, ignores the syslog and doesn't have any ido health check applied?

[icinga-migration]

Updated by ip1981 on 2016-08-08 14:26:38 +00:00

ignores the syslog and doesn't have any ido health check applied

wut? :-)

Now seriously, I don't see much difference between access denied to the database (exception) and the database is broken (exit).
Another reason to avoid exit is that it's used within shared library, this is neat-picking, but still.

While the database may be not usable for some reason, icinga still can do its work, instead of restarting every minute (and skipping most of checks).
This is matter of survivability. Correctly me if it's not true.

[dnsmichi]

There's pros and cons. If you send in a PR, we'll look into merging it.

[dnsmichi]

If tests work fine, I'm considering this for the next release.

[dnsmichi]

MariaDB [icinga]> show create table icinga_dbversion\G
*************************** 1. row ***************************
       Table: icinga_dbversion
Create Table: CREATE TABLE `icinga_dbversion` (
  `dbversion_id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
  `name` varchar(10) CHARACTER SET latin1 DEFAULT '',
  `version` varchar(10) CHARACTER SET latin1 DEFAULT '',
  `create_time` timestamp NULL DEFAULT NULL,
  `modify_time` timestamp NULL DEFAULT NULL,
  PRIMARY KEY (`dbversion_id`),
  UNIQUE KEY `dbversion` (`name`)
) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8
1 row in set (0.00 sec)

MariaDB [icinga]> update icinga_dbversion set version='1.13.3';
Query OK, 1 row affected (0.02 sec)
Rows matched: 1  Changed: 1  Warnings: 0

[2017-03-28 10:14:03 +0200] information/IdoMysqlConnection: 'ido-mysql' resumed.
[2017-03-28 10:14:04 +0200] critical/IdoMysqlConnection: Schema version '1.13.3' does not match the required version '1.14.2' (or newer)! Please check the upgrade documentation.
Context:
	(0) Reconnecting to MySQL IDO database 'ido-mysql'

[2017-03-28 10:14:04 +0200] critical/IdoMysqlConnection: Exception during database operation: Verify that your database is operational!
[2017-03-28 10:14:13 +0200] critical/IdoMysqlConnection: Schema version '1.13.3' does not match the required version '1.14.2' (or newer)! Please check the upgrade documentation.
Context:
	(0) Reconnecting to MySQL IDO database 'ido-mysql'

[2017-03-28 10:14:13 +0200] critical/IdoMysqlConnection: Exception during database operation: Verify that your database is operational!