-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat: ai rate limiting redis support #12751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
2194930
680af73
75b0aa7
14e6fce
d5552d8
82323ae
3b8e2ad
5204105
c857713
73737c1
b303401
0b76863
f86b98f
706bcb7
a10b0e7
ac70f00
f9ceaeb
5276e0c
7f02e0e
a6b71af
a716a34
5c409c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,9 +16,12 @@ | |||||||||||||||||||||||||||||||||||||||||||||
| -- | ||||||||||||||||||||||||||||||||||||||||||||||
| local redis = require("apisix.utils.redis") | ||||||||||||||||||||||||||||||||||||||||||||||
| local core = require("apisix.core") | ||||||||||||||||||||||||||||||||||||||||||||||
| local ngx = ngx | ||||||||||||||||||||||||||||||||||||||||||||||
| local get_phase = ngx.get_phase | ||||||||||||||||||||||||||||||||||||||||||||||
| local assert = assert | ||||||||||||||||||||||||||||||||||||||||||||||
| local setmetatable = setmetatable | ||||||||||||||||||||||||||||||||||||||||||||||
| local tostring = tostring | ||||||||||||||||||||||||||||||||||||||||||||||
| local util = require("apisix.plugins.limit-count.util") | ||||||||||||||||||||||||||||||||||||||||||||||
| local ngx_timer_at = ngx.timer.at | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local _M = {version = 0.3} | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -29,17 +32,6 @@ local mt = { | |||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local script = core.string.compress_script([=[ | ||||||||||||||||||||||||||||||||||||||||||||||
| assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1") | ||||||||||||||||||||||||||||||||||||||||||||||
| local ttl = redis.call('ttl', KEYS[1]) | ||||||||||||||||||||||||||||||||||||||||||||||
| if ttl < 0 then | ||||||||||||||||||||||||||||||||||||||||||||||
| redis.call('set', KEYS[1], ARGV[1] - ARGV[3], 'EX', ARGV[2]) | ||||||||||||||||||||||||||||||||||||||||||||||
| return {ARGV[1] - ARGV[3], ARGV[2]} | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
| return {redis.call('incrby', KEYS[1], 0 - ARGV[3]), ttl} | ||||||||||||||||||||||||||||||||||||||||||||||
| ]=]) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| function _M.new(plugin_name, limit, window, conf) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert(limit > 0 and window > 0) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -52,37 +44,66 @@ function _M.new(plugin_name, limit, window, conf) | |||||||||||||||||||||||||||||||||||||||||||||
| return setmetatable(self, mt) | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| function _M.incoming(self, key, cost) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local function log_phase_incoming_thread(premature, self, key, cost) | ||||||||||||||||||||||||||||||||||||||||||||||
| local conf = self.conf | ||||||||||||||||||||||||||||||||||||||||||||||
| local red, err = redis.new(conf) | ||||||||||||||||||||||||||||||||||||||||||||||
| if not red then | ||||||||||||||||||||||||||||||||||||||||||||||
| return red, err, 0 | ||||||||||||||||||||||||||||||||||||||||||||||
| return red, err | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
| return util.redis_log_phase_incoming(self, red, key, cost) | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local limit = self.limit | ||||||||||||||||||||||||||||||||||||||||||||||
| local window = self.window | ||||||||||||||||||||||||||||||||||||||||||||||
| local res | ||||||||||||||||||||||||||||||||||||||||||||||
| key = self.plugin_name .. tostring(key) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local ttl = 0 | ||||||||||||||||||||||||||||||||||||||||||||||
| res, err = red:eval(script, 1, key, limit, window, cost or 1) | ||||||||||||||||||||||||||||||||||||||||||||||
| local function log_phase_incoming(self, key, cost, dry_run) | ||||||||||||||||||||||||||||||||||||||||||||||
| if dry_run then | ||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if err then | ||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err, ttl | ||||||||||||||||||||||||||||||||||||||||||||||
| local ok, err = ngx_timer_at(0, log_phase_incoming_thread, self, key, cost) | ||||||||||||||||||||||||||||||||||||||||||||||
| if not ok then | ||||||||||||||||||||||||||||||||||||||||||||||
| core.log.error("failed to create timer: ", err) | ||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local remaining = res[1] | ||||||||||||||||||||||||||||||||||||||||||||||
| ttl = res[2] | ||||||||||||||||||||||||||||||||||||||||||||||
| return ok | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| function _M.incoming(self, key, cost, dry_run) | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be consistent with local and recommend using the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that the modification of the apisix/apisix/plugins/limit-count/init.lua Line 282 in 896d3c3
|
||||||||||||||||||||||||||||||||||||||||||||||
| if get_phase() == "log" then | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should use a new function to handle the log phase instead of dealing with it within the incoming function. This is because we need to mock parameters like remaining, which isn't reasonable, and the log phase doesn't require a status code. apisix/apisix/plugins/limit-count/init.lua Lines 301 to 322 in 896d3c3
|
||||||||||||||||||||||||||||||||||||||||||||||
| local ok, err = log_phase_incoming(self, key, cost, dry_run) | ||||||||||||||||||||||||||||||||||||||||||||||
| if not ok then | ||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err, 0 | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| -- best-effort result because lua-resty-redis is not allowed in log phase | ||||||||||||||||||||||||||||||||||||||||||||||
| return 0, self.limit, self.window | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local conf = self.conf | ||||||||||||||||||||||||||||||||||||||||||||||
| local red, err = redis.new(conf) | ||||||||||||||||||||||||||||||||||||||||||||||
| if not red then | ||||||||||||||||||||||||||||||||||||||||||||||
| return red, err, 0 | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local commit = true | ||||||||||||||||||||||||||||||||||||||||||||||
| if dry_run ~= nil then | ||||||||||||||||||||||||||||||||||||||||||||||
| commit = not dry_run | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local delay, remaining, ttl = util.redis_incoming(self, red, key, commit, cost) | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+90
to
+95
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
| if not delay then | ||||||||||||||||||||||||||||||||||||||||||||||
| local err = remaining | ||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err, ttl or 0 | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| local ok, err = red:set_keepalive(10000, 100) | ||||||||||||||||||||||||||||||||||||||||||||||
| if not ok then | ||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err, ttl | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if remaining < 0 then | ||||||||||||||||||||||||||||||||||||||||||||||
| return nil, "rejected", ttl | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
| return 0, remaining, ttl | ||||||||||||||||||||||||||||||||||||||||||||||
| return delay, remaining, ttl | ||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,79 @@ | ||||||||||||||||
| -- | ||||||||||||||||
beardnick marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
| -- Licensed to the Apache Software Foundation (ASF) under one or more | ||||||||||||||||
| -- contributor license agreements. See the NOTICE file distributed with | ||||||||||||||||
| -- this work for additional information regarding copyright ownership. | ||||||||||||||||
| -- The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||||||||||||
| -- (the "License"); you may not use this file except in compliance with | ||||||||||||||||
| -- the License. You may obtain a copy of the License at | ||||||||||||||||
| -- | ||||||||||||||||
| -- http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||
| -- | ||||||||||||||||
| -- Unless required by applicable law or agreed to in writing, software | ||||||||||||||||
| -- distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||
| -- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||
| -- See the License for the specific language governing permissions and | ||||||||||||||||
| -- limitations under the License. | ||||||||||||||||
| -- | ||||||||||||||||
| local core = require("apisix.core") | ||||||||||||||||
| local tostring = tostring | ||||||||||||||||
| local tonumber = tonumber | ||||||||||||||||
| local _M = {version = 0.1} | ||||||||||||||||
|
|
||||||||||||||||
| local commit_script = core.string.compress_script([=[ | ||||||||||||||||
| assert(tonumber(ARGV[3]) >= 0, "cost must be at least 0") | ||||||||||||||||
| local ttl = redis.call('ttl', KEYS[1]) | ||||||||||||||||
| if ttl < 0 then | ||||||||||||||||
| redis.call('set', KEYS[1], ARGV[1] - ARGV[3], 'EX', ARGV[2]) | ||||||||||||||||
| return {ARGV[1] - ARGV[3], ARGV[2]} | ||||||||||||||||
| end | ||||||||||||||||
| return {redis.call('incrby', KEYS[1], 0 - ARGV[3]), ttl} | ||||||||||||||||
| ]=]) | ||||||||||||||||
|
|
||||||||||||||||
| function _M.redis_incoming(self, red, key, commit, cost) | ||||||||||||||||
| local limit = self.limit | ||||||||||||||||
| local window = self.window | ||||||||||||||||
| key = self.plugin_name .. tostring(key) | ||||||||||||||||
|
|
||||||||||||||||
| local requested_cost = cost or 1 | ||||||||||||||||
| local script_cost = commit and requested_cost or 0 | ||||||||||||||||
| local res, err = red:eval(commit_script, 1, key, limit, window, script_cost) | ||||||||||||||||
|
Comment on lines
+37
to
+39
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| if err then | ||||||||||||||||
| return nil, err, 0 | ||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| local stored_remaining = tonumber(res[1]) | ||||||||||||||||
| if stored_remaining == nil then | ||||||||||||||||
| stored_remaining = limit - script_cost | ||||||||||||||||
| end | ||||||||||||||||
| local ttl = tonumber(res[2]) or window | ||||||||||||||||
|
|
||||||||||||||||
| local remaining | ||||||||||||||||
| if commit then | ||||||||||||||||
| remaining = stored_remaining | ||||||||||||||||
| else | ||||||||||||||||
| remaining = stored_remaining - requested_cost | ||||||||||||||||
| end | ||||||||||||||||
|
Comment on lines
+51
to
+56
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| if remaining < 0 then | ||||||||||||||||
| return nil, "rejected", ttl | ||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| return 0, remaining, ttl | ||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| function _M.redis_log_phase_incoming(self, red, key, cost) | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's redundant. We can call redis_incoming. |
||||||||||||||||
| local limit = self.limit | ||||||||||||||||
| local window = self.window | ||||||||||||||||
| key = self.plugin_name .. tostring(key) | ||||||||||||||||
|
|
||||||||||||||||
| local res, err = red:eval(commit_script, 1, key, limit, window, cost or 1) | ||||||||||||||||
| if err then | ||||||||||||||||
| return nil, err | ||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| return res[1] | ||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| return _M | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code should be placed in the ai-rate-limiting plugin, not in the limit-count plugin itself, because this code is only useful for ai-rate-limiting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree. Similar logic also appears in the limit-conn.
apisix/apisix/plugins/limit-conn/limit-conn-redis.lua
Lines 60 to 81 in 7e907a5
If I put this logic into ai-rate-limiting plugin, it will make the plugin too complicated. I have to copy a lot of logic from limit-count. Currently, the ai-rate-limiting is just a simple wrapper of limit-count.
Or, do you think I need to rewrite the ai-rate-limiting to something like limit-ai-redis.lua and limit-ai-redis-cluster.lua?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is quite large and hard to review. If you agree, I can split it into 3 separate PRs: