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

Jump to bottom

fix(limit-conn): always save the data of the limit object, and release it in log phase. #2465

Merged
merged 1 commit into from
Dec 10, 2020
Merged

fix(limit-conn): always save the data of the limit object, and release it in log phase. #2465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 3 additions & 4 deletions apisix/plugins/limit-conn.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ end

function _M.access(conf, ctx)
core.log.info("ver: ", ctx.conf_version)
local lim, err = core.lrucache.plugin_ctx(lrucache, ctx, nil,
create_limit_obj, conf)
local lim, err = lrucache(conf, nil, create_limit_obj, conf)
if not lim then
core.log.error("failed to instantiate a resty.limit.conn object: ", err)
return 500
Expand All @@ -88,9 +87,9 @@ function _M.access(conf, ctx)
if lim:is_committed() then
if not ctx.limit_conn then
ctx.limit_conn = core.tablepool.fetch("plugin#limit-conn", 0, 6)
else
core.table.insert_tail(ctx.limit_conn, lim, key, delay)
end

core.table.insert_tail(ctx.limit_conn, lim, key, delay)
end

if delay >= 0.001 then
Expand Down
68 changes: 63 additions & 5 deletions t/plugin/limit-conn.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,7 @@ done


=== TEST 26: create consumer and bind key-auth plugin
--- config
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
Expand Down Expand Up @@ -1019,7 +1019,7 @@ passed



=== TEST 27: create route and enable plugin 'key-auth'
=== TEST 27: create route and enable plugin 'key-auth'
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -1207,9 +1207,9 @@ qr/limit key: consumer_jackroute&consumer\d+/
content_by_lua_block {
local plugin = require("apisix.plugins.limit-conn")
local ok, err = plugin.check_schema({
conn = 1,
default_conn_delay = 0.1,
rejected_code = 503,
conn = 1,
default_conn_delay = 0.1,
rejected_code = 503,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why modify this test case？

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

old test cases have space at the end of line.

my editor helps me do the slim, this is fine.

image

key = 'consumer_name'
})
if not ok then
Expand All @@ -1225,3 +1225,61 @@ property "burst" is required
done
--- no_error_log
[error]



=== TEST 32: enable plugin: conn=1
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"plugins": {
"limit-conn": {
"conn": 1,
"burst": 0,
"default_conn_delay": 0.3,
"rejected_code": 503,
"key": "remote_addr"
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)
if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]



=== TEST 33: hit route and should not be limited
--- pipelined_requests eval
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the request be rejected here? I did not understand

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All requests are sent one after another, they are not concurrent.

So the request should never be limited for "conn": 1,.

In the old code, it will be limited due to this bug.

[
"GET /hello", "GET /hello", "GET /hello",
"GET /hello", "GET /hello", "GET /hello",
]
--- timeout: 10s
--- error_code eval
[
200, 200, 200,
200, 200, 200
]
--- no_error_log
[error]