vipshop / hiredis-vip

Support redis cluster. Maintained and used at vipshop.
BSD 3-Clause "New" or "Revised" License
322 stars 173 forks source link

redisReply is null #116

Open narayanashanubhog opened 5 years ago

narayanashanubhog commented 5 years ago

If i use the below code, i'm getting redisReply is null

struct timeval timeout = { 1, 500000 }; // 1.5 seconds redisClusterContext redis = redisClusterContextInit(); redisClusterSetOptionAddNodes(redis, "127.0.0.1:7000"); redisClusterSetOptionConnectTimeout(redis, timeout); redisClusterConnect2(redis); if (redis != NULL && redis->err) { printf("Error: %s\n",redis->errstr); // handle error exit(-1); } reply = (redisReply) redisClusterCommand(redis, "INFO"); if (reply == NULL){ error(0, "REDIS: INFO reply null to get version failed!"); }

When i add redisClusterSetOptionRouteUseSlots(redis); //The function that has to be called. Its throwing below exception Error: Command(cluster slots) reply error: node ip is not string.

Im using the redis version 5. Thanks in advance.

raghuiisc commented 5 years ago

As pointed in https://github.com/vipshop/hiredis-vip/issues/33 .. INFO is command for cluster node. For this to execute we need to execute rediscommand on node.

I tried the code locally by adding "redisClusterSetOptionRouteUseSlots(redis)" i do not see any issue. For exact error please print "redis->errstr".

If u do not want to use "redisClusterSetOptionRouteUseSlots(redis)", there is a bug in parsing the output of cluster nodes which is causing the issue. To fix, please apply the below patch.

diff --git a/hircluster.c b/hircluster.c index b051936..7538abb 100644 --- a/hircluster.c +++ b/hircluster.c @@ -23,6 +23,8 @@

define IP_PORT_SEPARATOR ":"

+#define PORT_SEPARATOR "@" +

define CLUSTER_ADDRESS_SEPARATOR ","

define CLUSTER_DEFAULT_MAX_REDIRECT_COUNT 5

@@ -534,7 +536,9 @@ static cluster_node node_get_with_nodes( sds node_infos, int info_count, uint8_t role) { sds *ip_port = NULL;

@@ -595,6 +609,11 @@ error: sdsfreesplitres(ip_port, count_ip_port); }

narayana-plivo commented 5 years ago

@raghuiisc :The same above code works in redis version 4.0. But latest version of redis > 5. For any redisClusterCommand, it will give redis reply null

raghuiisc commented 5 years ago

redis reply is NULL, but can you please print "redis->errstr" and exact code u are executing. I tried locally and it was working fine.

narayana-plivo commented 5 years ago

there is no issue in connecting redis cluster. if (redis != NULL && redis->err) { printf("Error: %s\n",redis->errstr); // handle error exit(-1); }

Only redisClusterCommand is returning Null always for any command you execute like hmset,hmget etc etc