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 report error attribute does not exist #9661

Closed
wxbty opened this issue Jun 14, 2023 · 9 comments · Fixed by #9816
Closed

bug: limit-conn report error attribute does not exist #9661

wxbty opened this issue Jun 14, 2023 · 9 comments · Fixed by #9816
Assignees
Labels
bug Something isn't working

Comments

@wxbty
Copy link
Member

wxbty commented Jun 14, 2023

Current Behavior

When adding stream_routes for the redis protocol, if the limit-conn plugin is used, an error will be reported

Expected Behavior

created route successfully

Error Logs

2023/06/14 17:35:07 [error] 30109#30109: 773 failed to run log_by_lua: /opt/linyao/apisix/apisix/plugins/limit-conn/init.lua:123: attempt to perform arithmetic on field 'request_time' (a nil value)
stack traceback:
/opt/linyao/apisix/apisix/plugins/limit-conn/init.lua:123: in function 'phase_func'
/opt/linyao/apisix/apisix/plugin.lua:1134: in function 'run_plugin'
/opt/linyao/apisix/apisix/init.lua:1083: in function 'stream_log_phase'
log_by_lua(nginx.conf:108):2: in main chunk while prereading client data, client: 127.0.0.1, server: 0.0.0.0:9100
2023/06/14 17:35:12 [error] 30113#30113: *2961 stream [lua] init.lua:106: phase_func(): decrease_to record the connection leaving request while prereading client data, client: 127.0.0.1, server: 0.0.0.0:9100
2023/06/14 17:35:12 [error] 30113#30113: 2961 failed to run log_by_lua: /opt/linyao/apisix/apisix/plugins/limit-conn/init.lua:123: attempt to perform arithmetic on field 'request_time' (a nil value)
stack traceback:
/opt/linyao/apisix/apisix/plugins/limit-conn/init.lua:123: in function 'phase_func'
/opt/linyao/apisix/apisix/plugin.lua:1134: in function 'run_plugin'
/opt/linyao/apisix/apisix/init.lua:1083: in function 'stream_log_phase'
log_by_lua(nginx.conf:108):2: in main chunk while prereading client data, client: 127.0.0.1, server: 0.0.0.0:9100

Steps to Reproduce

  1. Tcp dynamic proxy uses redis protocol
  2. Create a route with the limit-conn plugin as follow
    curl http://127.0.0.1:9180/apisix/admin/stream_routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
    {
    "plugins": {
    "limit-conn": {
    "conn": 5,
    "burst": 0,
    "default_conn_delay": 0.1,
    "rejected_code": 503,
    "key_type": "var",
    "key": "remote_addr"
    }
    },
    "upstream": {
    "type": "none",
    "nodes": {
    "127.0.0.1:6379": 1
    }
    },
    "protocol": {
    "name": "redis"
    }
    }
    '
  3. Check the error log and find the error message

Environment

