You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/08/07 07:02:49 UTC
[GitHub] [apisix] okaybase opened a new pull request #4774: feat(plugin): add degradation switch for traffic control plugin
okaybase opened a new pull request #4774:
URL: https://github.com/apache/apisix/pull/4774
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Add degradation switch for traffic control plugin.
When the limit function is temporarily unavailable(e.g. redis timeout for limit-count plugin), we can allow requests to continue by enable degradation switch.
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4774: feat(plugin): add degradation switch for traffic control plugin
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4774:
URL: https://github.com/apache/apisix/pull/4774#discussion_r684770012
##########
File path: apisix/plugins/limit-conn.lua
##########
@@ -33,6 +33,7 @@ local schema = {
rejected_code = {
type = "integer", minimum = 200, maximum = 599, default = 503
},
+ degradation = {type = "boolean", default = false}
Review comment:
done
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #4774: feat(plugin): add degradation switch for traffic control plugin
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #4774:
URL: https://github.com/apache/apisix/pull/4774
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4774: feat(plugin): add degradation switch for traffic control plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4774:
URL: https://github.com/apache/apisix/pull/4774#discussion_r684768302
##########
File path: t/plugin/limit-count-redis-cluster.t
##########
@@ -377,3 +377,62 @@ code: 503
code: 503
--- no_error_log
[error]
+
+
+
+=== TEST 10: set route, four redis nodes, no one is valid, with enable degradation switch
+--- 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,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 9999,
+ "time_window": 60,
+ "key": "http_x_real_ip",
+ "policy": "redis-cluster",
+ "degradation": true,
+ "redis_cluster_nodes": [
+ "127.0.0.1:8001",
+ "127.0.0.1:8002",
+ "127.0.0.1:8003",
+ "127.0.0.1:8004"
+ ],
+ "redis_cluster_name": "redis-cluster-1"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 11: enable degradation switch for TEST 10
+--- request
+GET /hello
+--- error_code eval
Review comment:
We don't need to check 200 code - it is checked by default
##########
File path: apisix/plugins/limit-conn.lua
##########
@@ -33,6 +33,7 @@ local schema = {
rejected_code = {
type = "integer", minimum = 200, maximum = 599, default = 503
},
+ degradation = {type = "boolean", default = false}
Review comment:
Would be better to use "allow_degradation"?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on pull request #4774: feat(plugin): add degradation switch for traffic control plugin
Posted by GitBox <gi...@apache.org>.
okaybase commented on pull request #4774:
URL: https://github.com/apache/apisix/pull/4774#issuecomment-894733618
> LGTM
Thanks for your review~ @nic-chen
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on pull request #4774: feat(plugin): add degradation switch for traffic control plugin
Posted by GitBox <gi...@apache.org>.
okaybase commented on pull request #4774:
URL: https://github.com/apache/apisix/pull/4774#issuecomment-894801824
> LGTM. Let's add new test in t/plugin/limit-conn.t
new test added, and improve a log info. @spacewander
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4774: feat(plugin): add degradation switch for traffic control plugin
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4774:
URL: https://github.com/apache/apisix/pull/4774#discussion_r684770068
##########
File path: t/plugin/limit-count-redis-cluster.t
##########
@@ -377,3 +377,62 @@ code: 503
code: 503
--- no_error_log
[error]
+
+
+
+=== TEST 10: set route, four redis nodes, no one is valid, with enable degradation switch
+--- 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,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 9999,
+ "time_window": 60,
+ "key": "http_x_real_ip",
+ "policy": "redis-cluster",
+ "degradation": true,
+ "redis_cluster_nodes": [
+ "127.0.0.1:8001",
+ "127.0.0.1:8002",
+ "127.0.0.1:8003",
+ "127.0.0.1:8004"
+ ],
+ "redis_cluster_name": "redis-cluster-1"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 11: enable degradation switch for TEST 10
+--- request
+GET /hello
+--- error_code eval
Review comment:
done
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #4774: feat(plugin): add degradation switch for traffic control plugin
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #4774:
URL: https://github.com/apache/apisix/pull/4774#discussion_r684700344
##########
File path: apisix/plugins/limit-conn/init.lua
##########
@@ -41,7 +41,11 @@ function _M.increase(conf, ctx)
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
+ if conf.degradation then
+ return
+ else
+ return 500
+ end
Review comment:
```suggestion
if conf.degradation then
return
end
return 500
```
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org