Merge pull request #4 from hamishforbes/master

Only return healthy slaves
ledgetech · Dec 9, 2015 · 7d61a02 · 7d61a02
2 parents 5db29e0 + 485efe6
commit 7d61a02
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 16 deletions.
diff --git a/Makefile b/Makefile
@@ -14,7 +14,7 @@ REDIS_LOG           = /redis.log
 REDIS_PREFIX        = /tmp/redis-
 
 # Overrideable redis test variables
-TEST_REDIS_PORTS              ?= 6379 6380
+TEST_REDIS_PORTS              ?= 6379 6380 6378
 TEST_REDIS_DATABASE           ?= 1
 
 REDIS_FIRST_PORT                    := $(firstword $(TEST_REDIS_PORTS))
@@ -65,7 +65,7 @@ INSTALL         ?= install
 .PHONY: all install test test_all start_redis_instances stop_redis_instances \
 	start_redis_instance stop_redis_instance cleanup_redis_instance flush_db \
 	create_sentinel_config delete_sentinel_config check_ports test_redis \
-	test_sentinel
+	test_sentinel sleep
 
 all: ;
 
@@ -74,7 +74,10 @@ install: all
 	$(INSTALL) lib/resty/redis/*.lua $(DESTDIR)/$(LUA_LIB_DIR)/resty/redis
 
 test: test_redis
-test_all: start_redis_instances test_redis test_sentinel stop_redis_instances
+test_all: start_redis_instances sleep test_redis stop_redis_instances
+
+sleep:
+	sleep 3
 
 start_redis_instances: check_ports create_sentinel_config
 	@$(foreach port,$(TEST_REDIS_PORTS), \
@@ -91,6 +94,7 @@ start_redis_instances: check_ports create_sentinel_config
 		prefix=$(REDIS_PREFIX)$(port) && \
 	) true
 
+
 stop_redis_instances: delete_sentinel_config 
 	-@$(foreach port,$(TEST_REDIS_PORTS) $(TEST_SENTINEL_PORTS), \
 		$(MAKE) stop_redis_instance cleanup_redis_instance port=$(port) \
@@ -140,12 +144,5 @@ test_redis: flush_db
 	$(TEST_REDIS_VARS) $(PROVE) $(TEST_FILE)
 	util/lua-releng
 
-test_sentinel: flush_db
-	$(TEST_SENTINEL_VARS) $(PROVE) $(SENTINEL_TEST_FILE)/01-master_up.t
-	$(REDIS_CLI) shutdown
-	$(TEST_SENTINEL_VARS) $(PROVE) $(SENTINEL_TEST_FILE)/02-master_down.t
-	sleep $(TEST_SENTINEL_PROMOTION_TIME)
-	$(TEST_SENTINEL_VARS) $(PROVE) $(SENTINEL_TEST_FILE)/03-slave_promoted.t
-
 test_leak: flush_db
 	$(TEST_REDIS_VARS) TEST_NGINX_CHECK_LEAK=1 $(PROVE) $(TEST_FILE)
diff --git a/lib/resty/redis/sentinel.lua b/lib/resty/redis/sentinel.lua
@@ -36,8 +36,11 @@ function _M.get_slaves(sentinel, master_name)
             for i = 1, num_recs, 2 do
                 host[slave[i]] = slave[i + 1]
             end
-            host.host = host.ip -- for parity with other functions
-            tbl_insert(hosts, host)
+
+            if host["master-link-status"] == "ok" then
+                host.host = host.ip -- for parity with other functions
+                tbl_insert(hosts, host)
+            end
         end
         if hosts[1] ~= nil then
             return hosts

diff --git a/t/sentinel.t b/t/sentinel.t
@@ -3,7 +3,7 @@
 use Test::Nginx::Socket::Lua;
 use Cwd qw(cwd);
 
-repeat_each(2);
+#repeat_each(2);
 
 plan tests => repeat_each() * (3 * blocks());
 
@@ -38,7 +38,7 @@ __DATA__
             end
 
             local redis_sentinel = require "resty.redis.sentinel"
-            
+
             local master, err = redis_sentinel.get_master(sentinel, "mymaster")
             if not master then
                 ngx.say(err)
@@ -74,7 +74,57 @@ port: 6379
             end
 
             local redis_sentinel = require "resty.redis.sentinel"
-            
+
+            local slaves, err = redis_sentinel.get_slaves(sentinel, "mymaster")
+            if not slaves then
+                ngx.say(err)
+            else
+                -- order is undefined
+                local all = {}
+                for i,slave in ipairs(slaves) do
+                    all[i] = tonumber(slave.port)
+                end
+                table.sort(all)
+                for _,p in ipairs(all) do
+                    ngx.say(p)
+                end
+            end
+
+            sentinel:close()
+        ';
+    }
+--- request
+    GET /t
+--- response_body
+6378
+6380
+--- no_error_log
+[error]
+
+=== TEST 3: Get only healthy slaves
+--- http_config eval: $::HttpConfig
+--- config
+    location /t {
+        content_by_lua '
+
+            local redis = require "resty.redis"
+            local r = redis.new()
+            r:connect("127.0.0.1", 6378)
+            r:slaveof("127.0.0.1", 7000)
+
+            ngx.sleep(9)
+
+            local redis_connector = require "resty.redis.connector"
+            local rc = redis_connector.new()
+
+            local sentinel, err = rc:connect{ url = "redis://127.0.0.1:6381" }
+            if not sentinel then
+                ngx.say("failed to connect: ", err)
+                return
+            end
+
+            local redis_sentinel = require "resty.redis.sentinel"
+
             local slaves, err = redis_sentinel.get_slaves(sentinel, "mymaster")
             if not slaves then
                 ngx.say(err)
@@ -90,9 +140,9 @@ port: 6379
     }
 --- request
     GET /t
+--- timeout: 10
 --- response_body
 host: 127.0.0.1
 port: 6380
 --- no_error_log
 [error]
-