Skip to content
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.

Sign up for GitHub

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

Improve rate limit policy #679

Closed
wants to merge 7 commits into from
Closed

Improve rate limit policy #679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6fad824
add redis initialization #667
y-tabata Apr 24, 2018
91b271e
use string.format and avoid global table lookup
y-tabata Apr 24, 2018
3bdad04
change default scope to service and use service_id in the context
y-tabata Apr 24, 2018
6314354
use definition in schema
y-tabata Apr 24, 2018
8713e9e
change os.time to ngx.time
y-tabata Apr 24, 2018
0397d67
change os.execute("sleep 1") to ngx.sleep(1)
y-tabata Apr 24, 2018
feae81d
add changelog
y-tabata Apr 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed

- Fixed set of valid values for the exit param of the Echo policy [PR #684](https://github.com/3scale/apicast/pull/684/)
### Changed
- Rate Limit policy to take into account changes in the config and improve performance [PR #679](https://github.com/3scale/apicast/pull/679)

## [3.2.0-rc1] - 2018-04-24

Expand Down
135 changes: 43 additions & 92 deletions gateway/src/apicast/policy/rate_limit/apicast-policy.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,44 @@
"version": "builtin",
"configuration": {
"type": "object",
"definitions": {
"key": {
"$id": "#/definitions/key",
"description": "The key corresponding to the limiter object",
"type": "object",
"properties": {
"name": {
"type": "string",
"description": "The name of the key, must be unique in the scope"
},
"scope": {
"type": "string",
"description": "Scope of the key",
"default": "service",
"oneOf": [{
"enum": ["global"],
"description": "Global scope, affecting to all services"
}, {
"enum": ["service"],
"description": "Service scope, affecting to one service"
}]
}
}
},
"error_handling": {
"$id": "#/definitions/error_handling",
"type": "string",
"description": "How to handle an error",
"default": "exit",
"oneOf": [{
"enum": ["exit"],
"description": "Respond with an error"
}, {
"enum": ["log"],
"description": "Let the request go through and only output logs"
}]
}
},
"properties": {
"limiters": {
"description": "List of limiters to be applied",
Expand All @@ -20,30 +58,7 @@
"description": "Limiting request concurrency (or concurrent connections)"
},
"key": {
"description": "The key corresponding to the limiter object",
"type": "object",
"properties": {
"name": {
"type": "string",
"description": "The name of the key, must be unique in the scope"
},
"scope": {
"type": "string",
"description": "Scope of the key",
"default": "global",
"oneOf": [{
"enum": ["global"],
"description": "Global scope, affecting to all services"
}, {
"enum": ["service"],
"description": "Service scope, affecting to one service"
}]
},
"service_name": {
"type": "string",
"description": "Name of service, necessary for service scope"
}
}
"$ref": "#/definitions/key"
},
"conn": {
"type": "integer",
Expand All @@ -70,30 +85,7 @@
"description": "Limiting request rate"
},
"key": {
"description": "The key corresponding to the limiter object",
"type": "object",
"properties": {
"name": {
"type": "string",
"description": "The name of the key, must be unique in the scope"
},
"scope": {
"type": "string",
"description": "Scope of the key",
"default": "global",
"oneOf": [{
"enum": ["global"],
"description": "Global scope, affecting to all services"
}, {
"enum": ["service"],
"description": "Service scope, affecting to one service"
}]
},
"service_name": {
"type": "string",
"description": "Name of service, necessary for service scope"
}
}
"$ref": "#/definitions/key"
},
"rate": {
"type": "integer",
Expand All @@ -115,30 +107,7 @@
"description": "Limiting request counts"
},
"key": {
"description": "The key corresponding to the limiter object",
"type": "object",
"properties": {
"name": {
"type": "string",
"description": "The name of the key, must be unique in the scope"
},
"scope": {
"type": "string",
"description": "Scope of the key",
"default": "global",
"oneOf": [{
"enum": ["global"],
"description": "Global scope, affecting to all services"
}, {
"enum": ["service"],
"description": "Service scope, affecting to one service"
}]
},
"service_name": {
"type": "string",
"description": "Name of service, necessary for service scope"
}
}
"$ref": "#/definitions/key"
},
"count": {
"type": "integer",
Expand Down Expand Up @@ -176,16 +145,7 @@
"default": 429
},
"error_handling": {
"type": "string",
"description": "How to handle an error",
"default": "exit",
"oneOf": [{
"enum": ["exit"],
"description": "Respond with an error"
}, {
"enum": ["log"],
"description": "Let the request go through and only output logs"
}]
"$ref": "#/definitions/error_handling"
}
}
}, {
Expand All @@ -202,16 +162,7 @@
"default": 500
},
"error_handling": {
"type": "string",
"description": "How to handle an error",
"default": "exit",
"oneOf": [{
"enum": ["exit"],
"description": "Respond with an error"
}, {
"enum": ["log"],
"description": "Let the request go through and only output logs"
}]
"$ref": "#/definitions/error_handling"
}
}
}]
Expand Down
58 changes: 50 additions & 8 deletions gateway/src/apicast/policy/rate_limit/rate_limit.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ local limit_traffic = require "resty.limit.traffic"
local ts = require ('apicast.threescale_utils')
local tonumber = tonumber
local next = next
local format = string.format
local insert = table.insert
local shdict_key = 'limiter'

