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.

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

bug: 'limit-conn' plugin seems working in an unexpected manner #2985

Closed
yitian-reevo opened this issue Dec 7, 2020 · 7 comments
Closed

bug: 'limit-conn' plugin seems working in an unexpected manner #2985

yitian-reevo opened this issue Dec 7, 2020 · 7 comments

Comments

@yitian-reevo
Copy link
Contributor

yitian-reevo commented Dec 7, 2020

Issue description

'limit-conn' plugin seems working in an unexpected manner

Environment

  • apisix version (cmd: apisix version):2.1
  • OS: Centos 7.6

Minimal test code / Steps to reproduce the issue

  1. Setup APISIX correctly. make deps, make init, make run
  2. Add one route contains limit-conn:
curl -i -X PUT http://127.0.0.1:9080/apisix/admin/routes/1 -H'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -d '
{
    "uri": "/*",
    "methods": ["PUT", "GET", "POST", "DELETE", "PATCH", "HEAD", "OPTIONS", "CONNECT", "TRACE"],
	"plugins": {
        "limit-conn": {
            "conn": 100,
            "burst": 0,
            "default_conn_delay": 0.1,
            "rejected_code": 503,
            "key": "remote_addr"
            }
	},
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "10.10.10.40:8000": 10
        }
    }
}'
  1. Test concurrency. Keeps running ab -c 1 -n 40 http://127.0.0.1:9080/ at least 3 times (let the number of requests exceeds limit value: 100) or more.
    This command means to send 40 requests one by one.

What's the actual result? (including assertion message & call stack if applicable)

Starting from the third round, after sending the 100th request. All the following requests will be blocked with 503 Code. It looks like the limitation works in a cumulative manner, not in a cocurrent manner.

What's the expected result?

the limit should never be touched and all requests should return 200 OK.

@yitian-reevo yitian-reevo changed the title bug: 'limit-conn' plugin seems not working in an unexpected manner bug: 'limit-conn' plugin seems working in an unexpected manner Dec 8, 2020
@yitian-reevo
Copy link
Contributor Author

yitian-reevo commented Dec 8, 2020

https://github.com/apache/apisix/blob/v2.1/apisix/plugins/limit-conn.lua#L88-L94

I think these code should be something like this:

    if lim:is_committed() then
        if not ctx.limit_conn then
            ctx.limit_conn = core.tablepool.fetch("plugin#limit-conn", 0, 6)
        end

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

@spacewander
Copy link
Member

@wfgydbu
Would you try https://github.com/apache/apisix/pull/2465/files and report if the issue can be fixed by this?

@yitian-reevo
Copy link
Contributor Author

Sure. I'd like to.

@yitian-reevo
Copy link
Contributor Author

yitian-reevo commented Dec 8, 2020

@spacewander I have ran some tests based on your modifications in #2465, looks fine on my machine.

@spacewander
Copy link
Member

Good to know. BTW, it is @membphis 's modifications.
@membphis
Would you solve the conflict in #2465 ?

@membphis
Copy link
Member

membphis commented Dec 8, 2020

fixed the conflict right now ^_^

@spacewander
Copy link
Member

Solved by #2465

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants