From ab627e15511bb0c664e5310ede344c5d8fafda42 Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Tue, 28 Jul 2020 23:20:46 +0800 Subject: [PATCH 1/3] change: allow to set `retries=0` for upstream node. --- apisix/balancer.lua | 7 +++-- apisix/schema_def.lua | 2 +- t/admin/upstream.t | 63 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/apisix/balancer.lua b/apisix/balancer.lua index cd9edcaac5f5..849daaafa39e 100644 --- a/apisix/balancer.lua +++ b/apisix/balancer.lua @@ -208,10 +208,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 + if not retries or retries < 0 then retries = #up_conf.nodes end - set_more_tries(retries) + + if retries > 0 then + set_more_tries(retries) + end end if checker then diff --git a/apisix/schema_def.lua b/apisix/schema_def.lua index 813395371a6b..c70c584492b6 100644 --- a/apisix/schema_def.lua +++ b/apisix/schema_def.lua @@ -273,7 +273,7 @@ local upstream_schema = { nodes = nodes_schema, retries = { type = "integer", - minimum = 1, + minimum = 0, }, timeout = { type = "object", diff --git a/t/admin/upstream.t b/t/admin/upstream.t index d1167ab80418..14ccfd968137 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] From ece197cff0cef06bc69bd57f40790787dd98f019 Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Tue, 28 Jul 2020 23:37:32 +0800 Subject: [PATCH 2/3] test: added test cases. --- apisix/balancer.lua | 2 +- t/node/upstream-retries.t | 211 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 t/node/upstream-retries.t diff --git a/apisix/balancer.lua b/apisix/balancer.lua index 849daaafa39e..7899b84ceff1 100644 --- a/apisix/balancer.lua +++ b/apisix/balancer.lua @@ -209,7 +209,7 @@ 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 + retries = #up_conf.nodes - 1 end if retries > 0 then diff --git a/t/node/upstream-retries.t b/t/node/upstream-retries.t new file mode 100644 index 000000000000..81f90fc8762e --- /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] From 8d772a06931b5940114ef8cf74044c03beb345bd Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Tue, 28 Jul 2020 23:46:07 +0800 Subject: [PATCH 3/3] doc --- doc/admin-api.md | 4 ++-- doc/zh-cn/admin-api.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/admin-api.md b/doc/admin-api.md index b1112e9ab238..caa3b1d4bc25 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_on` is set as `header`, `key` is the required parameter, and `header name` is customized. When `hash_on` is set to `cookie`, `key` is the required parameter, and `cookie name` is customized. When `hash_on` is set to `consumer`, `key` does not need to be set. In this case, the `key` adopted by the hash algorithm is the `consumer_id` authenticated. If the specified `hash_on` and `key` can not fetch values, it will be fetch `remote_addr` by default.| |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 16256611d791..198de1c9f501 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,