Select the next _random_ endpoint as the new backup.

aclisp · Apr 2, 2017 · b6a2054 · b6a2054
1 parent 595b543
commit b6a2054
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 25 deletions.
diff --git a/.gitignore b/.gitignore
@@ -3,3 +3,5 @@
 *.o
 *.out
 *.exe
+log/
+*.dSYM
diff --git a/Makefile b/Makefile
@@ -25,7 +25,7 @@ hiredispool.o: hiredispool.c hiredispool.h log.h hiredis/hiredis.h \
   hiredis/read.h hiredis/sds.h
 log.o: log.c log.h
 RedisClient.o: RedisClient.cpp RedisClient.h hiredispool.h \
-  hiredis/hiredis.h hiredis/read.h hiredis/sds.h
+  hiredis/hiredis.h hiredis/read.h hiredis/sds.h log.h
 
 $(STLIBNAME): $(OBJ)
 	$(STLIB_MAKE_CMD) $(OBJ)
@@ -42,6 +42,9 @@ test_hiredispool.exe: test_hiredispool.cpp hiredispool.h log.h $(STLIBNAME)
 test_RedisClient.exe: test_RedisClient.cpp RedisClient.h hiredispool.h log.h $(STLIBNAME)
 	$(CXX) -std=c++11 -o $@ $(REAL_CXXFLAGS) -I. $< $(STLIBNAME) $(REAL_LDFLAGS)
 
+test_RedisClient2.exe: test_RedisClient2.cpp RedisClient.h hiredispool.h log.h $(STLIBNAME)
+	$(CXX) -std=c++11 -o $@ $(REAL_CXXFLAGS) -I. $< $(STLIBNAME) $(REAL_LDFLAGS)
+
 .c.o:
 	$(CC) -std=c99 -c $(REAL_CFLAGS) $<
 

diff --git a/RedisClient.cpp b/RedisClient.cpp
@@ -9,8 +9,8 @@ using namespace std;
 
 RedisReplyPtr RedisClient::redisCommand(const char *format, ...)
 {
-    va_list ap;
     RedisReplyPtr reply;
+    va_list ap;
 
     va_start(ap, format);
     reply = redisvCommand(format, ap);

diff --git a/RedisClient.h b/RedisClient.h
@@ -58,52 +58,52 @@ struct RedisReplyRef
 class RedisReplyPtr
 {
 public:
-    explicit RedisReplyPtr(void* _reply = 0) : reply((redisReply*)_reply) {}
+    explicit RedisReplyPtr(void* _p = 0) : p((redisReply*)_p) {}
     ~RedisReplyPtr() {
-        //printf("freeReplyObject %p\n", (void*)reply);
-        freeReplyObject(reply);
+        //printf("freeReplyObject %p\n", (void*)p);
+        freeReplyObject(p);
     }
 
     // release ownership of the managed object
     redisReply* release() {
-        redisReply* temp = reply;
-        reply = NULL;
+        redisReply* temp = p;
+        p = NULL;
         return temp;
     }
 
     // transfer ownership
     RedisReplyPtr(RedisReplyPtr& other) {
-        reply = other.release();
+        p = other.release();
     }
     RedisReplyPtr& operator=(RedisReplyPtr& other) {
         if (this == &other)
             return *this;
         RedisReplyPtr temp(release());
-        reply = other.release();
+        p = other.release();
         return *this;
     }
 
     // automatic conversions
     RedisReplyPtr(RedisReplyRef _ref) {
-        reply = _ref.p;
+        p = _ref.p;
     }
     RedisReplyPtr& operator=(RedisReplyRef _ref) {
-        if (reply == _ref.p )
+        if (p == _ref.p)
             return *this;
         RedisReplyPtr temp(release());
-        reply = _ref.p;
+        p = _ref.p;
         return *this;
     }
     operator RedisReplyRef() { return RedisReplyRef(release()); }
 
-    bool notNull() const { return (reply != 0); }
-    bool isNull() const { return (reply == 0); }
-    redisReply* get() const { return reply; }
-    redisReply* operator->() const { return reply; }
-    redisReply& operator*() const { return *reply; }
+    bool notNull() const { return (p != 0); }
+    bool isNull() const { return (p == 0); }
+    redisReply* get() const { return p; }
+    redisReply* operator->() const { return p; }
+    redisReply& operator*() const { return *p; }
 
 private:
-    redisReply* reply;
+    redisReply* p;
 };
 
 // RedisClient provides a threadsafe redis client

diff --git a/hiredispool.c b/hiredispool.c
@@ -217,8 +217,12 @@ static int connect_single_socket(REDIS_SOCKET *redisocket, REDIS_INSTANCE *inst)
                 __func__, redisocket->id, redisocket->backup);
             redisocket->conn = c;
             redisocket->state = sockconnected;
-            /* Select the next endpoint as the new backup */
-            redisocket->backup = (redisocket->backup + 1) % inst->config->num_endpoints;
+            if (inst->config->num_endpoints > 1) {
+                /* Select the next _random_ endpoint as the new backup */
+                redisocket->backup = (redisocket->backup + (1 +
+                        rand() % (inst->config->num_endpoints - 1)
+                    )) % inst->config->num_endpoints;
+            }
 
             if (redisSetTimeout(c, timeout[1]) != REDIS_OK) {
                 log_(L_WARN|L_CONS, "%s: Failed to set timeout: blocking-mode: %d, %s",
@@ -275,8 +279,8 @@ static int redis_close_socket(REDIS_INSTANCE *inst, REDIS_SOCKET * redisocket)
 
     (void)inst;
 
-    DEBUG("%s: Closing redis socket %d", __func__,
-           redisocket->id);
+    DEBUG("%s: Closing redis socket =%d #%d @%d", __func__,
+           redisocket->state, redisocket->id, redisocket->backup);
 
     if (redisocket->state == sockconnected) {
         redisFree(redisocket->conn);

diff --git a/test_hiredispool.cpp b/test_hiredispool.cpp
@@ -21,15 +21,16 @@ int main(int argc, char** argv)
     };
     log_set_config(&c);
 
-    REDIS_ENDPOINT endpoints[2] = {
+    REDIS_ENDPOINT endpoints[4] = {
         { "127.0.0.1", 6379 },
         { "127.0.0.1", 6380 },
-        //{ "127.0.0.1", 6381 },
+        { "127.0.0.1", 6381 },
+        { "127.0.0.1", 6382 },
     };
 
     REDIS_CONFIG conf = {
         (REDIS_ENDPOINT*)&endpoints,
-        2,
+        4,
         10000,
         5000,
         20,