local new = _M.new
Expand Down Expand Up @@ -68,6 +70,9 @@ local function redis_shdict(url)
return nil
end
return val
end,
del = function(_, key)
return redis:del(key)
end
}
end
Expand Down Expand Up @@ -95,17 +100,52 @@ local function init_error_settings(config_error_settings)
return error_settings
end

local function initialize_redis_records(redis, seed, limiters, service_id)
for _, limiter in ipairs(limiters) do
local key
if limiter.key.scope == "global" then
key = format("%s_%s", limiter.name, limiter.key.name)
else
key = format("%s_%s_%s", service_id, limiter.name, limiter.key.name)
end

local seed_key = format("%s_seed", key)
local seed_value = redis:get(seed_key)

if not seed_value or tonumber(seed_value) ~= seed then
redis:set(seed_key, seed)

if limiter.name == "connections" or limiter.name == "leaky_bucket" then
redis:del(key)
else
local count_key = format("%s_count", key)
local count = redis:get(count_key)

local window_key = format("%s_window", key)
local window = redis:get(window_key)

if not count or not window or tonumber(count) ~= limiter.count or tonumber(window) ~= limiter.window then
redis:del(key)
redis:set(count_key, limiter.count)
redis:set(window_key, limiter.window)
end
end
end
end
end

function _M.new(config)
local self = new()
self.config = config or {}
self.limiters = config.limiters
self.redis_url = config.redis_url
self.error_settings = init_error_settings(config.error_settings)
self.seed = ngx.time()

return self
end

function _M:access()
function _M:access(context)
local limiters = {}
local keys = {}

Expand All @@ -118,6 +158,8 @@ function _M:access()
error(self.error_settings, "configuration_issue")
return
end

initialize_redis_records(red, self.seed, self.limiters, context.service.id)
end

for _, limiter in ipairs(self.limiters) do
Expand All @@ -130,16 +172,16 @@ function _M:access()

lim.dict = red or lim.dict

table.insert(limiters, lim)
insert(limiters, lim)

local key
if limiter.key.scope == "service" then
key = limiter.key.service_name.."_"..limiter.name.."_"..limiter.key.name
if limiter.key.scope == "global" then
key = format("%s_%s", limiter.name, limiter.key.name)
else
key = limiter.name.."_"..limiter.key.name
key = format("%s_%s_%s", context.service.id, limiter.name, limiter.key.name)
end

table.insert(keys, key)
insert(keys, key)

end

Expand All @@ -161,8 +203,8 @@ function _M:access()

for i, lim in ipairs(limiters) do
if lim.is_committed and lim:is_committed() then
table.insert(connections_committed, lim)
table.insert(keys_committed, keys[i])
insert(connections_committed, lim)
insert(keys_committed, keys[i])
end
end

Expand Down
Loading