From 03be4912beb2153d6412594feac9cb7370fc9395 Mon Sep 17 00:00:00 2001 From: yuchenghao Date: Tue, 6 Apr 2021 15:57:10 +0800 Subject: [PATCH 1/5] ensure automic operation in limit-count plugin --- .../limit-count/limit-count-redis-cluster.lua | 53 +++-------------- .../plugins/limit-count/limit-count-redis.lua | 59 ++----------------- 2 files changed, 13 insertions(+), 99 deletions(-) diff --git a/apisix/plugins/limit-count/limit-count-redis-cluster.lua b/apisix/plugins/limit-count/limit-count-redis-cluster.lua index 2be5caec637c..215a9ec608aa 100644 --- a/apisix/plugins/limit-count/limit-count-redis-cluster.lua +++ b/apisix/plugins/limit-count/limit-count-redis-cluster.lua @@ -77,57 +77,18 @@ function _M.incoming(self, key) local red = self.red_cli local limit = self.limit local window = self.window - local remaining key = self.plugin_name .. tostring(key) + local remaining,err = red:eval("if redis.call('ttl',KEYS[1]) < 0 then redis.call('set',KEYS[1],ARGV[1]-1,'EX',ARGV[2]) return ARGV[1]-1 end return redis.call('incrby',KEYS[1],-1)",1,key,limit,window) - local ret, err = red:ttl(key) - if not ret then - return false, "failed to get redis `" .. key .."` ttl: " .. err + if err then + return nil,err end - core.log.info("ttl key: ", key, " ret: ", ret, " err: ", err) - if ret < 0 then - local lock, err = resty_lock:new("plugin-limit-count") - if not lock then - return false, "failed to create lock: " .. err - end - - local elapsed, err = lock:lock(key) - if not elapsed then - return false, "failed to acquire the lock: " .. err - end - - ret = red:ttl(key) - if ret < 0 then - local ok, err = lock:unlock() - if not ok then - return false, "failed to unlock: " .. err - end - - ret, err = red:set(key, limit -1, "EX", window) - if not ret then - return nil, err - end - - return 0, limit -1 - end - - local ok, err = lock:unlock() - if not ok then - return false, "failed to unlock: " .. err - end - end - - remaining, err = red:incrby(key, -1) - if not remaining then - return nil, err + if remaining <0 then + core.log.error(remaining) + return nil,"rejected" end - - if remaining < 0 then - return nil, "rejected" - end - - return 0, remaining + return 0,remaining end diff --git a/apisix/plugins/limit-count/limit-count-redis.lua b/apisix/plugins/limit-count/limit-count-redis.lua index 24621437dffd..bab2b7a0074e 100644 --- a/apisix/plugins/limit-count/limit-count-redis.lua +++ b/apisix/plugins/limit-count/limit-count-redis.lua @@ -76,65 +76,18 @@ function _M.incoming(self, key) local limit = self.limit local window = self.window - local remaining key = self.plugin_name .. tostring(key) - -- todo: test case - local ret, err = red:ttl(key) - if not ret then - return false, "failed to get redis `" .. key .."` ttl: " .. err - end - - core.log.info("ttl key: ", key, " ret: ", ret, " err: ", err) - if ret < 0 then - -- todo: test case - local lock, err = resty_lock:new("plugin-limit-count") - if not lock then - return false, "failed to create lock: " .. err - end - - local elapsed, err = lock:lock(key) - if not elapsed then - return false, "failed to acquire the lock: " .. err - end - - ret = red:ttl(key) - if ret < 0 then - ok, err = lock:unlock() - if not ok then - return false, "failed to unlock: " .. err - end - - limit = limit -1 - ret, err = red:set(key, limit, "EX", window) - if not ret then - return nil, err - end + local remaining,err = red:eval("if redis.call('ttl',KEYS[1]) < 0 then redis.call('set',KEYS[1],ARGV[1]-1,'EX',ARGV[2]) return ARGV[1]-1 end return redis.call('incrby',KEYS[1],-1)",1,key,limit,window) - return 0, limit - end - - ok, err = lock:unlock() - if not ok then - return false, "failed to unlock: " .. err - end + if err then + return nil,err end - remaining, err = red:incrby(key, -1) - if not remaining then - return nil, err + if remaining <0 then + return nil,"rejected" end - - local ok, err = red:set_keepalive(10000, 100) - if not ok then - return nil, err - end - - if remaining < 0 then - return nil, "rejected" - end - - return 0, remaining + return 0,remaining end From b05cca231513f73578dbb72f89c4a9c2031cdda5 Mon Sep 17 00:00:00 2001 From: yuchenghao Date: Tue, 6 Apr 2021 17:05:45 +0800 Subject: [PATCH 2/5] fix: code lint --- .../plugins/limit-count/limit-count-redis-cluster.lua | 9 +++++++-- apisix/plugins/limit-count/limit-count-redis.lua | 11 ++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/apisix/plugins/limit-count/limit-count-redis-cluster.lua b/apisix/plugins/limit-count/limit-count-redis-cluster.lua index 215a9ec608aa..57caccc0d726 100644 --- a/apisix/plugins/limit-count/limit-count-redis-cluster.lua +++ b/apisix/plugins/limit-count/limit-count-redis-cluster.lua @@ -17,7 +17,6 @@ local rediscluster = require("resty.rediscluster") local core = require("apisix.core") -local resty_lock = require("resty.lock") local setmetatable = setmetatable local tostring = tostring local ipairs = ipairs @@ -72,13 +71,19 @@ function _M.new(plugin_name, limit, window, conf) return setmetatable(self, mt) end +local script = "if redis.call('ttl',KEYS[1]) < 0 then " + .. "redis.call('set',KEYS[1],ARGV[1]-1,'EX',ARGV[2]) " + .. "return ARGV[1]-1 " + .. "end " + .. "return redis.call('incrby',KEYS[1],-1)" function _M.incoming(self, key) local red = self.red_cli local limit = self.limit local window = self.window key = self.plugin_name .. tostring(key) - local remaining,err = red:eval("if redis.call('ttl',KEYS[1]) < 0 then redis.call('set',KEYS[1],ARGV[1]-1,'EX',ARGV[2]) return ARGV[1]-1 end return redis.call('incrby',KEYS[1],-1)",1,key,limit,window) + + local remaining,err = red:eval(script,1,key,limit,window) if err then return nil,err diff --git a/apisix/plugins/limit-count/limit-count-redis.lua b/apisix/plugins/limit-count/limit-count-redis.lua index bab2b7a0074e..84b74e724750 100644 --- a/apisix/plugins/limit-count/limit-count-redis.lua +++ b/apisix/plugins/limit-count/limit-count-redis.lua @@ -16,7 +16,6 @@ -- local redis_new = require("resty.redis").new local core = require("apisix.core") -local resty_lock = require("resty.lock") local assert = assert local setmetatable = setmetatable local tostring = tostring @@ -30,6 +29,13 @@ local mt = { } +local script = "if redis.call('ttl',KEYS[1]) < 0 then " + .. "redis.call('set',KEYS[1],ARGV[1]-1,'EX',ARGV[2]) " + .. "return ARGV[1]-1 " + .. "end " + .. "return redis.call('incrby',KEYS[1],-1)" + + function _M.new(plugin_name, limit, window, conf) assert(limit > 0 and window > 0) @@ -77,8 +83,7 @@ function _M.incoming(self, key) local limit = self.limit local window = self.window key = self.plugin_name .. tostring(key) - - local remaining,err = red:eval("if redis.call('ttl',KEYS[1]) < 0 then redis.call('set',KEYS[1],ARGV[1]-1,'EX',ARGV[2]) return ARGV[1]-1 end return redis.call('incrby',KEYS[1],-1)",1,key,limit,window) + local remaining, err = red:eval(script,1,key,limit,window) if err then return nil,err From b6a9f1ba51ec1b881df3c1cb1cb676bad40461a9 Mon Sep 17 00:00:00 2001 From: yuchenghao Date: Tue, 6 Apr 2021 18:08:13 +0800 Subject: [PATCH 3/5] fix: remove unused code --- apisix/plugins/limit-count/limit-count-redis-cluster.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/apisix/plugins/limit-count/limit-count-redis-cluster.lua b/apisix/plugins/limit-count/limit-count-redis-cluster.lua index 57caccc0d726..d9df9953f591 100644 --- a/apisix/plugins/limit-count/limit-count-redis-cluster.lua +++ b/apisix/plugins/limit-count/limit-count-redis-cluster.lua @@ -90,7 +90,6 @@ function _M.incoming(self, key) end if remaining <0 then - core.log.error(remaining) return nil,"rejected" end return 0,remaining From e87d0b530a5b7f92007dfb5afa724891f3118950 Mon Sep 17 00:00:00 2001 From: yuchenghao Date: Wed, 7 Apr 2021 11:33:45 +0800 Subject: [PATCH 4/5] fix: code lint --- .../limit-count/limit-count-redis-cluster.lua | 22 ++++++++++--------- .../plugins/limit-count/limit-count-redis.lua | 20 +++++++++-------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/apisix/plugins/limit-count/limit-count-redis-cluster.lua b/apisix/plugins/limit-count/limit-count-redis-cluster.lua index d9df9953f591..68be4a0d6e2b 100644 --- a/apisix/plugins/limit-count/limit-count-redis-cluster.lua +++ b/apisix/plugins/limit-count/limit-count-redis-cluster.lua @@ -29,6 +29,13 @@ local mt = { } +local script = "if redis.call('ttl', KEYS[1]) < 0 then " + .. "redis.call('set', KEYS[1], ARGV[1] - 1, 'EX', ARGV[2]) " + .. "return ARGV[1] - 1 " + .. "end " + .. "return redis.call('incrby', KEYS[1], -1)" + + local function new_redis_cluster(conf) local config = { name = "apisix-redis-cluster", @@ -71,11 +78,6 @@ function _M.new(plugin_name, limit, window, conf) return setmetatable(self, mt) end -local script = "if redis.call('ttl',KEYS[1]) < 0 then " - .. "redis.call('set',KEYS[1],ARGV[1]-1,'EX',ARGV[2]) " - .. "return ARGV[1]-1 " - .. "end " - .. "return redis.call('incrby',KEYS[1],-1)" function _M.incoming(self, key) local red = self.red_cli @@ -83,16 +85,16 @@ function _M.incoming(self, key) local window = self.window key = self.plugin_name .. tostring(key) - local remaining,err = red:eval(script,1,key,limit,window) + local remaining, err = red:eval(script, 1, key, limit, window) if err then - return nil,err + return nil, err end - if remaining <0 then - return nil,"rejected" + if remaining < 0 then + return nil, "rejected" end - return 0,remaining + return 0, remaining end diff --git a/apisix/plugins/limit-count/limit-count-redis.lua b/apisix/plugins/limit-count/limit-count-redis.lua index 84b74e724750..5b8b6c0528ac 100644 --- a/apisix/plugins/limit-count/limit-count-redis.lua +++ b/apisix/plugins/limit-count/limit-count-redis.lua @@ -29,11 +29,11 @@ local mt = { } -local script = "if redis.call('ttl',KEYS[1]) < 0 then " - .. "redis.call('set',KEYS[1],ARGV[1]-1,'EX',ARGV[2]) " - .. "return ARGV[1]-1 " +local script = "if redis.call('ttl', KEYS[1]) < 0 then " + .. "redis.call('set', KEYS[1], ARGV[1] - 1, 'EX', ARGV[2]) " + .. "return ARGV[1] - 1 " .. "end " - .. "return redis.call('incrby',KEYS[1],-1)" + .. "return redis.call('incrby', KEYS[1], -1)" function _M.new(plugin_name, limit, window, conf) @@ -82,17 +82,19 @@ function _M.incoming(self, key) local limit = self.limit local window = self.window + local remaining key = self.plugin_name .. tostring(key) - local remaining, err = red:eval(script,1,key,limit,window) + + remaining, err = red:eval(script, 1, key, limit, window) if err then - return nil,err + return nil, err end - if remaining <0 then - return nil,"rejected" + if remaining < 0 then + return nil, "rejected" end - return 0,remaining + return 0, remaining end From a84533e98a866bceaf095e5468625072fac170e1 Mon Sep 17 00:00:00 2001 From: yuchenghao Date: Wed, 7 Apr 2021 20:40:56 +0800 Subject: [PATCH 5/5] fix: replace quotes with brackets --- .../limit-count/limit-count-redis-cluster.lua | 12 +++++++----- apisix/plugins/limit-count/limit-count-redis.lua | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/apisix/plugins/limit-count/limit-count-redis-cluster.lua b/apisix/plugins/limit-count/limit-count-redis-cluster.lua index 68be4a0d6e2b..a2da8bd61f26 100644 --- a/apisix/plugins/limit-count/limit-count-redis-cluster.lua +++ b/apisix/plugins/limit-count/limit-count-redis-cluster.lua @@ -29,11 +29,13 @@ local mt = { } -local script = "if redis.call('ttl', KEYS[1]) < 0 then " - .. "redis.call('set', KEYS[1], ARGV[1] - 1, 'EX', ARGV[2]) " - .. "return ARGV[1] - 1 " - .. "end " - .. "return redis.call('incrby', KEYS[1], -1)" +local script = [=[ + if redis.call('ttl', KEYS[1]) < 0 then + redis.call('set', KEYS[1], ARGV[1] - 1, 'EX', ARGV[2]) + return ARGV[1] - 1 + end + return redis.call('incrby', KEYS[1], -1) +]=] local function new_redis_cluster(conf) diff --git a/apisix/plugins/limit-count/limit-count-redis.lua b/apisix/plugins/limit-count/limit-count-redis.lua index 5b8b6c0528ac..7958bdc840d9 100644 --- a/apisix/plugins/limit-count/limit-count-redis.lua +++ b/apisix/plugins/limit-count/limit-count-redis.lua @@ -29,11 +29,13 @@ local mt = { } -local script = "if redis.call('ttl', KEYS[1]) < 0 then " - .. "redis.call('set', KEYS[1], ARGV[1] - 1, 'EX', ARGV[2]) " - .. "return ARGV[1] - 1 " - .. "end " - .. "return redis.call('incrby', KEYS[1], -1)" +local script = [=[ + if redis.call('ttl', KEYS[1]) < 0 then + redis.call('set', KEYS[1], ARGV[1] - 1, 'EX', ARGV[2]) + return ARGV[1] - 1 + end + return redis.call('incrby', KEYS[1], -1) +]=] function _M.new(plugin_name, limit, window, conf)