tokers commented on a change in pull request #3300:
URL: https://github.com/apache/apisix/pull/3300#discussion_r565836644
##########
File path: t/node/ewma.t
##########
@@ -215,3 +215,74 @@ GET /t
--- error_code: 200
--- no_error_log
[error]
+
+
+=== TEST 4: about filter tried servers
+--- timeout: 5
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+
+ --remove the 1981 node,
+ --add the 1984 node (invalid node)
+ --keep two nodes for triggering ewma logic in server_picker
function of balancer phase
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 100,
+ "127.0.0.1:1984": 100
+ },
+ "type": "ewma"
+ },
+ "uri": "/ewma"
+ }]]
+ )
+
+ if code ~= 200 then
+ ngx.say("update route failed")
+ return
+ end
+
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/ewma"
+
+ local ports_count = {}
+ for i = 1, 12 do
+ local httpc = http.new()
+ httpc:set_timeout(1000)
+ local res, err = httpc:request_uri(uri, {method = "GET",
keepalive = false})
+ if not res then
+ ngx.say(err)
+ return
+ end
+
+ ports_count[res.body] = (ports_count[res.body] or 0) + 1
+ end
+
+ local ports_arr = {}
+ for port, count in pairs(ports_count) do
+ table.insert(ports_arr, {port = port, count = count})
+ end
+
+ local function cmd(a, b)
+ return a.port > b.port
+ end
+ table.sort(ports_arr, cmd)
+
+ ngx.say(require("toolkit.json").encode(ports_arr))
+ ngx.exit(200)
+ }
+ }
+--- request
+GET /t
+--- response_body
+[{"count":12,"port":"1980"}]
+--- error_code: 200
+--- no_error_log
+[error]
+
Review comment:
Redundant empty lines.
##########
File path: apisix/balancer/ewma.lua
##########
@@ -35,73 +58,69 @@ local function decay_ewma(ewma, last_touched_at, rtt, now)
return ewma
end
-
local function store_stats(upstream, ewma, now)
local success, err, forcible = shm_last_touched_at:set(upstream, now)
if not success then
- core.log.error("balancer_ewma_last_touched_at:set failed ", err)
+ core.log.error("shm_last_touched_at:set failed: ", err)
end
if forcible then
- core.log.warn("balancer_ewma_last_touched_at:set valid items forcibly
overwritten")
+ core.log.warn("shm_last_touched_at:set valid items forcibly
overwritten")
end
success, err, forcible = shm_ewma:set(upstream, ewma)
if not success then
- core.log.error("balancer_ewma:set failed ", err)
+ core.log.error("shm_ewma:set failed: ", err)
end
if forcible then
- core.log.warn("balancer_ewma:set valid items forcibly overwritten")
+ core.log.warn("shm_ewma:set valid items forcibly overwritten")
end
end
-
local function get_or_update_ewma(upstream, rtt, update)
+ local lock_err = nil
+ if update then
+ lock_err = lock(upstream)
+ end
+
local ewma = shm_ewma:get(upstream) or 0
+ if lock_err ~= nil then
Review comment:
Why check `lock_err` after fetching `ewma`? It's so strange we don't
check the return value after `lock` return. Or you should get `ewma` before
fetching the lock.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
