diff --git a/build/openresty/patches/ngx_lua-0.10.25_01-dyn_upstream_keepalive.patch b/build/openresty/patches/ngx_lua-0.10.25_01-dyn_upstream_keepalive.patch index aa339c32a9c..f0b20bdd12d 100644 --- a/build/openresty/patches/ngx_lua-0.10.25_01-dyn_upstream_keepalive.patch +++ b/build/openresty/patches/ngx_lua-0.10.25_01-dyn_upstream_keepalive.patch @@ -1,33 +1,8 @@ -diff --git a/bundle/nginx-1.21.4/src/http/ngx_http_upstream.c b/bundle/nginx-1.21.4/src/http/ngx_http_upstream.c -index b07e564..9e25905 100644 ---- a/bundle/nginx-1.21.4/src/http/ngx_http_upstream.c -+++ b/bundle/nginx-1.21.4/src/http/ngx_http_upstream.c -@@ -4304,6 +4304,7 @@ ngx_http_upstream_next(ngx_http_request_t *r, ngx_http_upstream_t *u, - if (u->peer.cached && ft_type == NGX_HTTP_UPSTREAM_FT_ERROR) { - /* TODO: inform balancer instead */ - u->peer.tries++; -+ u->peer.notify(&u->peer, u->peer.data, NGX_HTTP_UPSTREAM_NOFITY_CACHED_CONNECTION_ERROR); - } - - switch (ft_type) { -diff --git a/bundle/nginx-1.21.4/src/http/ngx_http_upstream.h b/bundle/nginx-1.21.4/src/http/ngx_http_upstream.h -index a385222..1cd214c 100644 ---- a/bundle/nginx-1.21.4/src/http/ngx_http_upstream.h -+++ b/bundle/nginx-1.21.4/src/http/ngx_http_upstream.h -@@ -56,6 +56,8 @@ - #define NGX_HTTP_UPSTREAM_IGN_VARY 0x00000200 - - -+#define NGX_HTTP_UPSTREAM_NOFITY_CACHED_CONNECTION_ERROR 0x1 -+ - typedef struct { - ngx_uint_t status; - ngx_msec_t response_time; diff --git a/bundle/ngx_lua-0.10.25/src/ngx_http_lua_balancer.c b/bundle/ngx_lua-0.10.25/src/ngx_http_lua_balancer.c -index af4da73..99d073a 100644 +index af4da73..407c115 100644 --- a/bundle/ngx_lua-0.10.25/src/ngx_http_lua_balancer.c +++ b/bundle/ngx_lua-0.10.25/src/ngx_http_lua_balancer.c -@@ -16,46 +16,106 @@ +@@ -16,46 +16,104 @@ #include "ngx_http_lua_directive.h" @@ -121,8 +96,6 @@ index af4da73..99d073a 100644 - ngx_http_request_t *r); static void ngx_http_lua_balancer_free_peer(ngx_peer_connection_t *pc, void *data, ngx_uint_t state); -+static void ngx_http_lua_balancer_notify_peer(ngx_peer_connection_t *pc, -+ void *data, ngx_uint_t type); +static ngx_int_t ngx_http_lua_balancer_create_keepalive_pool(lua_State *L, + ngx_log_t *log, ngx_str_t *cpool_name, ngx_uint_t cpool_size, + ngx_http_lua_balancer_keepalive_pool_t **cpool); @@ -154,7 +127,7 @@ index af4da73..99d073a 100644 ngx_int_t -@@ -102,6 +162,61 @@ ngx_http_lua_balancer_handler_inline(ngx_http_request_t *r, +@@ -102,6 +160,61 @@ ngx_http_lua_balancer_handler_inline(ngx_http_request_t *r, } @@ -216,7 +189,7 @@ index af4da73..99d073a 100644 char * ngx_http_lua_balancer_by_lua_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) -@@ -125,18 +240,20 @@ char * +@@ -125,18 +238,20 @@ char * ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { @@ -245,7 +218,7 @@ index af4da73..99d073a 100644 if (cmd->post == NULL) { return NGX_CONF_ERROR; } -@@ -188,11 +305,42 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, +@@ -188,11 +303,42 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, lscf->balancer.src_key = cache_key; @@ -288,7 +261,7 @@ index af4da73..99d073a 100644 } uscf->peer.init_upstream = ngx_http_lua_balancer_init; -@@ -208,14 +356,18 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, +@@ -208,14 +354,18 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, static ngx_int_t @@ -311,7 +284,7 @@ index af4da73..99d073a 100644 us->peer.init = ngx_http_lua_balancer_init_peer; return NGX_OK; -@@ -226,33 +378,39 @@ static ngx_int_t +@@ -226,33 +376,38 @@ static ngx_int_t ngx_http_lua_balancer_init_peer(ngx_http_request_t *r, ngx_http_upstream_srv_conf_t *us) { @@ -344,7 +317,6 @@ index af4da73..99d073a 100644 + r->upstream->peer.data = bp; r->upstream->peer.get = ngx_http_lua_balancer_get_peer; r->upstream->peer.free = ngx_http_lua_balancer_free_peer; -+ r->upstream->peer.notify = ngx_http_lua_balancer_notify_peer; #if (NGX_HTTP_SSL) + bp->original_set_session = r->upstream->peer.set_session; @@ -362,7 +334,7 @@ index af4da73..99d073a 100644 return NGX_OK; } -@@ -260,25 +418,26 @@ ngx_http_lua_balancer_init_peer(ngx_http_request_t *r, +@@ -260,25 +415,26 @@ ngx_http_lua_balancer_init_peer(ngx_http_request_t *r, static ngx_int_t ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) { @@ -400,7 +372,7 @@ index af4da73..99d073a 100644 if (ctx == NULL) { ctx = ngx_http_lua_create_ctx(r); if (ctx == NULL) { -@@ -296,21 +455,23 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) +@@ -296,21 +452,23 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) ctx->context = NGX_HTTP_LUA_CONTEXT_BALANCER; @@ -431,7 +403,7 @@ index af4da73..99d073a 100644 if (rc == NGX_ERROR) { return NGX_ERROR; } -@@ -332,79 +493,88 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) +@@ -332,79 +490,88 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) } } @@ -565,7 +537,7 @@ index af4da73..99d073a 100644 return rc; } -@@ -413,24 +583,364 @@ static void +@@ -413,24 +580,354 @@ static void ngx_http_lua_balancer_free_peer(ngx_peer_connection_t *pc, void *data, ngx_uint_t state) { @@ -705,16 +677,6 @@ index af4da73..99d073a 100644 +} + + -+static void -+ngx_http_lua_balancer_notify_peer(ngx_peer_connection_t *pc, void *data, -+ ngx_uint_t type) -+{ -+ if (type == NGX_HTTP_UPSTREAM_NOFITY_CACHED_CONNECTION_ERROR) { -+ pc->tries--; -+ } -+} -+ -+ +static ngx_int_t +ngx_http_lua_balancer_create_keepalive_pool(lua_State *L, ngx_log_t *log, + ngx_str_t *cpool_name, ngx_uint_t cpool_size, @@ -833,17 +795,15 @@ index af4da73..99d073a 100644 + + if (lua_isnil(L, -1)) { + lua_pop(L, 1); /* orig stack */ - return; - } - -- /* fallback */ ++ return; ++ } ++ + ngx_http_lua_assert(lua_istable(L, -1)); + + lua_pushlstring(L, (const char *)cpool->cpool_name.data, cpool->cpool_name.len); + lua_pushnil(L); /* pools nil */ + lua_rawset(L, -3); /* pools */ - -- ngx_http_upstream_free_round_robin_peer(pc, data, state); ++ + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, log, 0, + "lua balancer: keepalive free pool, " + "name: %V, cpool: %p", @@ -916,14 +876,16 @@ index af4da73..99d073a 100644 + goto close; + } + -+ return; -+ } -+ + return; + } + +- /* fallback */ +close: + + item = c->data; + c->log = ev->log; -+ + +- ngx_http_upstream_free_round_robin_peer(pc, data, state); + ngx_http_lua_balancer_close(c); + + ngx_queue_remove(&item->queue); @@ -935,7 +897,7 @@ index af4da73..99d073a 100644 } -@@ -441,12 +951,12 @@ ngx_http_lua_balancer_set_session(ngx_peer_connection_t *pc, void *data) +@@ -441,12 +938,12 @@ ngx_http_lua_balancer_set_session(ngx_peer_connection_t *pc, void *data) { ngx_http_lua_balancer_peer_data_t *bp = data; @@ -950,7 +912,7 @@ index af4da73..99d073a 100644 } -@@ -455,13 +965,12 @@ ngx_http_lua_balancer_save_session(ngx_peer_connection_t *pc, void *data) +@@ -455,13 +952,12 @@ ngx_http_lua_balancer_save_session(ngx_peer_connection_t *pc, void *data) { ngx_http_lua_balancer_peer_data_t *bp = data; @@ -966,7 +928,7 @@ index af4da73..99d073a 100644 } #endif -@@ -469,14 +978,14 @@ ngx_http_lua_balancer_save_session(ngx_peer_connection_t *pc, void *data) +@@ -469,14 +965,14 @@ ngx_http_lua_balancer_save_session(ngx_peer_connection_t *pc, void *data) int ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, @@ -988,7 +950,7 @@ index af4da73..99d073a 100644 if (r == NULL) { *err = "no request found"; -@@ -501,18 +1010,6 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, +@@ -501,18 +997,6 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, return NGX_ERROR; } @@ -1007,7 +969,7 @@ index af4da73..99d073a 100644 ngx_memzero(&url, sizeof(ngx_url_t)); url.url.data = ngx_palloc(r->pool, addr_len); -@@ -536,6 +1033,8 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, +@@ -536,6 +1020,8 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, return NGX_ERROR; } @@ -1016,7 +978,7 @@ index af4da73..99d073a 100644 if (url.addrs && url.addrs[0].sockaddr) { bp->sockaddr = url.addrs[0].sockaddr; bp->socklen = url.addrs[0].socklen; -@@ -546,6 +1045,72 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, +@@ -546,6 +1032,72 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, return NGX_ERROR; } @@ -1089,7 +1051,7 @@ index af4da73..99d073a 100644 return NGX_OK; } -@@ -555,14 +1120,13 @@ ngx_http_lua_ffi_balancer_set_timeouts(ngx_http_request_t *r, +@@ -555,14 +1107,13 @@ ngx_http_lua_ffi_balancer_set_timeouts(ngx_http_request_t *r, long connect_timeout, long send_timeout, long read_timeout, char **err) { @@ -1107,7 +1069,7 @@ index af4da73..99d073a 100644 if (r == NULL) { *err = "no request found"; -@@ -587,15 +1151,9 @@ ngx_http_lua_ffi_balancer_set_timeouts(ngx_http_request_t *r, +@@ -587,15 +1138,9 @@ ngx_http_lua_ffi_balancer_set_timeouts(ngx_http_request_t *r, return NGX_ERROR; } @@ -1125,7 +1087,7 @@ index af4da73..99d073a 100644 if (!bp->cloned_upstream_conf) { /* we clone the upstream conf for the current request so that * we do not affect other requests at all. */ -@@ -650,12 +1208,10 @@ ngx_http_lua_ffi_balancer_set_more_tries(ngx_http_request_t *r, +@@ -650,12 +1195,10 @@ ngx_http_lua_ffi_balancer_set_more_tries(ngx_http_request_t *r, int count, char **err) { #if (nginx_version >= 1007005) @@ -1141,7 +1103,7 @@ index af4da73..99d073a 100644 ngx_http_lua_balancer_peer_data_t *bp; if (r == NULL) { -@@ -681,13 +1237,7 @@ ngx_http_lua_ffi_balancer_set_more_tries(ngx_http_request_t *r, +@@ -681,13 +1224,7 @@ ngx_http_lua_ffi_balancer_set_more_tries(ngx_http_request_t *r, return NGX_ERROR; } @@ -1156,7 +1118,7 @@ index af4da73..99d073a 100644 #if (nginx_version >= 1007005) max_tries = r->upstream->conf->next_upstream_tries; -@@ -713,12 +1263,10 @@ int +@@ -713,12 +1250,10 @@ int ngx_http_lua_ffi_balancer_get_last_failure(ngx_http_request_t *r, int *status, char **err) { @@ -1172,7 +1134,7 @@ index af4da73..99d073a 100644 if (r == NULL) { *err = "no request found"; -@@ -743,13 +1291,7 @@ ngx_http_lua_ffi_balancer_get_last_failure(ngx_http_request_t *r, +@@ -743,13 +1278,7 @@ ngx_http_lua_ffi_balancer_get_last_failure(ngx_http_request_t *r, return NGX_ERROR; } diff --git a/changelog/unreleased/kong/balancer_respect_max_retries.yml b/changelog/unreleased/kong/balancer_respect_max_retries.yml deleted file mode 100644 index 1884ad1ce9f..00000000000 --- a/changelog/unreleased/kong/balancer_respect_max_retries.yml +++ /dev/null @@ -1,3 +0,0 @@ -message: Fix an issue that the actual number of retry times exceeds the `retries` setting. -type: bugfix -scope: Core diff --git a/spec/02-integration/05-proxy/10-balancer/08-retries_spec.lua b/spec/02-integration/05-proxy/10-balancer/08-retries_spec.lua deleted file mode 100644 index b3245055dfe..00000000000 --- a/spec/02-integration/05-proxy/10-balancer/08-retries_spec.lua +++ /dev/null @@ -1,128 +0,0 @@ -local helpers = require "spec.helpers" -local cjson = require "cjson" - -local function get_log(typ, n) - local entries - helpers.wait_until(function() - local client = assert(helpers.http_client(helpers.mock_upstream_host, - helpers.mock_upstream_port)) - local res = client:get("/read_log/" .. typ, { - headers = { - Accept = "application/json" - } - }) - local raw = assert.res_status(200, res) - local body = cjson.decode(raw) - - entries = body.entries - return #entries > 0 - end, 10) - if n then - assert(#entries == n, "expected " .. n .. " log entries, but got " .. #entries) - end - return entries -end - -for _, strategy in helpers.each_strategy() do - describe("Balancer: respect max retries [#" .. strategy .. "]", function() - local service - - lazy_setup(function() - local bp = helpers.get_db_utils(strategy, { - "routes", - "services", - "plugins", - }) - - service = bp.services:insert { - name = "retry_service", - host = "127.0.0.1", - port = 62351, - retries = 5, - } - - local route = bp.routes:insert { - service = service, - paths = { "/hello" }, - strip_path = false, - } - - bp.plugins:insert { - route = { id = route.id }, - name = "http-log", - config = { - queue = { - max_batch_size = 1, - max_coalescing_delay = 0.1, - }, - http_endpoint = "http://" .. helpers.mock_upstream_host - .. ":" - .. helpers.mock_upstream_port - .. "/post_log/http" - } - } - - local fixtures = { - http_mock = {} - } - - fixtures.http_mock.my_server_block = [[ - server { - listen 0.0.0.0:62351; - location /hello { - content_by_lua_block { - local request_counter = ngx.shared.request_counter - local first_request = request_counter:get("first_request") - if first_request == nil then - request_counter:set("first_request", "yes") - ngx.say("hello") - else - ngx.exit(ngx.HTTP_CLOSE) - end - } - } - } - ]] - - assert(helpers.start_kong({ - database = strategy, - nginx_conf = "spec/fixtures/custom_nginx.template", - nginx_http_lua_shared_dict = "request_counter 1m", - }, nil, nil, fixtures)) - end) - - lazy_teardown(function() - helpers.stop_kong() - end) - - it("exceeded limit", function() - -- First request should succeed and save connection to upstream in keepalive pool - local proxy_client1 = helpers.proxy_client() - local res = assert(proxy_client1:send { - method = "GET", - path = "/hello", - }) - - assert.res_status(200, res) - - proxy_client1:close() - - -- Second request should failed 1 times and retry 5 times and then return 502 - local proxy_client2 = helpers.proxy_client() - - res = assert(proxy_client2:send { - method = "GET", - path = "/hello", - }) - - assert.res_status(502, res) - - -- wait for the http-log plugin to flush the log - ngx.sleep(1) - - local entries = get_log("http", 2) - assert.equal(#entries[2].tries, 6) - assert.equal(entries[2].upstream_status, "502, 502, 502, 502, 502, 502") - end) - end) -end