AlinsRan commented on code in PR #12751:
URL: https://github.com/apache/apisix/pull/12751#discussion_r2583737605
##########
apisix/plugins/limit-count/limit-count-redis.lua:
##########
@@ -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)
+ if get_phase() == "log" then
Review Comment:
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.
https://github.com/apache/apisix/blob/896d3c389196e7a06fddeb5c03006bf300fa4704/apisix/plugins/limit-count/init.lua#L301-L322
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
