@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.