-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Redis DB Index should be configurable #676
Comments
Could you please elaborate on your use case? AFAIK, it is recommended to use a separate Redis instance per application. This is also backed by the Redis documentation for the As a possible use case for Redis database support in Icinga DB, I would see the use of an existing Redis server. However, when you are already in a hosting scenario where you can install Icinga DB, I don't see a reason why you couldn't install another Redis. This might even improve performance due to Redis "mostly single-threaded" architecture, which suggests "launch[ing] several Redis instances to scale out on several cores" in its Redis benchmark, Pitfalls and misconceptions documentation. |
We run Icinga (without Redis currently) along with another application (which interfaces with Icinga) and in this application already uses Redis, currently on DB 0. We are looking to upgrade Icinga and potentially enable Redis for it and I thought instead of going through the trouble of setting up a another Redis (with replication, backup and all that fuss), we could just use another Redis DB to avoid key conflicts. Another option would be if Icingadb allows to configure a key prefix, e.g. A single redis thread can easily handle upwards of 20k queries/sec, so I don't think performance is a problem, unless icingadb expects such query amounts, but at this point, one would better invested by using a multithreaded Redis fork like Dragonfly. |
Depending on your setup, a Redis replication might not be necessary. Please take a look at the Distributed Setup manual of Icinga DB, if this fits your use case. In an High Availability Setup with two Icinga DB instances, each one needs its own Redis instance1.
Creating namespaces by prefixing keys seems like an ugly hack, which might result in nasty issues on the long run. If one really wants to reuse an existing Redis setup, databases might be the way to go2. However, as I wrote earlier, multiple Redis instances might be a better separation and might even perform better, as others stated before me. Due to the fact that using multiple databases would exclude Redis Cluster and I cannot see why Icinga DB would require multiple databases, in my opinion nothing stands against allowing configuring other databases next to database zero. I would create a patch, as this change should be trivial. Unless @lippserd or someone else familiar with the code objects. Edit: BTW, there is no hard limit of sixteen databases. This is just the default in the configuration. Footnotes
|
Ah, so as per https://icinga.com/docs/icinga-db/latest/doc/05-Distributed-Setups/, Redis is local to each Icinga master node, so in essence Redis is used as a local in-memory database similar to badger. If above is correct then HA will likely not work correctly with a shared Redis DB, because the application expects Redis to be a dedicated resource? I'll leave it up to you if you want to add this
Good to know, so the option should be made a uint64 likely. |
Yes. The Redis is used for communication between an Icinga 2 and the Icinga DB1. Think about it as both a message queue and storage for "volatile data like check results"2.
In my understanding of Icinga DB, this is correct.
To make the connection work, this must be configured both for Icinga 2 as well as for Icinga DB. Fortunately, Icinga 2 already supports a DB selection. There's the undocumented Getting away from this theoretical level, I have put together a trivial testing scenario. The shortest possible patch against Icinga DB might look like this: diff --git a/pkg/config/redis.go b/pkg/config/redis.go
index 38571e3..14d1b78 100644
--- a/pkg/config/redis.go
+++ b/pkg/config/redis.go
@@ -22,6 +22,7 @@ type Redis struct {
Host string `yaml:"host"`
Port int `yaml:"port" default:"6380"`
Password string `yaml:"password"`
+ Database int `yaml:"database" default:"0"`
TlsOptions TLS `yaml:",inline"`
Options icingaredis.Options `yaml:"options"`
}
@@ -48,7 +49,7 @@ func (r *Redis) NewClient(logger *logging.Logger) (*icingaredis.Client, error) {
options := &redis.Options{
Dialer: dialWithLogging(dialer, logger),
Password: r.Password,
- DB: 0, // Use default DB,
+ DB: r.Database,
ReadTimeout: r.Options.Timeout,
TLSConfig: tlsConfig,
} Afterwards, the Icinga 2 configuration should contain an
…and Icinga DB's configuration should contain the following
This modifications were enough to make my test environment use the sixth Redis database, as I verified through However, as I wrote before and the documentation states, reusing a Redis might only make sense for some scenarios. For a single node setup with a low load, this might make sense. However, in an HA setup, this would only lead to problems, IMHO. Thus, I would love to hear the opinion of the contributors if we should proceed with this.
Currently, Footnotes |
A Redis server might support multiple databases, allowing to use the same Redis for different applications. The database can either be configured for the configuration or later changed via SELECT[0]. By default, the database 0 will be used. While both the used go-redis library and even Icinga 2[1] support this feature, we should expose this here and make this configurable. As discussed in Icinga DB issue #676[2], this is not something one might want to encourage all users to use. However, there are definitely setups where this option might come in handy. [0]: https://redis.io/docs/latest/commands/select/ [1]: https://github.com/Icinga/icinga2/blob/bca1a8447af26313123183efa863f947f9590731/lib/icingadb/icingadb.ti#L23 [2]: Icinga/icingadb#676
Currently this application is hardcoded to use Redis DB index 0 which can lead to conflicts in redis when other applications also use DB 0.
It should be made configurable with a
db
option with default value 0 and possible values 0-15, e.g. 16 databases in total, which is what Redis offers by default.The text was updated successfully, but these errors were encountered: