lua-resty-balancer icon indicating copy to clipboard operation
lua-resty-balancer copied to clipboard

roundrobin:new() should support empty nodes, not just throw error

Open stone-wind opened this issue 5 years ago • 7 comments

stone-wind avatar Jul 16 '20 05:07 stone-wind

Why?

spacewander avatar Jul 16 '20 08:07 spacewander

chash:new() almost support empty nodes, and then it can dynamic set/delete with only one init action. it's convenient. chash's _find_id, find, next have problem when support empty nodes. it's easy to reach if we allow return nil.

stone-wind avatar Jul 16 '20 09:07 stone-wind

case nginx do not allow 0 server in upstream, maybe chash should throw error when get empty nodes like roundrobin

stone-wind avatar Jul 16 '20 10:07 stone-wind

Pull request is welcome.

spacewander avatar Jul 16 '20 10:07 spacewander

@stone-wind A little question: would it be better if we don't allow find, next call and throw error like the current behavior. A nil server id is not valid. An empty roundrobin is allowed, but if you want to get any nodes from it, you need to fill it first.

spacewander avatar Jul 17 '20 01:07 spacewander

@spacewander my server_list come from redis, weight equal 0 means don't need proxy conn to this backend. so my choose is support empty nodes and 0 weight node, and throw error when weight is less than 0 in init(reinit) func.

stone-wind avatar Jul 20 '20 12:07 stone-wind

@stone-wind I think you can achieve this with a fake backend, without modifying the exist code? When the fake backend is chosen, you don't need to proxy.

spacewander avatar Jul 21 '20 01:07 spacewander