[sr-dev] [kamailio/kamailio] Fix redis pipeline (#1102)

Paul Claudiu Boriga notifications at github.com
Fri Apr 28 09:16:12 CEST 2017


Hello, I have found some bugs with the original commit that adds redis_pipe_cmd. If a command to the server fails, this may leave the redisContext in an invalid state, and this means that a reconnect to the server needs to be performed. 

The problem is that when reconnecting,  a new context is created, and all the contents of old  redisContext is lost, including the hiredis output buffer (obuf) which stored the piped commands. This means that all the commands are lost, and when a second retry is made this contains no commands and may fail, leaving the redisContext in an invalid state. This can in some cases break the REDIS pipelining feature, and cause all commands sent using redis_pipe_cmd()/redis_execute() to fail from that point onwards.

The solution is to make sure that the connection is active when adding commands to the hiredis output buffer. For this, commands are stored in a separate buffer, and are only added to the redisContext when actually sending the command (when calling redis_execute). This way it is possible to  see if any errors exist by checking the err member of the redisContext structure before appending the commands, and also commands are not lost since they are stored separately and if reconnecting the new commands can be appended to the output buffer of the new redisContext.

Commands are stored in buffers using the sds format hiredis offers (by calling redisFormatCommand  function offered by hiredis). This way they can be directly appended to the output buffer using redisAppendFormattedCommand. 

There is a problem on older hiredis versions (older than 0.12) because redisAppendFormattedCommand was not exported to the library API, so for older hiredis version the buffers are appended in our code. 
We tested the code using hiredis library version 0.10.1 and 0.12.1 and there is no problem with this approach.

In the second commit we fixed a memory leak when re-using reply ids in the same pipeline command sequence occurs.
You can view, comment on, or merge this pull request online at:

  https://github.com/kamailio/kamailio/pull/1102

-- Commit Summary --

  * ndb_redis: fix connection problems with pipelining
  * ndb_redis: fix memory leak

-- File Changes --

    M src/modules/ndb_redis/redis_client.c (118)
    M src/modules/ndb_redis/redis_client.h (11)

-- Patch Links --

https://github.com/kamailio/kamailio/pull/1102.patch
https://github.com/kamailio/kamailio/pull/1102.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1102
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kamailio.org/pipermail/sr-dev/attachments/20170428/2a3d1e35/attachment.html>


More information about the sr-dev mailing list