make random start a default and only behaviour

openresty · May 27, 2019 · 1a2e79f · 1a2e79f
1 parent b4cf1e6
commit 1a2e79f
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 62 deletions.
diff --git a/README.markdown b/README.markdown
@@ -98,6 +98,7 @@ Synopsis
 
             local rr_up = package.loaded.my_rr_up
 
+            -- Note that Round Robin picks the first server randomly
             local server = rr_up:find()
 
             assert(b.set_current_peer(server))
@@ -126,7 +127,7 @@ Both `resty.chash` and `resty.roundrobin` have the same apis.
 
 new
 ---
-**syntax:** `obj, err = class.new(nodes, random_start?)`
+**syntax:** `obj, err = class.new(nodes)`
 
 Instantiates an object of this class. The `class` value is returned by the call `require "resty.chash"`.
 
@@ -135,9 +136,6 @@ when we need to keep consistency with nginx chash.
 
 The `id` can be any string value when we do not need to keep consistency with nginx chash.
 
-`random_start` can optionally be set when initializing `resty.roundrobin`. If it is set then
-the algorithm will pick first node uniformly at random instead of starting with the first one.
-
 ```lua
 local nodes = {
     -- id => weight

diff --git a/lib/resty/roundrobin.lua b/lib/resty/roundrobin.lua
@@ -48,30 +48,26 @@ local function get_gcd(nodes)
 end
 
 local function get_random_node_id(nodes)
-  local count = 0
-  for _, _ in pairs(nodes) do
-    count = count + 1
-  end
+    local count = 0
+    for _, _ in pairs(nodes) do
+      count = count + 1
+    end
 
-  local id = nil
-  local random_index = math_random(count)
+    local id = nil
+    local random_index = math_random(count)
 
-  for _ = 1, random_index do
-    id = next(nodes, id)
-  end
+    for _ = 1, random_index do
+      id = next(nodes, id)
+    end
 
-  return id
+    return id
 end
 
 
-function _M.new(_, nodes, random_start)
+function _M.new(_, nodes)
     local newnodes = copy(nodes)
     local only_key, gcd, max_weight = get_gcd(newnodes)
-    local last_id = nil
-
-    if random_start then
-      last_id = get_random_node_id(nodes)
-    end
+    local last_id = get_random_node_id(nodes)
 
     local self = {
         nodes = newnodes,  -- it's safer to copy one
@@ -80,7 +76,6 @@ function _M.new(_, nodes, random_start)
         gcd = gcd,
         cw = max_weight,
         last_id = last_id,
-        random_start = random_start,
     }
     return setmetatable(self, mt)
 end
@@ -91,12 +86,8 @@ function _M.reinit(self, nodes)
     self.only_key, self.gcd, self.max_weight = get_gcd(newnodes)
 
     self.nodes = newnodes
-    self.last_id = nil
+    self.last_id = get_random_node_id(nodes)
     self.cw = self.max_weight
-
-    if self.random_start then
-      self.last_id = get_random_node_id(nodes)
-    end
 end
 
 

diff --git a/t/roundrobin.t b/t/roundrobin.t
@@ -28,6 +28,8 @@ __DATA__
 --- config
     location /t {
         content_by_lua_block {
+            math.randomseed(75098)
+
             local roundrobin = require "resty.roundrobin"
 
             local servers = {
@@ -53,7 +55,6 @@ GET /t
 gcd: 2
 id: server1
 id: server1
-id: server1
 id: server2
 id: server1
 id: server2
@@ -65,6 +66,7 @@ id: server2
 id: server1
 id: server2
 id: server3
+id: server1
 --- no_error_log
 [error]
 
@@ -75,6 +77,8 @@ id: server3
 --- config
     location /t {
         content_by_lua_block {
+            math.randomseed(75098)
+
             local roundrobin = require "resty.roundrobin"
 
             local servers = {
@@ -104,70 +108,43 @@ id: server3
 --- request
 GET /t
 --- response_body
-server1: 50001
+server1: 50000
 server3: 16666
-server2: 33333
+server2: 33334
 --- no_error_log
 [error]
 
 
 
-=== TEST 3: new with random start
+=== TEST 3: random start
 --- http_config eval: $::HttpConfig
 --- config
     location /t {
         content_by_lua_block {
-            math.randomseed(75098)
-
-            local roundrobin = require "resty.roundrobin"
-
-            local servers = {
-                ["server1"] = 1,
-                ["server2"] = 1,
-                ["server3"] = 1,
-            }
-
-            local rr = roundrobin:new(servers, true)
-            local id = rr:find()
-            ngx.say(id)
-        }
-    }
---- request
-GET /t
---- response_body
-server3
---- no_error_log
-[error]
-
-
-
-=== TEST 4: reinit with random start
---- http_config eval: $::HttpConfig
---- config
-    location /t {
-        content_by_lua_block {
-            math.randomseed(75098)
+            math.randomseed(9975098)
 
             local roundrobin = require "resty.roundrobin"
 
             local servers = {
                 ["server1"] = 1,
                 ["server2"] = 1,
                 ["server3"] = 1,
+                ["server4"] = 1,
             }
 
             local rr = roundrobin:new(servers, true)
             local id = rr:find()
             ngx.say(id)
 
-            math.randomseed(99111)
+            math.randomseed(11175098)
 
             local new_servers = {
                 ["server1"] = 1,
                 ["server2"] = 1,
                 ["server3"] = 1,
                 ["server4"] = 1,
                 ["server5"] = 1,
+                ["server6"] = 1,
             }
 
             rr:reinit(new_servers)
@@ -178,7 +155,7 @@ server3
 --- request
 GET /t
 --- response_body
-server3
+server2
 server5
 --- no_error_log
 [error]