spacewander commented on a change in pull request #3300:
URL: https://github.com/apache/apisix/pull/3300#discussion_r565987438
##########
File path: apisix/balancer/ewma.lua
##########
@@ -119,38 +138,76 @@ local function _trans_format(up_nodes)
return next(peers) and peers or nil
end
-
local function _ewma_find(ctx, up_nodes)
local peers
- local endpoint
- if not up_nodes
- or core.table.nkeys(up_nodes) == 0 then
+ if not up_nodes or nkeys(up_nodes) == 0 then
return nil, 'up_nodes empty'
end
- peers = lrucache_trans_format(ctx.upstream_key, ctx.upstream_version,
- _trans_format, up_nodes)
+ peers = lrucache_trans_format(ctx.upstream_key, ctx.upstream_version,
_trans_format, up_nodes)
if not peers then
return nil, 'up_nodes trans error'
end
- if #peers > 1 then
- endpoint = pick_and_score(peers)
+ local tried_endpoints
+ if not ctx.balancer_tried_servers then
+ tried_endpoints = {}
Review comment:
It seems we can save the table allocation of `tried_endpoints` and
`filtered_peers` when `not ctx.balancer_tried_servers`?
##########
File path: apisix/balancer/ewma.lua
##########
@@ -119,38 +138,76 @@ local function _trans_format(up_nodes)
return next(peers) and peers or nil
end
-
local function _ewma_find(ctx, up_nodes)
local peers
- local endpoint
- if not up_nodes
- or core.table.nkeys(up_nodes) == 0 then
+ if not up_nodes or nkeys(up_nodes) == 0 then
return nil, 'up_nodes empty'
end
- peers = lrucache_trans_format(ctx.upstream_key, ctx.upstream_version,
- _trans_format, up_nodes)
+ peers = lrucache_trans_format(ctx.upstream_key, ctx.upstream_version,
_trans_format, up_nodes)
if not peers then
return nil, 'up_nodes trans error'
end
- if #peers > 1 then
- endpoint = pick_and_score(peers)
+ local tried_endpoints
+ if not ctx.balancer_tried_servers then
+ tried_endpoints = {}
+ ctx.balancer_tried_servers = tried_endpoints
Review comment:
We already set `ctx.balancer_tried_servers` in the `after_balancer`?
##########
File path: apisix/balancer/ewma.lua
##########
@@ -119,38 +138,76 @@ local function _trans_format(up_nodes)
return next(peers) and peers or nil
end
-
local function _ewma_find(ctx, up_nodes)
local peers
- local endpoint
- if not up_nodes
- or core.table.nkeys(up_nodes) == 0 then
+ if not up_nodes or nkeys(up_nodes) == 0 then
return nil, 'up_nodes empty'
end
- peers = lrucache_trans_format(ctx.upstream_key, ctx.upstream_version,
- _trans_format, up_nodes)
+ peers = lrucache_trans_format(ctx.upstream_key, ctx.upstream_version,
_trans_format, up_nodes)
if not peers then
return nil, 'up_nodes trans error'
end
- if #peers > 1 then
- endpoint = pick_and_score(peers)
+ local tried_endpoints
+ if not ctx.balancer_tried_servers then
+ tried_endpoints = {}
+ ctx.balancer_tried_servers = tried_endpoints
else
- endpoint = peers[1]
+ tried_endpoints = ctx.balancer_tried_servers
end
- return endpoint.host .. ":" .. endpoint.port
-end
+ local filtered_peers
+ for _, peer in ipairs(peers) do
+ if not tried_endpoints[get_upstream_name(peer)] then
+ if not filtered_peers then
+ filtered_peers = {}
+ end
+ table_insert(filtered_peers, peer)
+ end
+ end
+ if not filtered_peers then
+ core.log.warn("all endpoints have been retried")
+ filtered_peers = table_deepcopy(peers)
Review comment:
Need a test to cover this branch. And since we don't need to copy the
peers?
----------------------------------------------------------------
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]
