membphis commented on a change in pull request #3300:
URL: https://github.com/apache/apisix/pull/3300#discussion_r557934625
##########
File path: apisix/balancer/ewma.lua
##########
@@ -25,80 +28,102 @@ local lrucache_trans_format = core.lrucache.new({
ttl = 300, count = 256
})
-
-local function decay_ewma(ewma, last_touched_at, rtt, now)
- local td = now - last_touched_at
- td = math.max(td, 0)
- local weight = math.exp(-td / DECAY_TIME)
-
- ewma = ewma * weight + rtt * (1.0 - weight)
- return ewma
+local ewma_lock, ewma_lock_err = resty_lock:new("balancer_ewma_locks",
{timeout = 0, exptime = 0.1})
+if not ewma_lock then
+ error(ewma_lock_err)
end
+local _M = { name = "ewma" }
-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)
- end
- if forcible then
- core.log.warn("balancer_ewma_last_touched_at:set valid items forcibly
overwritten")
+local function lock(upstream)
+ local _, err = ewma_lock:lock(upstream .. LOCK_KEY)
+ if err then
+ if err ~= "timeout" then
+ ngx.log(ngx.ERR, string.format("EWMA Balancer failed to lock: %s",
tostring(err)))
Review comment:
better style(better performance):
```suggestion
core.log.error("EWMA Balancer failed to lock: ", err)
```
##########
File path: apisix/balancer/ewma.lua
##########
@@ -25,80 +28,102 @@ local lrucache_trans_format = core.lrucache.new({
ttl = 300, count = 256
})
-
-local function decay_ewma(ewma, last_touched_at, rtt, now)
- local td = now - last_touched_at
- td = math.max(td, 0)
- local weight = math.exp(-td / DECAY_TIME)
-
- ewma = ewma * weight + rtt * (1.0 - weight)
- return ewma
+local ewma_lock, ewma_lock_err = resty_lock:new("balancer_ewma_locks",
{timeout = 0, exptime = 0.1})
+if not ewma_lock then
+ error(ewma_lock_err)
end
+local _M = { name = "ewma" }
-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)
- end
- if forcible then
- core.log.warn("balancer_ewma_last_touched_at:set valid items forcibly
overwritten")
+local function lock(upstream)
+ local _, err = ewma_lock:lock(upstream .. LOCK_KEY)
+ if err then
+ if err ~= "timeout" then
+ ngx.log(ngx.ERR, string.format("EWMA Balancer failed to lock: %s",
tostring(err)))
end
+ end
- success, err, forcible = shm_ewma:set(upstream, ewma)
- if not success then
- core.log.error("balancer_ewma:set failed ", err)
- end
- if forcible then
- core.log.warn("balancer_ewma:set valid items forcibly overwritten")
- end
+ return err
end
+local function unlock()
+ local ok, err = ewma_lock:unlock()
+ if not ok then
+ ngx.log(ngx.ERR, string.format("EWMA Balancer failed to unlock: %s",
tostring(err)))
+ end
-local function get_or_update_ewma(upstream, rtt, update)
- local ewma = shm_ewma:get(upstream) or 0
- local now = ngx_now()
- local last_touched_at = shm_last_touched_at:get(upstream) or 0
- ewma = decay_ewma(ewma, last_touched_at, rtt, now)
+ return err
+end
- if not update then
- return ewma
- end
+local function decay_ewma(ewma, last_touched_at, rtt, now)
+ local td = now - last_touched_at
+ td = (td > 0) and td or 0
+ local weight = math.exp(-td/DECAY_TIME)
- store_stats(upstream, ewma, now)
+ ewma = ewma * weight + rtt * (1.0 - weight)
+ return ewma
+end
- return ewma
+local function store_stats(upstream, ewma, now)
+ local success, err, forcible = shm_last_touched_at:set(upstream, now)
+ if not success then
+ ngx.log(ngx.WARN, "shm_last_touched_at:set failed " .. err)
+ end
+ if forcible then
+ ngx.log(ngx.WARN, "shm_last_touched_at:set valid items forcibly
overwritten")
+ end
+
+ success, err, forcible = shm_ewma:set(upstream, ewma)
+ if not success then
+ ngx.log(ngx.WARN, "shm_ewma:set failed " .. err)
+ end
+ if forcible then
+ ngx.log(ngx.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 function score(upstream)
- -- Original implementation used names
- -- Endpoints don't have names, so passing in host:Port as key instead
- local upstream_name = upstream.host .. ":" .. upstream.port
- return get_or_update_ewma(upstream_name, 0, false)
-end
+ local ewma = ngx.shared.balancer_ewma:get(upstream) or 0
+ if lock_err ~= nil then
Review comment:
it seems wrong, please confirm this code
##########
File path: apisix/balancer/ewma.lua
##########
@@ -25,80 +28,102 @@ local lrucache_trans_format = core.lrucache.new({
ttl = 300, count = 256
})
-
-local function decay_ewma(ewma, last_touched_at, rtt, now)
- local td = now - last_touched_at
- td = math.max(td, 0)
- local weight = math.exp(-td / DECAY_TIME)
-
- ewma = ewma * weight + rtt * (1.0 - weight)
- return ewma
+local ewma_lock, ewma_lock_err = resty_lock:new("balancer_ewma_locks",
{timeout = 0, exptime = 0.1})
+if not ewma_lock then
+ error(ewma_lock_err)
end
+local _M = { name = "ewma" }
-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)
- end
- if forcible then
- core.log.warn("balancer_ewma_last_touched_at:set valid items forcibly
overwritten")
+local function lock(upstream)
+ local _, err = ewma_lock:lock(upstream .. LOCK_KEY)
+ if err then
+ if err ~= "timeout" then
+ ngx.log(ngx.ERR, string.format("EWMA Balancer failed to lock: %s",
tostring(err)))
end
+ end
- success, err, forcible = shm_ewma:set(upstream, ewma)
- if not success then
- core.log.error("balancer_ewma:set failed ", err)
- end
- if forcible then
- core.log.warn("balancer_ewma:set valid items forcibly overwritten")
- end
+ return err
end
+local function unlock()
+ local ok, err = ewma_lock:unlock()
+ if not ok then
+ ngx.log(ngx.ERR, string.format("EWMA Balancer failed to unlock: %s",
tostring(err)))
Review comment:
ditto
please fix similar points
----------------------------------------------------------------
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]
