search

openresty / lua-resty-balancer

A generic consistent hash implementation for OpenResty/Lua
322 stars 77 forks source link

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

Open stone-wind opened 4 years ago

spacewander commented 4 years ago

Why?

stone-wind commented 4 years ago

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 commented 4 years ago

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

spacewander commented 4 years ago

Pull request is welcome.

spacewander commented 4 years ago

@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.

stone-wind commented 4 years ago

@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.

spacewander commented 4 years ago

@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.