@miconda commented on this pull request.
> @@ -42,6 +42,11 @@ MODULE_VERSION
str redis_keys = str_init("");
str redis_schema_path = str_init(SHARE_DIR "db_redis/kamailio");
int db_redis_verbosity = 1;
+int db_redis_use_hashes = 0;
+int db_redis_expires = 0;
+
+str expires = str_init("");
+char expires_buf[20] = {0};
These should be also prefixed with the module name, `expires` is quite common
name in SIP and other libs, being declared with `extern` in other files, the
symbol can conflict and be resolved to other globals. Like:
```
str db_redis_expires_str = str_init("");
char db_redis_expires_buf[20] = {0};
```
> @@ -315,6 +328,38 @@ int db_redis_connect(km_redis_con_t *con)
strcpy(con->srem_key_lua, reply->str);
freeReplyObject(reply);
reply = NULL;
+
+ if(expires.len) {
+ reply = redisCommand(con->con, "INFO server");
+ if(!reply) {
+ LM_ERR("failed to get INFO from Redis server\n");
+ goto err;
+ }
+
+ char *version_str = strstr(reply->str, "redis_version:");
It is recommended that variables are declared at the beginning of the function
or of the block to work fine with older C versions.
> +
+ if(expires.len) {
+ reply = redisCommand(con->con, "INFO server");
+ if(!reply) {
+ LM_ERR("failed to get INFO from Redis server\n");
+ goto err;
+ }
+
+ char *version_str = strstr(reply->str, "redis_version:");
+ if(!version_str) {
+ LM_ERR("Redis version not found in INFO reply\n");
+ goto err;
+ }
+
+ version_str += strlen("redis_version:"); // Skip past the field
name
+ int major = 0, minor = 0, patch = 0;
It is recommended that variables are declared at the beginning of the function
or of the block to work fine with older C versions.
> @@ -243,6 +243,52 @@ modparam("db_redis", "opt_tls", 1)
</example>
</section>
+ <section id="db_redis.p.use_redis_hashes">
+ <title><varname>use_redis_hashes</varname> (int)</title>
+ <para>
+ Controls whether to use Redis hashes for
storing the records in the database.
+ If 0, sets are used (which is the default).
+ </para>
+ <para>
+ Motivation of hashes is the implementation of
HEXPIRE command in Redis,
+ available since Redis v 7.4.0 onwards, which
allows expiring individual
+ keys inside. Hashes do bring the disadvantage
of storing DUMMY values,
+ since only the keys are useful for us.
Is it a reason to store exactly `DUMMY`, or it was just a random pick? Maybe
this value should be made a mod param (e.g., `hash_value`)?
> + </para>
+ <para>
+ Default value: 0.
+ </para>
+ <example>
+ <title>Enabling redis hashes</title>
+ <programlisting format="linespecific">
+...
+modparam("db_redis", "use_redis_hashes", 1)
+...
+ </programlisting>
+ </example>
+ </section>
+
+ <section id="db_redis.p.expires">
+ <title><varname>expires</varname> (int)</title>
To be more clear that is related to the hash table, I would propose to name
this modparam `hash_expires`.
> @@ -243,6 +243,52 @@ modparam("db_redis", "opt_tls", 1)
</example>
</section>
+ <section id="db_redis.p.use_redis_hashes">
+ <title><varname>use_redis_hashes</varname> (int)</title>
+ <para>
I am not sure if in the future would be another need for a different value, so
it is just a suggestion to name this modparam `hash_mode`, so another value
(e.g., `2`) can be considered instead of adding a new parameter in the future.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/4306#pullrequestreview-2979139311
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/4306/review/2979139...@github.com>_______________________________________________
Kamailio - Development Mailing List -- sr-dev@lists.kamailio.org
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org
Important: keep the mailing list in the recipients, do not reply only to the
sender!
