This module implements a generic db driver for kamailio. It requires a "schema" and "key" definition of "tables" and corresponding keys for redis in the kamailio config file, otherwise it's supposed to work with every module.
Implemented methods are query (w/o order-by), insert, update, delete.
Tested with usrloc and acc.
<!-- Kamailio Pull Request Template -->
<!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception -->
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [x] Each component has a single commit (if not, squash them into one commit) - [ ] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [ ] PR should be backported to stable branches - [x] Tested changes locally - [x] Related to issue #1137
#### Description <!-- Describe your changes in detail --> This module implements a generic db driver for redis, to be used by any module (with the help of some schema configuration in the config file).
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/1432
-- Commit Summary --
* db_redis: Implement db_redis generic db driver
-- File Changes --
M src/Makefile.groups (2) A src/modules/db_redis/Makefile (37) A src/modules/db_redis/README (199) A src/modules/db_redis/db_redis_mod.c (109) A src/modules/db_redis/db_redis_mod.h (31) A src/modules/db_redis/doc/Makefile (4) A src/modules/db_redis/doc/db_redis.xml (37) A src/modules/db_redis/doc/db_redis_admin.xml (190) A src/modules/db_redis/redis_connection.c (287) A src/modules/db_redis/redis_connection.h (71) A src/modules/db_redis/redis_dbase.c (2056) A src/modules/db_redis/redis_dbase.h (92) A src/modules/db_redis/redis_table.c (697) A src/modules/db_redis/redis_table.h (63)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/1432.patch https://github.com/kamailio/kamailio/pull/1432.diff
@agranig pushed 1 commit.
dcb8611 db_redis: Cleanups and fixes
@agranig pushed 1 commit.
2378fb9 db_redis: Fix missed freeing redis reply objects
IMO, this one can be merged.
I haven't had the time for an in depth analyzis, spotted a small type in the readme, ndb_redis can be used to send any command to redis.
Then, as you expressed in some comments, having schema definition in a file may make possible to generate it from the xml db schema definitions from src/lib/srdb1/schema, in a simialar matter done for `db_text` or `db_mongodb`. If the parameter is kept, might be good to use function callbacks for it in order to allow setting it many times without overwriting previous value, otherwise its value can become too big for a single modparm (e.g., like done for htable parameter for htable module). Anyhow, these are enhancements that can be done over the time, as I said, merging at this phase is ok from my point of view, it will make it easier for others to test and improve its stability.
miconda approved this pull request.
@agranig pushed 1 commit.
609c350 db_redis: Use schema files and improve keys def
Hi @miconda, I've improved the module with the suggestions you made:
* utilize xml schema definitions and load them from files to avoid having to define them as mod params * allow multiple keys mod params definitions to make config more readable * reconnect improvements * documentation fixes and improvements
Merged #1432.
A quick note that I updated the path for including hiredis.h:
``` diff --git a/src/modules/db_redis/redis_connection.h b/src/modules/db_redis/redis_connection.h index d6ed4e4ec3..35d58e46a4 100644 --- a/src/modules/db_redis/redis_connection.h +++ b/src/modules/db_redis/redis_connection.h @@ -23,7 +23,7 @@ #ifndef _REDIS_CONNECTION_H_ #define _REDIS_CONNECTION_H_
-#include <hiredis.h> +#include <hiredis/hiredis.h>
#include "db_redis_mod.h" ```
On Debian 9 is where the file is located based on pkg-config output, it doesn't compile otherwise (same on MacOS with macports).
I looked in ndb_redis module and has the same path. If you have it in your system working without `<hiredis/hiredis.h>`, then we need to do some defines to set the proper path so it works on common distros.
Thanks again for this contribution!
The relevant question - how to implement authentication while connecting to redis storage? I read this document: https://kamailio.org/docs/modules/devel/modules/db_redis.html and didn't find any reference in it.
As I understood we use following syntax to connect to the redis db: redis://[username]@host:port/database
But authentication capability is not provided as I can see.
Of course there is an excellent module named ndb_redis. But my main goal is to perform authentication with cnxcc module, which is not able to do that (Carlos Ruiz Díaz will add this possibility later?).
Please correct me if I'm wrong.
Try redis://password@host:port/db and let me know if it works for you.
Well, for that moment it doesn't work.
My configuration is as follows:
#!define DBURL_CNXCC "redis://here_my_password@redis.domain.com:6379/1" loadmodule "db_redis.so" modparam("db_redis", "schema_path", "/usr/share/kamailio/db_redis/kamailio")
modparam("cnxcc", "redis", DBURL_CNXCC)
I think this is wrong way to do that. cnxcc module uses redis db as htable and stores real-time values (like credit, call duration etc.) that exist only when call is up for certain customer, then it drops whole key.
Should I use: 'modparam("db_redis", "keys"...' ? Or db_redis is to be improved to work with cnxcc? (cuz it uses not "db_url" modparam but "redis" modparam).
@zenichev - cnxcc connects directly to redis, doesn't use a DB connector at all, such as db_mysql or db_redis. In other words, there is no relation between db_redis and cnxcc.
@miconda - as I mentioned above, I need working authentication for cnxcc (that is not able to work with authentication). So that, I'm trying to find a way to do that, this time by db_redis. Do you think this is a wrong way?
@zenichev - as said, db_redis cannot be used with cnxcc. Start a discussion on sr-users@lists.kamailio.org with what you need to achieve. The issue tracker is only for bug reports, pull requests should be discussed in the context of the committed code: to review, improve, etc... it is not how to use the new features.
@miconda Ok I understood it clearly. Thank you for your answer. Will try to start discussion on sr-users list.
-
Andreas Granig -
Daniel-Constantin Mierla
-
zenichev