From 18a645dcf2e01405cc53e440b2ae7841e894eb0a Mon Sep 17 00:00:00 2001 From: wxbty Date: Wed, 14 Jun 2023 17:51:46 +0800 Subject: [PATCH 1/2] fix: only enable upstream_response_time for http Signed-off-by: Sn0rt --- apisix/plugins/limit-conn/init.lua | 13 +-- t/stream-plugin/limit-conn2.t | 134 +++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 t/stream-plugin/limit-conn2.t diff --git a/apisix/plugins/limit-conn/init.lua b/apisix/plugins/limit-conn/init.lua index 3337980fd815..c6ce55f240d0 100644 --- a/apisix/plugins/limit-conn/init.lua +++ b/apisix/plugins/limit-conn/init.lua @@ -16,6 +16,7 @@ -- local limit_conn_new = require("resty.limit.conn").new local core = require("apisix.core") +local is_http = ngx.config.subsystem == "http" local sleep = core.sleep local shdict_name = "plugin-limit-conn" if ngx.config.subsystem == "stream" then @@ -115,11 +116,13 @@ function _M.decrease(conf, ctx) local use_delay = limit_conn[i + 3] local latency - if not use_delay then - if ctx.proxy_passed then - latency = ctx.var.upstream_response_time - else - latency = ctx.var.request_time - delay + if is_http then + if not use_delay then + if ctx.proxy_passed then + latency = ctx.var.upstream_response_time + else + latency = ctx.var.request_time - delay + end end end core.log.debug("request latency is ", latency) -- for test diff --git a/t/stream-plugin/limit-conn2.t b/t/stream-plugin/limit-conn2.t new file mode 100644 index 000000000000..07266eb3e1e1 --- /dev/null +++ b/t/stream-plugin/limit-conn2.t @@ -0,0 +1,134 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX; + +my $nginx_binary = $ENV{'TEST_NGINX_BINARY'} || 'nginx'; +my $version = eval { `$nginx_binary -V 2>&1` }; + +if ($version !~ m/\/apisix-nginx-module/) { + plan(skip_all => "apisix-nginx-module not installed"); +} else { + plan('no_plan'); +} + +$ENV{TEST_NGINX_REDIS_PORT} ||= 1985; + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!$block->extra_yaml_config) { + my $extra_yaml_config = <<_EOC_; +xrpc: + protocols: + - name: redis +_EOC_ + $block->set_value("extra_yaml_config", $extra_yaml_config); + } + + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } + + $block; +}); + +worker_connections(1024); +run_tests; + +__DATA__ + +=== TEST 1: init +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/stream_routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "limit-conn": { + "conn": 2, + "burst": 1, + "default_conn_delay": 0.1, + "key": "$remote_port $server_addr", + "key_type": "var_combination" + } + }, + "upstream": { + "type": "none", + "nodes": { + "127.0.0.1:6379": 1 + } + }, + "protocol": { + "name": "redis" + } + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 2: hit +--- config + location /t { + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local res, err = red:hmset("animals", "dog", "bark", "cat", "meow") + if not res then + ngx.say("failed to set animals: ", err) + return + end + ngx.say("hmset animals: ", res) + + local res, err = red:hmget("animals", "dog", "cat") + if not res then + ngx.say("failed to get animals: ", err) + return + end + ngx.say("hmget animals: ", res) + + ok, err = red:close() + if not ok then + ngx.say("failed to close: ", err) + return + end + } + } +--- response_body +hmset animals: OK +hmget animals: barkmeow +--- no_error_log +attempt to perform arithmetic on field 'request_time' +--- stream_conf_enable From 6c92d5e568e0486403e4ef747652c0096dd4ed3d Mon Sep 17 00:00:00 2001 From: Sn0rt Date: Tue, 11 Jul 2023 16:30:28 +0800 Subject: [PATCH 2/2] fix: rename the test case Signed-off-by: Sn0rt --- t/stream-plugin/limit-conn2.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/stream-plugin/limit-conn2.t b/t/stream-plugin/limit-conn2.t index 07266eb3e1e1..9efb2b6dfb1f 100644 --- a/t/stream-plugin/limit-conn2.t +++ b/t/stream-plugin/limit-conn2.t @@ -52,7 +52,7 @@ run_tests; __DATA__ -=== TEST 1: init +=== TEST 1: create a stream router with limit-conn --- config location /t { content_by_lua_block { @@ -92,7 +92,7 @@ passed -=== TEST 2: hit +=== TEST 2: access the redis via proxy --- config location /t { content_by_lua_block {