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

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