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 log_phase_incoming 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]
