Database reconnection can cause errors to be reported incorrectly

[eriksejr]

After an upgrade to 1.2.25 I also pulled the develop branch of thold from git and updated it. I started to have problems with thold_daemon not doing anything. These problems showed up as cacti constantly sending me emails that the daemon process was not running, when in fact it was. It just was never updating the "heartbeat" value in the settings table in the database.

Debugging the daemon by running it with --debug --foreground revealed that it constantly said there was no connection to the database, but everything else cacti-related was working just fine, so I dug deeper...

I noticed that in the main loop of thold_daemon.php, you call db_check_reconnect() from lib/database.php and use the return value in a conditional:

while (true) {
        if (db_check_reconnect()) {
           ... do stuff
        } else {
                thold_daemon_debug('WARNING: No database connection.  Sleeping for 60 seconds.');

                sleep(60);
        }
}

Forgive me if I missed it, but reading the code of db_check_reconnect() in lib/databases.php it seems to me that this function does not ever return anything. Based on the little I know about PHP, if a function returns without an explicit return value then the implicit return value is NULL.

In which case this above conditional is if(null) which will always evaluate to false, causing thold_daemon to never actually do anything but complain about a lack of a database connection and sleep for 60 seconds.

To test this I changed the conditional to if (is_null(db_check_reconnect())) and thold_daemon now appears to be working. Is this a bug or did I jump the gun using the develop branch of thold in combination with the release version of Cacti 1.2.25?

[TheWitness]

That's a big whoopsie.

[TheWitness]

If you grab the latest lib/database.php from the 1.2.x branch of Cacti. This should be fixed now.

[eriksejr]

Thanks, I have patched my database.php and so far so good on this front, though I hit an issue when thold tried to send some notifications overnight - I opened a new ticket about that under the thold plugin

[TheWitness]

Okay cool.