Skip to content

Commit

Permalink
Use singleton for db and redis connections (#1112)
Browse files Browse the repository at this point in the history
Required for an upcoming change. Though, I wanted to change this already
a long time ago.

There's absolutely no need to have several open connections to the same
database. Which was the case before, since the connection was
established for each user (class) of the `Database` trait.

But with PDO, queries are by default all serially processed. Only for
MySQL it is possible to change this, by [disabling query
buffering](https://www.php.net/manual/en/mysqlinfo.concepts.buffering.php).

But Icinga DB Web is incompatible anyway, with disabled query buffering,
and so changing this to a singleton is fine in my opinion.

I've deliberately kept the `Database` trait, to not update all usages of
it. In case only the connection is required, the trait still has its
use.
  • Loading branch information
nilmerg authored Dec 17, 2024
2 parents 9c86ab9 + c57298e commit 8cf9302
Show file tree
Hide file tree
Showing 8 changed files with 238 additions and 104 deletions.
3 changes: 3 additions & 0 deletions doc/05-Upgrading.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ The following classes have been deprecated and will be removed in a future relea
* `\Icinga\Module\Icingadb\Command\Object\ScheduleHostDowntimeCommand`
* `\Icinga\Module\Icingadb\Command\Object\ScheduleServiceDowntimeCommand`

The following methods have been deprecated and will be removed in a future release:
* `\Icinga\Module\Icingadb\Common\IcingaRedis::instance()`: Use `\Icinga\Module\Icingadb\Common\Backend::getRedis()` instead.

## Upgrading to Icinga DB Web v1.1

**Breaking Changes**
Expand Down
168 changes: 168 additions & 0 deletions library/Icingadb/Common/Backend.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
<?php

/* Icinga DB Web | (c) 2024 Icinga GmbH | GPLv2 */

namespace Icinga\Module\Icingadb\Common;

use Icinga\Application\Config as AppConfig;
use Icinga\Data\ResourceFactory;
use Icinga\Module\Icingadb\Model\Schema;
use ipl\Sql\Adapter\Pgsql;
use ipl\Sql\Config as SqlConfig;
use ipl\Sql\Connection;
use ipl\Sql\Expression;
use ipl\Sql\QueryBuilder;
use ipl\Sql\Select;
use PDO;

/**
* Singleton providing access to the Icinga DB and Redis
*/
final class Backend
{
/** @var ?Connection */
private static $db;

/** @var ?int */
private static $schemaVersion;

/** @var ?IcingaRedis */
private static $redis;

/**
* Set the connection to the Icinga DB
*
* Usually not required, as the connection is created on demand. Useful for testing.
*
* @param Connection $db
*
* @return void
*/
public static function setDb(Connection $db): void
{
self::$db = $db;
}

/**
* Get the connection to the Icinga DB
*
* @return Connection
*/
public static function getDb(): Connection
{
if (self::$db === null) {
$config = new SqlConfig(ResourceFactory::getResourceConfig(
AppConfig::module('icingadb')->get('icingadb', 'resource')
));

$config->options = [PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_OBJ];
if ($config->db === 'mysql') {
$config->options[PDO::MYSQL_ATTR_INIT_COMMAND] = "SET SESSION SQL_MODE='STRICT_TRANS_TABLES"
. ",NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'";
}

self::$db = new Connection($config);

$adapter = self::$db->getAdapter();
if ($adapter instanceof Pgsql) {
$quoted = $adapter->quoteIdentifier('user');
self::$db->getQueryBuilder()
->on(QueryBuilder::ON_SELECT_ASSEMBLED, function (&$sql) use ($quoted) {
// user is a reserved key word in PostgreSQL, so we need to quote it.
// TODO(lippserd): This is pretty hacky,
// reconsider how to properly implement identifier quoting.
$sql = str_replace(' user ', sprintf(' %s ', $quoted), $sql);
$sql = str_replace(' user.', sprintf(' %s.', $quoted), $sql);
$sql = str_replace('(user.', sprintf('(%s.', $quoted), $sql);
})
->on(QueryBuilder::ON_ASSEMBLE_SELECT, function (Select $select) {
// For SELECT DISTINCT, all ORDER BY columns must appear in SELECT list.
if (! $select->getDistinct() || ! $select->hasOrderBy()) {
return;
}

$candidates = [];
foreach ($select->getOrderBy() as list($columnOrAlias, $_)) {
if ($columnOrAlias instanceof Expression) {
// Expressions can be and include anything,
// also columns that aren't already part of the SELECT list,
// so we're not trying to guess anything here.
// Such expressions must be in the SELECT list if necessary and
// referenced manually with an alias in ORDER BY.
continue;
}

$candidates[$columnOrAlias] = true;
}

foreach ($select->getColumns() as $alias => $column) {
if (is_int($alias)) {
if ($column instanceof Expression) {
// This is the complement to the above consideration.
// If it is an unaliased expression, ignore it.
continue;
}
} else {
unset($candidates[$alias]);
}

if (! $column instanceof Expression) {
unset($candidates[$column]);
}
}

if (! empty($candidates)) {
$select->columns(array_keys($candidates));
}
});
}
}

return self::$db;
}

/**
* Get the schema version of the Icinga DB
*
* @return int
*/
public static function getDbSchemaVersion(): int
{
if (self::$schemaVersion === null) {
self::$schemaVersion = Schema::on(self::getDb())
->columns('version')
->first()
->version ?? 0;
}

return self::$schemaVersion;
}

/**
* Set the connection to the Icinga Redis
*
* Usually not required, as the connection is created on demand. Useful for testing.
*
* @param IcingaRedis $redis
*
* @return void
*/
public static function setRedis(IcingaRedis $redis): void
{
self::$redis = $redis;
}

/**
* Get the connection to the Icinga Redis
*
* @return IcingaRedis
*/
public static function getRedis(): IcingaRedis
{
if (self::$redis === null) {
self::$redis = new IcingaRedis();
}

return self::$redis;
}
}
84 changes: 1 addition & 83 deletions library/Icingadb/Common/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,99 +4,17 @@

namespace Icinga\Module\Icingadb\Common;

use Icinga\Application\Config as AppConfig;
use Icinga\Data\ResourceFactory;
use Icinga\Exception\ConfigurationError;
use ipl\Sql\Adapter\Pgsql;
use ipl\Sql\Config as SqlConfig;
use ipl\Sql\Connection;
use ipl\Sql\Expression;
use ipl\Sql\QueryBuilder;
use ipl\Sql\Select;
use PDO;

trait Database
{
/** @var Connection Connection to the Icinga database */
private $db;

/**
* Get the connection to the Icinga database
*
* @return Connection
*
* @throws ConfigurationError If the related resource configuration does not exist
*/
public function getDb(): Connection
{
if ($this->db === null) {
$config = new SqlConfig(ResourceFactory::getResourceConfig(
AppConfig::module('icingadb')->get('icingadb', 'resource')
));

$config->options = [PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_OBJ];
if ($config->db === 'mysql') {
$config->options[PDO::MYSQL_ATTR_INIT_COMMAND] = "SET SESSION SQL_MODE='STRICT_TRANS_TABLES"
. ",NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'";
}

$this->db = new Connection($config);

$adapter = $this->db->getAdapter();
if ($adapter instanceof Pgsql) {
$quoted = $adapter->quoteIdentifier('user');
$this->db->getQueryBuilder()
->on(QueryBuilder::ON_SELECT_ASSEMBLED, function (&$sql) use ($quoted) {
// user is a reserved key word in PostgreSQL, so we need to quote it.
// TODO(lippserd): This is pretty hacky,
// reconsider how to properly implement identifier quoting.
$sql = str_replace(' user ', sprintf(' %s ', $quoted), $sql);
$sql = str_replace(' user.', sprintf(' %s.', $quoted), $sql);
$sql = str_replace('(user.', sprintf('(%s.', $quoted), $sql);
})
->on(QueryBuilder::ON_ASSEMBLE_SELECT, function (Select $select) {
// For SELECT DISTINCT, all ORDER BY columns must appear in SELECT list.
if (! $select->getDistinct() || ! $select->hasOrderBy()) {
return;
}

$candidates = [];
foreach ($select->getOrderBy() as list($columnOrAlias, $_)) {
if ($columnOrAlias instanceof Expression) {
// Expressions can be and include anything,
// also columns that aren't already part of the SELECT list,
// so we're not trying to guess anything here.
// Such expressions must be in the SELECT list if necessary and
// referenced manually with an alias in ORDER BY.
continue;
}

$candidates[$columnOrAlias] = true;
}

foreach ($select->getColumns() as $alias => $column) {
if (is_int($alias)) {
if ($column instanceof Expression) {
// This is the complement to the above consideration.
// If it is an unaliased expression, ignore it.
continue;
}
} else {
unset($candidates[$alias]);
}

if (! $column instanceof Expression) {
unset($candidates[$column]);
}
}

if (! empty($candidates)) {
$select->columns(array_keys($candidates));
}
});
}
}

return $this->db;
return Backend::getDb();
}
}
30 changes: 11 additions & 19 deletions library/Icingadb/Common/IcingaRedis.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@

class IcingaRedis
{
/** @var static The singleton */
protected static $instance;

/** @var Redis Connection to the Icinga Redis */
private $redis;

Expand All @@ -24,35 +21,30 @@ class IcingaRedis
/**
* Get the singleton
*
* @deprecated Use {@see Backend::getRedis()} instead
* @return static
*/
public static function instance(): self
{
if (self::$instance === null) {
self::$instance = new static();
}

return self::$instance;
return Backend::getRedis();
}

/**
* Get whether Redis is unavailable
*
* @return bool
*/
public static function isUnavailable(): bool
public function isUnavailable(): bool
{
$self = self::instance();

if (! $self->redisUnavailable && $self->redis === null) {
if (! $this->redisUnavailable && $this->redis === null) {
try {
$self->getConnection();
$this->getConnection();
} catch (Exception $_) {
// getConnection already logs the error
}
}

return $self->redisUnavailable;
return $this->redisUnavailable;
}

/**
Expand Down Expand Up @@ -126,7 +118,7 @@ public function getConnection(): Redis
*/
public static function fetchHostState(array $ids, array $columns): Generator
{
return self::fetchState('icinga:host:state', $ids, $columns);
return Backend::getRedis()->fetchState('icinga:host:state', $ids, $columns);
}

/**
Expand All @@ -139,7 +131,7 @@ public static function fetchHostState(array $ids, array $columns): Generator
*/
public static function fetchServiceState(array $ids, array $columns): Generator
{
return self::fetchState('icinga:service:state', $ids, $columns);
return Backend::getRedis()->fetchState('icinga:service:state', $ids, $columns);
}

/**
Expand All @@ -151,10 +143,10 @@ public static function fetchServiceState(array $ids, array $columns): Generator
*
* @return Generator
*/
protected static function fetchState(string $key, array $ids, array $columns): Generator
protected function fetchState(string $key, array $ids, array $columns): Generator
{
try {
$results = self::instance()->getConnection()->hmget($key, $ids);
$results = $this->getConnection()->hmget($key, $ids);
} catch (Exception $_) {
// The error has already been logged elsewhere
return;
Expand Down Expand Up @@ -192,7 +184,7 @@ protected static function fetchState(string $key, array $ids, array $columns): G
public static function getLastIcingaHeartbeat(Redis $redis = null)
{
if ($redis === null) {
$redis = self::instance()->getConnection();
$redis = Backend::getRedis()->getConnection();
}

// Predis doesn't support streams (yet).
Expand Down
2 changes: 1 addition & 1 deletion library/Icingadb/Common/ObjectInspectionDetail.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ protected function createRedisInfo(): array
$title = new HtmlElement('h2', null, Text::create(t('Volatile State Details')));

try {
$json = IcingaRedis::instance()->getConnection()
$json = Backend::getRedis()->getConnection()
->hGet("icinga:{$this->object->getTableName()}:state", bin2hex($this->object->id));
} catch (Exception $e) {
return [$title, sprintf('Failed to load redis data: %s', $e->getMessage())];
Expand Down
Loading

0 comments on commit 8cf9302

Please sign in to comment.