lua-resty-limit-traffic icon indicating copy to clipboard operation
lua-resty-limit-traffic copied to clipboard

Some advise for limit count module

Open Angry-Rabbit opened this issue 6 years ago • 5 comments

  1. In module resty/limit/count.lua, At line: 54 ~ 69 in method _M.incoming() it seemed never to executed;
  2. Can not set limit like req limit module "set_rate", Change cache the incoming amount instead of remaining should be fine for this feature.

Angry-Rabbit avatar Sep 28 '17 09:09 Angry-Rabbit

@Angry-Rabbit For 1), that bit is for race condition handling, which should be very rare to hit but still possible in the wild. For 2), I don't understand your intention and use case. Will you elaborate?

agentzh avatar Sep 28 '17 17:09 agentzh

@agentzh Thank you for your answer. I see now. For the second point, the scenes is I want to deploy multiple openresty instances as an high availability API gateway server group, when any of these servers are breakdown or recover, the other servers can automatically adjust the limit threshold according to the number of living server nodes detected by the heartbeat.

Exmple: I've deployed muti server instances, the number of breakdown server instance is M, living server instance is N. and limit rules list like blow:

-- Upper concurrent incoming for each API  limit 500 
local lim_conn, err =   limit_conn.new("my_limit_conn_store", 500/N, 100, 0.5)

-- Limit 5000 count for each ip daily
local lim_count, err = limit_count.new("my_limit_count_store", 5000/N, 3600*24)

If some instance breakdown, it can be detected by heartbeat and return the new number of living instance N2. For conn limit, The conn can changed simply like blow:

limit_conn:set_conn(500/N2)

But for count limit it seemed cannot do this dynamic change. I think if count limit module provide some functions(like, incoming_amount(), set_limit() ) should be conveniently to do this change, just program like this:

-- Get incoming amount
local incoming_amount = limit_count.incoming_amount(key)

-- Dynamic change limit.
-- total_remaining:  The total of remaining amount, stored in cache server, like redis. It is obtained by the calculation of the server node limit data that sync to the cache server periodically
-- N2:  The number of living server instance
limit_count.set_limit(incoming_amount + total_remaining/N2)

Angry-Rabbit avatar Sep 29 '17 06:09 Angry-Rabbit

@Angry-Rabbit It's technically possible to adjust the count limit threshold on the fly. Will you contribute a patch for it? Thanks!

agentzh avatar Sep 29 '17 07:09 agentzh

@agentzh Yes, I'll try. This just in my mind now, need some time to do it.

Angry-Rabbit avatar Sep 29 '17 07:09 Angry-Rabbit

We are fighting similar problem. We would like to change rate limits on the fly, because they are pulled from some external store and people can upgrade/downgrade their plan.

I'd be happy to contribute a patch. But feel like the best way forward is to change the "decrement" model (keeping remaining amount) to "increment" (keeping current amount).

Right now the count module decrements from the limit to 0: https://github.com/openresty/lua-resty-limit-traffic/blob/ef080731b80fe00a385a4aed8a7f7f575d01c152/lib/resty/limit/count.lua#L47

But I'd like to change it to track the current value: https://github.com/openresty/lua-resty-limit-traffic/blob/ef080731b80fe00a385a4aed8a7f7f575d01c152/lib/resty/limit/conn.lua#L53 and then just verify it is over/under the limit. This would also offer the flexibility of using different limits on the same key (for example on different paths or for different groups of users).

Would you be ok with that change @agentzh ?

mikz avatar May 15 '18 07:05 mikz