Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
@miconda commented on this pull request.
In src/modules/db_redis/db_redis_mod.c:
> @@ -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};
In src/modules/db_redis/redis_connection.c:
> @@ -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.
In src/modules/db_redis/redis_connection.c:
> +
+ 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.
In src/modules/db_redis/doc/db_redis_admin.xml:
> @@ -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)?
In src/modules/db_redis/doc/db_redis_admin.xml:
> + </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.
In src/modules/db_redis/doc/db_redis_admin.xml:
> @@ -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, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.