You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by me...@apache.org on 2020/07/29 11:21:59 UTC

[incubator-apisix] branch master updated: feature: allow to set `retries=0` for upstream object. (#1919)

This is an automated email from the ASF dual-hosted git repository.

membphis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-apisix.git


The following commit(s) were added to refs/heads/master by this push:
     new fbc3c68  feature: allow to set `retries=0` for upstream object. (#1919)
fbc3c68 is described below

commit fbc3c6818d7650bd558502719704614ff54c951b
Author: YuanSheng Wang <me...@gmail.com>
AuthorDate: Wed Jul 29 19:21:50 2020 +0800

    feature: allow to set `retries=0` for upstream object. (#1919)
---
 apisix/balancer.lua       |   9 +-
 apisix/schema_def.lua     |   2 +-
 doc/admin-api.md          |   4 +-
 doc/zh-cn/admin-api.md    |   4 +-
 t/admin/upstream.t        |  63 ++++++++++++++
 t/node/upstream-retries.t | 211 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 285 insertions(+), 8 deletions(-)

diff --git a/apisix/balancer.lua b/apisix/balancer.lua
index 7542572..145856f 100644
--- a/apisix/balancer.lua
+++ b/apisix/balancer.lua
@@ -211,10 +211,13 @@ local function pick_server(route, ctx)
 
     if ctx.balancer_try_count == 1 then
         local retries = up_conf.retries
-        if not retries or retries <= 0 then
-            retries = #up_conf.nodes
+        if not retries or retries < 0 then
+            retries = #up_conf.nodes - 1
+        end
+
+        if retries > 0 then
+            set_more_tries(retries)
         end
-        set_more_tries(retries)
     end
 
     if checker then
diff --git a/apisix/schema_def.lua b/apisix/schema_def.lua
index f16e41b..0a8993b 100644
--- a/apisix/schema_def.lua
+++ b/apisix/schema_def.lua
@@ -278,7 +278,7 @@ local upstream_schema = {
         nodes = nodes_schema,
         retries = {
             type = "integer",
-            minimum = 1,
+            minimum = 0,
         },
         timeout = {
             type = "object",
diff --git a/doc/admin-api.md b/doc/admin-api.md
index b1112e9..caa3b1d 100644
--- a/doc/admin-api.md
+++ b/doc/admin-api.md
@@ -344,7 +344,7 @@ In addition to the basic complex equalization algorithm selection, APISIX's Upst
 |hash_on         |optional|This option is only valid if the `type` is `chash`. Supported types `vars`(Nginx variables), `header`(custom header), `cookie`, `consumer`, the default value is `vars`.|
 |key             |required|This option is only valid if the `type` is `chash`. Find the corresponding node `id` according to `hash_on` and `key`. When `hash_on` is set as `vars`, `key` is the required parameter, for now, it support nginx built-in variables like `uri, server_name, server_addr, request_uri, remote_port, remote_addr, query_string, host, hostname, arg_***`, `arg_***` is arguments in the request line, [Nginx variables list](http://nginx.org/en/docs/varindex.html). When `hash_ [...]
 |checks          |optional|Configure the parameters of the health check. For details, refer to [health-check](health-check.md).|
-|retries         |optional|Pass the request to the next upstream using the underlying Nginx retry mechanism, the retry mechanism is enabled by default and set the number of retries according to the number of backend nodes. If `retries` option is explicitly set, it will override the default value.|
+|retries         |optional|Pass the request to the next upstream using the underlying Nginx retry mechanism, the retry mechanism is enabled by default and set the number of retries according to the number of backend nodes. If `retries` option is explicitly set, it will override the default value. `0` means disable retry mechanism.|
 |enable_websocket|optional| enable `websocket`(boolean), default `false`.|
 |timeout|optional| Set the timeout for connection, sending and receiving messages. |
 |name     |optional|Identifies upstream names|
@@ -355,7 +355,7 @@ Config Example:
 ```shell
 {
     "id": "1",                  # id
-    "retries": 0,               # retry time
+    "retries": 1,               # retry times
     "timeout": {                # Set the timeout for connection, sending and receiving messages.
         "connect":15,
         "send":15,
diff --git a/doc/zh-cn/admin-api.md b/doc/zh-cn/admin-api.md
index 1625661..198de1c 100644
--- a/doc/zh-cn/admin-api.md
+++ b/doc/zh-cn/admin-api.md
@@ -353,7 +353,7 @@ APISIX 的 Upstream 除了基本的复杂均衡算法选择外,还支持对上
 |type            |必需|枚举|`roundrobin` 支持权重的负载,`chash` 一致性哈希,两者是二选一的|`roundrobin`||
 |key             |条件必需|匹配类型|该选项只有类型是 `chash` 才有效。根据 `key` 来查找对应的 node `id`,相同的 `key` 在同一个对象中,永远返回相同 id,目前支持的 Nginx 内置变量有 `uri, server_name, server_addr, request_uri, remote_port, remote_addr, query_string, host, hostname, arg_***`,其中 `arg_***` 是来自URL的请求参数,[Nginx 变量列表](http://nginx.org/en/docs/varindex.html)||
 |checks          |可选|health_checker|配置健康检查的参数,详细可参考[health-check](../health-check.md)||
-|retries         |可选|整型|使用底层的 Nginx 重试机制将请求传递给下一个上游,默认不启用重试机制||
+|retries         |可选|整型|使用底层的 Nginx 重试机制将请求传递给下一个上游,默认启用重试且次数为后端 node 数量。如果指定了具体重试次数,它将覆盖默认值。`0` 代表不启用重试机制||
 |timeout         |可选|超时时间对象|设置连接、发送消息、接收消息的超时时间||
 |enable_websocket     |可选 |辅助|是否允许启用 websocket 能力||
 |hash_on     |可选 |辅助|该参数作为一致性 hash 的入参||
@@ -365,7 +365,7 @@ upstream 对象 json 配置内容:
 ```shell
 {
     "id": "1",                  # id
-    "retries": 0,               # 请求重试次数
+    "retries": 1,               # 请求重试次数
     "timeout": {                # 设置连接、发送消息、接收消息的超时时间
         "connect":15,
         "send":15,
diff --git a/t/admin/upstream.t b/t/admin/upstream.t
index d1167ab..14ccfd9 100644
--- a/t/admin/upstream.t
+++ b/t/admin/upstream.t
@@ -1361,3 +1361,66 @@ GET /t
 {"error_msg":"invalid configuration: property \"id\" validation failed: object matches none of the requireds"}
 --- no_error_log
 [error]
+
+
+
+=== TEST 42: retries is 0
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/upstreams/a-b-c-ABC_0123',
+                ngx.HTTP_PUT,
+                [[{
+                    "nodes": {
+                        "127.0.0.1:8080": 1,
+                        "127.0.0.1:8090": 1
+                    },
+                    "retries": 0,
+                    "type": "roundrobin"
+                }]]
+            )
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 43: retries is -1 (INVALID)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/upstreams/a-b-c-ABC_0123',
+                ngx.HTTP_PUT,
+                [[{
+                    "nodes": {
+                        "127.0.0.1:8080": 1,
+                        "127.0.0.1:8090": 1
+                    },
+                    "retries": -1,
+                    "type": "roundrobin"
+                }]]
+            )
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.print(body)
+        }
+    }
+--- request
+GET /t
+--- error_code: 400
+--- response_body
+{"error_msg":"invalid configuration: property \"retries\" validation failed: expected -1 to be greater than 0"}
+--- no_error_log
+[error]
diff --git a/t/node/upstream-retries.t b/t/node/upstream-retries.t
new file mode 100644
index 0000000..81f90fc
--- /dev/null
+++ b/t/node/upstream-retries.t
@@ -0,0 +1,211 @@
+#
+# 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 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: set upstream(id: 1), no retries
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/upstreams/1',
+                ngx.HTTP_PUT,
+                [[{
+                    "nodes": {
+                        "127.0.0.1:1": 1,
+                        "127.0.0.2:1": 1,
+                        "127.0.0.3:1": 1,
+                        "127.0.0.4:1": 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 2: set route(id: 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,
+                 [[{
+                        "uri": "/hello",
+                        "upstream_id": "1"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: /not_found
+--- request
+GET /not_found
+--- error_code: 404
+--- response_body
+{"error_msg":"failed to match any routes"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: hit routes
+--- request
+GET /hello
+--- error_code: 502
+--- grep_error_log eval
+qr/\[error\]/
+--- grep_error_log_out
+[error]
+[error]
+[error]
+[error]
+
+
+
+=== TEST 5: hit routes
+--- request
+GET /hello
+--- error_code: 502
+--- error_log
+connect() failed
+
+
+
+=== TEST 6: set upstream(id: 1), retries = 1
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/upstreams/1',
+                ngx.HTTP_PUT,
+                [[{
+                    "nodes": {
+                        "127.0.0.1:1": 1,
+                        "127.0.0.2:1": 1,
+                        "127.0.0.3:1": 1,
+                        "127.0.0.4:1": 1
+                    },
+                    "retries": 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 7: hit routes
+--- request
+GET /hello
+--- error_code: 502
+--- grep_error_log eval
+qr/\[error\]/
+--- grep_error_log_out
+[error]
+[error]
+
+
+
+=== TEST 8: set upstream(id: 1), retries = 0
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/upstreams/1',
+                ngx.HTTP_PUT,
+                [[{
+                    "nodes": {
+                        "127.0.0.1:1": 1,
+                        "127.0.0.2:1": 1,
+                        "127.0.0.3:1": 1,
+                        "127.0.0.4:1": 1
+                    },
+                    "retries": 0,
+                    "type": "roundrobin"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: hit routes
+--- request
+GET /hello
+--- error_code: 502
+--- grep_error_log eval
+qr/\[error\]/
+--- grep_error_log_out
+[error]