APISIX version (run apisix version): master latest
Operating system (run uname -a): centos7
OpenResty / Nginx version (run openresty -V or nginx -V): openresty/1.21.4.1
etcd version, if relevant (run curl http://127.0.0.1:9090/v1/server_info): 3.5.0
APISIX Dashboard version, if relevant:
Plugin runner version, for issues related to plugin runners:
LuaRocks version, for installation issues (run luarocks --version):

@shreemaan-abhishek
Copy link
Contributor

let me check.

@shreemaan-abhishek
Copy link
Contributor

It works on my machine. I tested on the local build of the apisix, how did you install apisix?

@wxbty
Copy link
Member Author

wxbty commented Jun 16, 2023

The way to compile from source. I checked the relevant code, the request_time attribute is defined in the route of http, but it does not seem to be in the route of stream.

@lingsamuel lingsamuel added the bug Something isn't working label Jun 19, 2023
@moonming moonming moved this to 📋 Backlog in Apache APISIX backlog Jun 19, 2023
@wxbty
Copy link
Member Author

wxbty commented Jun 25, 2023

There may be a problem with the scene that appears, and the error occurs when the connection is disconnected
image

@shreemaan-abhishek
Copy link
Contributor

the error occurs when the connection is disconnected

Please guide me on how I can recreate this environment/situation on my machine.

@wxbty
Copy link
Member Author

wxbty commented Jun 25, 2023

Steps

Follow the steps to create a redis stream proxy normally, then connect to use redis, and finally exit and disconnect the redis connection to observe the log

@moonming moonming moved this from 📋 Backlog to 👀 In review in Apache APISIX backlog Jun 27, 2023
@shreemaan-abhishek
Copy link
Contributor

yes, I could reproduce the bug 👍🏼

@Sn0rt
Copy link
Contributor

Sn0rt commented Jul 10, 2023

reproduce guide

install redis

$ sudo apt install redis                                                                                                  100 [09:23:03]
[sudo] password for guohao:
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
redis is already the newest version (5:6.0.16-1ubuntu1).
The following package was automatically installed and is no longer required:
  openjdk-11-jre
Use 'sudo apt autoremove' to remove it.
0 upgraded, 0 newly installed, 0 to remove and 17 not upgraded.

enable stream proxy

diff --git a/conf/config-default.yaml b/conf/config-default.yaml
index e40dc174..3d8dd4c4 100755
--- a/conf/config-default.yaml
+++ b/conf/config-default.yaml
@@ -73,15 +73,15 @@ apisix:
                                 # radixtree_uri_with_parameter: similar to radixtree_uri but match URI with parameters. See https://github.com/api7/lua-resty-radixtree/#parameters-in-path for more details.
     ssl: radixtree_sni          # radixtree_sni: match route by SNI

-  # stream_proxy:                 # TCP/UDP L4 proxy
-  #   only: true                  # Enable L4 proxy only without L7 proxy.
-  #   tcp:
-  #     - addr: 9100              # Set the TCP proxy listening ports.
-  #       tls: true
-  #     - addr: "127.0.0.1:9101"
-  #   udp:                        # Set the UDP proxy listening ports.
-  #     - 9200
-  #     - "127.0.0.1:9201"
+  stream_proxy:                 # TCP/UDP L4 proxy
+     only: true                  # Enable L4 proxy only without L7 proxy.
+     tcp:
+       - addr: 9100              # Set the TCP proxy listening ports.
+         tls: true
+       - addr: "127.0.0.1:9101"
+     udp:                        # Set the UDP proxy listening ports.
+       - 9200
+       - "127.0.0.1:9201"

   # dns_resolver:                 # If not set, read from `/etc/resolv.conf`
   #   - 1.1.1.1
@@ -141,7 +141,7 @@ nginx_config:                     # Config for render the template to generate n
   # user: root                    # Set the execution user of the worker process. This is only
                                   # effective if the master process runs with super-user privileges.
   error_log: logs/error.log       # Location of the error log.
-  error_log_level:  warn          # Logging level: info, debug, notice, warn, error, crit, alert, or emerg.
+  error_log_level:  debug          # Logging level: info, debug, notice, warn, error, crit, alert, or emerg.
   worker_processes: auto          # Automatically determine the optimal number of worker processes based

set the router

curl http://127.0.0.1:9180/apisix/admin/stream_routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
  "plugins": {
    "limit-conn": {
      "conn": 1,
      "burst": 0,
      "default_conn_delay": 0.1,
      "rejected_code": 503,
      "key_type": "var",
      "key": "remote_addr"
    }
  },
  "upstream": {
    "type": "none",
    "nodes": {
      "127.0.0.1:6379": 1
    }
  }
}
'

reload the APISIX instance

$ make reload                                                                                                                 [09:24:37]
[ info ] reload -> [ Start ]
/home/guohao/apisix/bin/apisix reload
/usr/local/openresty//luajit/bin/luajit ./apisix/cli/apisix.lua reload
Warning! Current maximum number of open file descriptors [1024] is not greater than 1024, please increase user limits by execute 'ulimit -n <new user limits>' , otherwise the performance is low.
[ info ] reload -> [ Done ]

test the stream proxy

$ redis-cli -h localhost -p 9101                                                                                              [09:24:53]
localhost:9101> set key test
OK
localhost:9101> get key
"test"
image

now update the

 #       priority: 7999
 #       file: t/wasm/log/main.go.wasm

-# xrpc:
-#   protocols:
-#     - name: pingpong
+xrpc:
+  protocols:
+  - name: redis
+

already reproduce

2023/07/10 18:44:48 [info] 41973#1389691: *574985 stream [lua] init.lua:1114: stream_log_phase(): enter stream_log_phase while prereading client data, client: 127.0.0.1, server: 127.0.0.1:9101
2023/07/10 18:44:48 [error] 41973#1389691: *574985 failed to run log_by_lua*: ...ohao/workspace/apisix/apisix/plugins/limit-conn/init.lua:122: attempt to perform arithmetic on field 'request_time' (a nil value)
stack traceback:
	...ohao/workspace/apisix/apisix/plugins/limit-conn/init.lua:122: in function 'phase_func'
	/Users/guohao/workspace/apisix/apisix/plugin.lua:1134: in function 'run_plugin'
	/Users/guohao/workspace/apisix/apisix/init.lua:1116: in function 'stream_log_phase'
	log_by_lua(nginx.conf:113):2: in main chunk while prereading client data, client: 127.0.0.1, server: 127.0.0.1:9101
2023/07/10 18:44:49 [info] 41975#1389701: *575076 [lua] timers.lua:39: run timer[plugin#server-info], context: ngx.timer

@Sn0rt
Copy link
Contributor

Sn0rt commented Jul 11, 2023

the test case for reproduce

#
# 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
        }
    }
--- error_log
attempt to perform arithmetic on field 'request_time'
--- stream_conf_enable

and

$ prove -I. -I../test-nginx/inc -I../test-nginx/lib -r t/stream-plugin/limit-conn2.t
t/stream-plugin/limit-conn2.t .. ok
All tests successful.
Files=1, Tests=5,  1 wallclock secs ( 0.01 usr  0.00 sys +  0.21 cusr  0.08 csys =  0.30 CPU)
Result: PASS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
4 participants