From 905637c4f458b679ebeee9c2179ef2acd4fc15fd Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Fri, 2 Feb 2024 14:53:33 +0800 Subject: [PATCH] Revert "fix(balancer): respect max retries (#12346)" This reverts commit 2c7d1d65c6af1b453ebdd3430fdff4679e8c01f9. --- ...ua-0.10.21_02-dyn_upstream_keepalive.patch | 673 ++++++++++-------- .../kong/balancer_respect_max_retries.yml | 3 - .../05-proxy/10-balancer/08-retries_spec.lua | 124 ---- 3 files changed, 390 insertions(+), 410 deletions(-) delete mode 100644 changelog/unreleased/kong/balancer_respect_max_retries.yml delete mode 100644 spec/02-integration/05-proxy/10-balancer/08-retries_spec.lua diff --git a/build/openresty/patches/ngx_lua-0.10.21_02-dyn_upstream_keepalive.patch b/build/openresty/patches/ngx_lua-0.10.21_02-dyn_upstream_keepalive.patch index bad0683c3ae..84304498b42 100644 --- a/build/openresty/patches/ngx_lua-0.10.21_02-dyn_upstream_keepalive.patch +++ b/build/openresty/patches/ngx_lua-0.10.21_02-dyn_upstream_keepalive.patch @@ -5,43 +5,16 @@ Subject: [PATCH 1/3] feature: implemented keepalive pooling in 'balancer_by_lua*'. --- - .../nginx-1.21.4/src/http/ngx_http_upstream.c | 1 + - .../nginx-1.21.4/src/http/ngx_http_upstream.h | 2 + - .../src/ngx_http_lua_balancer.c | 848 ++++++++++++++---- - .../ngx_lua-0.10.21/src/ngx_http_lua_common.h | 11 +- - .../ngx_lua-0.10.21/src/ngx_http_lua_module.c | 3 + - 5 files changed, 689 insertions(+), 176 deletions(-) - -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..a1c4a0d 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 + src/ngx_http_lua_balancer.c | 738 ++++++++++++++++++++++++++++++------ + src/ngx_http_lua_common.h | 4 + + src/ngx_http_lua_module.c | 3 + + 3 files changed, 629 insertions(+), 116 deletions(-) - -+#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.21/src/ngx_http_lua_balancer.c b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c -index e4ac57a..ddbbf8d 100644 +index f71a3e00..0d403716 100644 --- a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c +++ b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c -@@ -16,46 +16,105 @@ +@@ -16,46 +16,102 @@ #include "ngx_http_lua_directive.h" @@ -75,20 +48,25 @@ index e4ac57a..ddbbf8d 100644 + + ngx_uint_t more_tries; + ngx_uint_t total_tries; -+ + +- ngx_http_lua_srv_conf_t *conf; +- ngx_http_request_t *request; + int last_peer_state; -+ + +- ngx_uint_t more_tries; +- ngx_uint_t total_tries; + uint32_t cpool_crc32; -+ + +- struct sockaddr *sockaddr; +- socklen_t socklen; + void *data; -- ngx_http_lua_srv_conf_t *conf; -- ngx_http_request_t *request; +- ngx_str_t *host; +- in_port_t port; + ngx_event_get_peer_pt original_get_peer; + ngx_event_free_peer_pt original_free_peer; -- ngx_uint_t more_tries; -- ngx_uint_t total_tries; +- int last_peer_state; +#if (NGX_HTTP_SSL) + ngx_event_set_peer_session_pt original_set_session; + ngx_event_save_peer_session_pt original_save_session; @@ -97,17 +75,12 @@ index e4ac57a..ddbbf8d 100644 + ngx_http_request_t *request; + ngx_http_lua_srv_conf_t *conf; + ngx_http_lua_balancer_keepalive_pool_t *cpool; - -- struct sockaddr *sockaddr; -- socklen_t socklen; ++ + ngx_str_t *host; - -- ngx_str_t *host; -- in_port_t port; ++ + struct sockaddr *sockaddr; + socklen_t socklen; - -- int last_peer_state; ++ + unsigned keepalive:1; #if !(HAVE_NGX_UPSTREAM_TIMEOUT_FIELDS) @@ -135,8 +108,6 @@ index e4ac57a..ddbbf8d 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, uint32_t cpool_crc32, ngx_uint_t cpool_size, + ngx_http_lua_balancer_keepalive_pool_t **cpool); @@ -162,12 +133,11 @@ index e4ac57a..ddbbf8d 100644 + (bp->sockaddr && bp->socklen) + + -+static char ngx_http_lua_balancer_keepalive_pools_table_key; -+static struct sockaddr *ngx_http_lua_balancer_default_server_sockaddr; ++static char ngx_http_lua_balancer_keepalive_pools_table_key; ngx_int_t -@@ -102,6 +161,61 @@ ngx_http_lua_balancer_handler_inline(ngx_http_request_t *r, +@@ -102,6 +158,61 @@ ngx_http_lua_balancer_handler_inline(ngx_http_request_t *r, } @@ -229,7 +199,7 @@ index e4ac57a..ddbbf8d 100644 char * ngx_http_lua_balancer_by_lua_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) -@@ -125,16 +239,18 @@ char * +@@ -125,16 +236,16 @@ char * ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { @@ -241,9 +211,7 @@ index e4ac57a..ddbbf8d 100644 + u_char *cache_key = NULL; + u_char *name; + ngx_str_t *value; -+ ngx_url_t url; ngx_http_upstream_srv_conf_t *uscf; -+ ngx_http_upstream_server_t *us; + ngx_http_lua_srv_conf_t *lscf = conf; dd("enter"); @@ -254,7 +222,7 @@ index e4ac57a..ddbbf8d 100644 if (cmd->post == NULL) { return NGX_CONF_ERROR; } -@@ -178,11 +294,42 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, +@@ -178,11 +289,19 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, lscf->balancer.src_key = cache_key; @@ -262,29 +230,6 @@ index e4ac57a..ddbbf8d 100644 + uscf = ngx_http_conf_get_module_srv_conf(cf, ngx_http_upstream_module); -+ if (uscf->servers->nelts == 0) { -+ us = ngx_array_push(uscf->servers); -+ if (us == NULL) { -+ return NGX_CONF_ERROR; -+ } -+ -+ ngx_memzero(us, sizeof(ngx_http_upstream_server_t)); -+ ngx_memzero(&url, sizeof(ngx_url_t)); -+ -+ ngx_str_set(&url.url, "0.0.0.1"); -+ url.default_port = 80; -+ -+ if (ngx_parse_url(cf->pool, &url) != NGX_OK) { -+ return NGX_CONF_ERROR; -+ } -+ -+ us->name = url.url; -+ us->addrs = url.addrs; -+ us->naddrs = url.naddrs; -+ -+ ngx_http_lua_balancer_default_server_sockaddr = us->addrs[0].sockaddr; -+ } -+ if (uscf->peer.init_upstream) { ngx_conf_log_error(NGX_LOG_WARN, cf, 0, "load balancing method redefined"); @@ -297,7 +242,7 @@ index e4ac57a..ddbbf8d 100644 } uscf->peer.init_upstream = ngx_http_lua_balancer_init; -@@ -198,14 +345,18 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, +@@ -198,14 +317,18 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, static ngx_int_t @@ -320,7 +265,7 @@ index e4ac57a..ddbbf8d 100644 us->peer.init = ngx_http_lua_balancer_init_peer; return NGX_OK; -@@ -216,33 +367,39 @@ static ngx_int_t +@@ -216,33 +339,38 @@ static ngx_int_t ngx_http_lua_balancer_init_peer(ngx_http_request_t *r, ngx_http_upstream_srv_conf_t *us) { @@ -353,7 +298,6 @@ index e4ac57a..ddbbf8d 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; @@ -371,7 +315,7 @@ index e4ac57a..ddbbf8d 100644 return NGX_OK; } -@@ -250,25 +407,26 @@ ngx_http_lua_balancer_init_peer(ngx_http_request_t *r, +@@ -250,25 +378,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) { @@ -389,9 +333,9 @@ index e4ac57a..ddbbf8d 100644 + ngx_http_request_t *r; + ngx_http_lua_ctx_t *ctx; + ngx_http_lua_srv_conf_t *lscf; ++ ngx_http_lua_main_conf_t *lmcf; + ngx_http_lua_balancer_keepalive_item_t *item; + ngx_http_lua_balancer_peer_data_t *bp = data; -+ void *pdata; ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0, - "lua balancer peer, tries: %ui", pc->tries); @@ -409,7 +353,7 @@ index e4ac57a..ddbbf8d 100644 if (ctx == NULL) { ctx = ngx_http_lua_create_ctx(r); if (ctx == NULL) { -@@ -286,21 +444,24 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) +@@ -286,9 +415,15 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) ctx->context = NGX_HTTP_LUA_CONTEXT_BALANCER; @@ -424,24 +368,16 @@ index e4ac57a..ddbbf8d 100644 + bp->keepalive = 0; bp->total_tries++; -- lmcf = ngx_http_get_module_main_conf(r, ngx_http_lua_module); -- -- /* balancer_by_lua does not support yielding and -- * there cannot be any conflicts among concurrent requests, -- * thus it is safe to store the peer data in the main conf. -- */ -- lmcf->balancer_peer_data = bp; -+ pdata = r->upstream->peer.data; -+ r->upstream->peer.data = bp; + lmcf = ngx_http_get_module_main_conf(r, ngx_http_lua_module); +@@ -300,7 +435,6 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) + lmcf->balancer_peer_data = bp; rc = lscf->balancer.handler(r, lscf, L); - -+ r->upstream->peer.data = pdata; -+ +- if (rc == NGX_ERROR) { return NGX_ERROR; } -@@ -322,79 +483,87 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) +@@ -322,105 +456,414 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) } } @@ -461,16 +397,10 @@ index e4ac57a..ddbbf8d 100644 } - dd("tries: %d", (int) r->upstream->peer.tries); -- -- return NGX_OK; -- } -- -- return ngx_http_upstream_get_round_robin_peer(pc, &bp->rrp); --} + if (ngx_http_lua_balancer_keepalive_is_enabled(bp)) { + ngx_http_lua_balancer_get_keepalive_pool(L, bp->cpool_crc32, + &bp->cpool); - ++ + if (bp->cpool == NULL + && ngx_http_lua_balancer_create_keepalive_pool(L, pc->log, + bp->cpool_crc32, @@ -480,105 +410,65 @@ index e4ac57a..ddbbf8d 100644 + { + return NGX_ERROR; + } - --static ngx_int_t --ngx_http_lua_balancer_by_chunk(lua_State *L, ngx_http_request_t *r) --{ -- u_char *err_msg; -- size_t len; -- ngx_int_t rc; ++ + ngx_http_lua_assert(bp->cpool); - -- /* init nginx context in Lua VM */ -- ngx_http_lua_set_req(L, r); ++ + if (!ngx_queue_empty(&bp->cpool->cache)) { + q = ngx_queue_head(&bp->cpool->cache); - --#ifndef OPENRESTY_LUAJIT -- ngx_http_lua_create_new_globals_table(L, 0 /* narr */, 1 /* nrec */); ++ + item = ngx_queue_data(q, ngx_http_lua_balancer_keepalive_item_t, + queue); + c = item->connection; - -- /* {{{ make new env inheriting main thread's globals table */ -- lua_createtable(L, 0, 1 /* nrec */); /* the metatable for the new env */ -- ngx_http_lua_get_globals_table(L); -- lua_setfield(L, -2, "__index"); -- lua_setmetatable(L, -2); /* setmetatable({}, {__index = _G}) */ -- /* }}} */ ++ + ngx_queue_remove(q); + ngx_queue_insert_head(&bp->cpool->free, q); - -- lua_setfenv(L, -2); /* set new running env for the code closure */ --#endif /* OPENRESTY_LUAJIT */ ++ + c->idle = 0; + c->sent = 0; + c->log = pc->log; + c->read->log = pc->log; + c->write->log = pc->log; + c->pool->log = pc->log; - -- lua_pushcfunction(L, ngx_http_lua_traceback); -- lua_insert(L, 1); /* put it under chunk and args */ ++ + if (c->read->timer_set) { + ngx_del_timer(c->read); + } - -- /* protected call user code */ -- rc = lua_pcall(L, 0, 1, 1); ++ + pc->cached = 1; + pc->connection = c; - -- lua_remove(L, 1); /* remove traceback function */ ++ + ngx_log_debug3(NGX_LOG_DEBUG_HTTP, pc->log, 0, + "lua balancer: keepalive reusing connection %p, " + "requests: %ui, cpool: %p", + c, c->requests, bp->cpool); - -- dd("rc == %d", (int) rc); ++ + return NGX_DONE; + } - -- if (rc != 0) { -- /* error occurred when running loaded code */ -- err_msg = (u_char *) lua_tolstring(L, -1, &len); ++ + bp->cpool->connections++; - -- if (err_msg == NULL) { -- err_msg = (u_char *) "unknown reason"; -- len = sizeof("unknown reason") - 1; ++ + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0, + "lua balancer: keepalive no free connection, " + "cpool: %p", bp->cpool); - } - -- ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, -- "failed to run balancer_by_lua*: %*s", len, err_msg); -+ return NGX_OK; -+ } - -- lua_settop(L, 0); /* clear remaining elems on stack */ -+ rc = bp->original_get_peer(pc, bp->data); -+ if (rc == NGX_ERROR) { -+ return rc; -+ } -+ -+ if (pc->sockaddr == ngx_http_lua_balancer_default_server_sockaddr) { -+ ngx_log_error(NGX_LOG_ERR, pc->log, 0, -+ "lua balancer: no peer set"); ++ } - return NGX_ERROR; + return NGX_OK; } -- lua_settop(L, 0); /* clear remaining elems on stack */ - return rc; +- return ngx_http_upstream_get_round_robin_peer(pc, &bp->rrp); ++ return bp->original_get_peer(pc, bp->data); } -@@ -403,24 +572,347 @@ static void - ngx_http_lua_balancer_free_peer(ngx_peer_connection_t *pc, void *data, - ngx_uint_t state) + +-static ngx_int_t +-ngx_http_lua_balancer_by_chunk(lua_State *L, ngx_http_request_t *r) ++static void ++ngx_http_lua_balancer_free_peer(ngx_peer_connection_t *pc, void *data, ++ ngx_uint_t state) { -- ngx_http_lua_balancer_peer_data_t *bp = data; +- u_char *err_msg; +- size_t len; +- ngx_int_t rc; + ngx_queue_t *q; + ngx_connection_t *c; + ngx_http_upstream_t *u; @@ -586,24 +476,38 @@ index e4ac57a..ddbbf8d 100644 + ngx_http_lua_balancer_keepalive_pool_t *cpool; + ngx_http_lua_balancer_peer_data_t *bp = data; - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0, -- "lua balancer free peer, tries: %ui", pc->tries); +- /* init nginx context in Lua VM */ +- ngx_http_lua_set_req(L, r); ++ ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0, + "lua balancer: free peer, tries: %ui", pc->tries); -+ + +-#ifndef OPENRESTY_LUAJIT +- ngx_http_lua_create_new_globals_table(L, 0 /* narr */, 1 /* nrec */); + u = bp->request->upstream; + c = pc->connection; -- if (bp->sockaddr && bp->socklen) { +- /* {{{ make new env inheriting main thread's globals table */ +- lua_createtable(L, 0, 1 /* nrec */); /* the metatable for the new env */ +- ngx_http_lua_get_globals_table(L); +- lua_setfield(L, -2, "__index"); +- lua_setmetatable(L, -2); /* setmetatable({}, {__index = _G}) */ +- /* }}} */ + if (ngx_http_lua_balancer_peer_set(bp)) { - bp->last_peer_state = (int) state; ++ bp->last_peer_state = (int) state; - if (pc->tries) { - pc->tries--; - } +- lua_setfenv(L, -2); /* set new running env for the code closure */ +-#endif /* OPENRESTY_LUAJIT */ ++ if (pc->tries) { ++ pc->tries--; ++ } +- lua_pushcfunction(L, ngx_http_lua_traceback); +- lua_insert(L, 1); /* put it under chunk and args */ + if (ngx_http_lua_balancer_keepalive_is_enabled(bp)) { + cpool = bp->cpool; -+ + +- /* protected call user code */ +- rc = lua_pcall(L, 0, 1, 1); + if (state & NGX_PEER_FAILED + || c == NULL + || c->read->eof @@ -614,21 +518,29 @@ index e4ac57a..ddbbf8d 100644 + { + goto invalid; + } -+ + +- lua_remove(L, 1); /* remove traceback function */ + if (bp->keepalive_requests + && c->requests >= bp->keepalive_requests) + { + goto invalid; + } -+ + +- dd("rc == %d", (int) rc); + if (!u->keepalive) { + goto invalid; + } -+ + +- if (rc != 0) { +- /* error occurred when running loaded code */ +- err_msg = (u_char *) lua_tolstring(L, -1, &len); + if (!u->request_body_sent) { + goto invalid; + } -+ + +- if (err_msg == NULL) { +- err_msg = (u_char *) "unknown reason"; +- len = sizeof("unknown reason") - 1; + if (ngx_terminate || ngx_exiting) { + goto invalid; + } @@ -705,24 +617,15 @@ index e4ac57a..ddbbf8d 100644 + if (cpool->connections == 0) { + ngx_http_lua_balancer_free_keepalive_pool(pc->log, cpool); + } -+ } -+ - return; - } - -- /* fallback */ -+ bp->original_free_peer(pc, bp->data, state); -+} -+ + } -- ngx_http_upstream_free_round_robin_peer(pc, data, state); -+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--; +- ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, +- "failed to run balancer_by_lua*: %*s", len, err_msg); ++ return; + } + +- lua_settop(L, 0); /* clear remaining elems on stack */ ++ bp->original_free_peer(pc, bp->data, state); +} + + @@ -745,12 +648,14 @@ index e4ac57a..ddbbf8d 100644 + + size = sizeof(ngx_http_lua_balancer_keepalive_pool_t) + + sizeof(ngx_http_lua_balancer_keepalive_item_t) * cpool_size; -+ + + upool = lua_newuserdata(L, size); /* pools upool */ + if (upool == NULL) { -+ return NGX_ERROR; -+ } -+ + return NGX_ERROR; + } + +- lua_settop(L, 0); /* clear remaining elems on stack */ +- return rc; + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, log, 0, + "lua balancer: keepalive create pool, crc32: %ui, " + "size: %ui", cpool_crc32, cpool_size); @@ -778,13 +683,16 @@ index e4ac57a..ddbbf8d 100644 + *cpool = upool; + + return NGX_OK; -+} -+ -+ -+static void + } + + + static void +-ngx_http_lua_balancer_free_peer(ngx_peer_connection_t *pc, void *data, +- ngx_uint_t state) +ngx_http_lua_balancer_get_keepalive_pool(lua_State *L, uint32_t cpool_crc32, + ngx_http_lua_balancer_keepalive_pool_t **cpool) -+{ + { +- ngx_http_lua_balancer_peer_data_t *bp = data; + ngx_http_lua_balancer_keepalive_pool_t *upool; + + /* get upstream connection pools table */ @@ -802,13 +710,19 @@ index e4ac57a..ddbbf8d 100644 + lua_pushvalue(L, -2); /* pools pools_table_key pools */ + lua_rawset(L, LUA_REGISTRYINDEX); /* pools */ + } -+ + +- ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0, +- "lua balancer free peer, tries: %ui", pc->tries); + ngx_http_lua_assert(lua_istable(L, -1)); -+ + +- if (bp->sockaddr && bp->socklen) { +- bp->last_peer_state = (int) state; + lua_rawgeti(L, -1, cpool_crc32); /* pools upool? */ + upool = lua_touserdata(L, -1); + lua_pop(L, 2); /* orig stack */ -+ + +- if (pc->tries) { +- pc->tries--; + *cpool = upool; +} + @@ -906,18 +820,20 @@ index e4ac57a..ddbbf8d 100644 + + if (ngx_handle_read_event(c->read, 0) != NGX_OK) { + goto close; -+ } -+ -+ return; -+ } -+ + } + + return; + } + +- /* fallback */ +close: + + item = c->data; + c->log = ev->log; + + ngx_http_lua_balancer_close(c); -+ + +- ngx_http_upstream_free_round_robin_peer(pc, data, state); + ngx_queue_remove(&item->queue); + ngx_queue_insert_head(&item->cpool->free, &item->queue); + @@ -927,7 +843,7 @@ index e4ac57a..ddbbf8d 100644 } -@@ -431,12 +923,12 @@ ngx_http_lua_balancer_set_session(ngx_peer_connection_t *pc, void *data) +@@ -431,12 +874,12 @@ ngx_http_lua_balancer_set_session(ngx_peer_connection_t *pc, void *data) { ngx_http_lua_balancer_peer_data_t *bp = data; @@ -942,7 +858,7 @@ index e4ac57a..ddbbf8d 100644 } -@@ -445,13 +937,12 @@ ngx_http_lua_balancer_save_session(ngx_peer_connection_t *pc, void *data) +@@ -445,13 +888,12 @@ ngx_http_lua_balancer_save_session(ngx_peer_connection_t *pc, void *data) { ngx_http_lua_balancer_peer_data_t *bp = data; @@ -958,7 +874,7 @@ index e4ac57a..ddbbf8d 100644 } #endif -@@ -459,14 +950,13 @@ ngx_http_lua_balancer_save_session(ngx_peer_connection_t *pc, void *data) +@@ -459,14 +901,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, @@ -975,39 +891,12 @@ index e4ac57a..ddbbf8d 100644 + ngx_url_t url; + ngx_http_upstream_t *u; + ngx_http_lua_ctx_t *ctx; ++ ngx_http_lua_main_conf_t *lmcf; + ngx_http_lua_balancer_peer_data_t *bp; if (r == NULL) { *err = "no request found"; -@@ -491,18 +981,6 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, - return NGX_ERROR; - } - -- lmcf = ngx_http_get_module_main_conf(r, ngx_http_lua_module); -- -- /* we cannot read r->upstream->peer.data here directly because -- * it could be overridden by other modules like -- * ngx_http_upstream_keepalive_module. -- */ -- bp = lmcf->balancer_peer_data; -- if (bp == NULL) { -- *err = "no upstream peer data found"; -- return NGX_ERROR; -- } -- - ngx_memzero(&url, sizeof(ngx_url_t)); - - url.url.data = ngx_palloc(r->pool, addr_len); -@@ -526,6 +1004,8 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, - return NGX_ERROR; - } - -+ bp = (ngx_http_lua_balancer_peer_data_t *) u->peer.data; -+ - if (url.addrs && url.addrs[0].sockaddr) { - bp->sockaddr = url.addrs[0].sockaddr; - bp->socklen = url.addrs[0].socklen; -@@ -536,6 +1016,59 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, +@@ -536,6 +978,70 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, return NGX_ERROR; } @@ -1024,6 +913,7 @@ index e4ac57a..ddbbf8d 100644 +{ + ngx_http_upstream_t *u; + ngx_http_lua_ctx_t *ctx; ++ ngx_http_lua_main_conf_t *lmcf; + ngx_http_lua_balancer_peer_data_t *bp; + + if (r == NULL) { @@ -1049,7 +939,17 @@ index e4ac57a..ddbbf8d 100644 + return NGX_ERROR; + } + -+ bp = (ngx_http_lua_balancer_peer_data_t *) u->peer.data; ++ lmcf = ngx_http_get_module_main_conf(r, ngx_http_lua_module); ++ ++ /* we cannot read r->upstream->peer.data here directly because ++ * it could be overridden by other modules like ++ * ngx_http_upstream_keepalive_module. ++ */ ++ bp = lmcf->balancer_peer_data; ++ if (bp == NULL) { ++ *err = "no upstream peer data found"; ++ return NGX_ERROR; ++ } + + if (!ngx_http_lua_balancer_peer_set(bp)) { + *err = "no current peer set"; @@ -1067,7 +967,239 @@ index e4ac57a..ddbbf8d 100644 return NGX_OK; } -@@ -545,14 +1078,13 @@ ngx_http_lua_ffi_balancer_set_timeouts(ngx_http_request_t *r, +diff --git a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_common.h b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_common.h +index 781a2454..9ce6836a 100644 +--- a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_common.h ++++ b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_common.h +@@ -328,6 +328,10 @@ union ngx_http_lua_srv_conf_u { + #endif + + struct { ++ ngx_http_upstream_init_pt original_init_upstream; ++ ngx_http_upstream_init_peer_pt original_init_peer; ++ uintptr_t data; ++ + ngx_http_lua_srv_conf_handler_pt handler; + ngx_str_t src; + u_char *src_key; +diff --git a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_module.c b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_module.c +index 9816d864..5d7cedfd 100644 +--- a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_module.c ++++ b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_module.c +@@ -1117,6 +1117,9 @@ ngx_http_lua_create_srv_conf(ngx_conf_t *cf) + * lscf->srv.ssl_session_fetch_src = { 0, NULL }; + * lscf->srv.ssl_session_fetch_src_key = NULL; + * ++ * lscf->balancer.original_init_upstream = NULL; ++ * lscf->balancer.original_init_peer = NULL; ++ * lscf->balancer.data = NULL; + * lscf->balancer.handler = NULL; + * lscf->balancer.src = { 0, NULL }; + * lscf->balancer.src_key = NULL; +-- +2.26.2 + + +From 4c5cb29a265b2f9524434322adf15d07deec6c7f Mon Sep 17 00:00:00 2001 +From: Thibault Charbonnier +Date: Tue, 17 Sep 2019 11:43:54 -0700 +Subject: [PATCH 2/3] feature: we now avoid the need for 'upstream' blocks to + define a stub 'server' directive when using 'balancer_by_lua*'. + +--- + src/ngx_http_lua_balancer.c | 42 +++++++++++++++++++++++++++++++++++-- + 1 file changed, 40 insertions(+), 2 deletions(-) + +diff --git a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c +index 0d403716..5c862d22 100644 +--- a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c ++++ b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c +@@ -111,7 +111,8 @@ static void ngx_http_lua_balancer_save_session(ngx_peer_connection_t *pc, + (bp->sockaddr && bp->socklen) + + +-static char ngx_http_lua_balancer_keepalive_pools_table_key; ++static char ngx_http_lua_balancer_keepalive_pools_table_key; ++static struct sockaddr *ngx_http_lua_balancer_default_server_sockaddr; + + + ngx_int_t +@@ -239,7 +240,9 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, + u_char *cache_key = NULL; + u_char *name; + ngx_str_t *value; ++ ngx_url_t url; + ngx_http_upstream_srv_conf_t *uscf; ++ ngx_http_upstream_server_t *us; + ngx_http_lua_srv_conf_t *lscf = conf; + + dd("enter"); +@@ -293,6 +296,29 @@ ngx_http_lua_balancer_by_lua(ngx_conf_t *cf, ngx_command_t *cmd, + + uscf = ngx_http_conf_get_module_srv_conf(cf, ngx_http_upstream_module); + ++ if (uscf->servers->nelts == 0) { ++ us = ngx_array_push(uscf->servers); ++ if (us == NULL) { ++ return NGX_CONF_ERROR; ++ } ++ ++ ngx_memzero(us, sizeof(ngx_http_upstream_server_t)); ++ ngx_memzero(&url, sizeof(ngx_url_t)); ++ ++ ngx_str_set(&url.url, "0.0.0.1"); ++ url.default_port = 80; ++ ++ if (ngx_parse_url(cf->pool, &url) != NGX_OK) { ++ return NGX_CONF_ERROR; ++ } ++ ++ us->name = url.url; ++ us->addrs = url.addrs; ++ us->naddrs = url.naddrs; ++ ++ ngx_http_lua_balancer_default_server_sockaddr = us->addrs[0].sockaddr; ++ } ++ + if (uscf->peer.init_upstream) { + ngx_conf_log_error(NGX_LOG_WARN, cf, 0, + "load balancing method redefined"); +@@ -525,7 +551,19 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) + return NGX_OK; + } + +- return bp->original_get_peer(pc, bp->data); ++ rc = bp->original_get_peer(pc, bp->data); ++ if (rc == NGX_ERROR) { ++ return rc; ++ } ++ ++ if (pc->sockaddr == ngx_http_lua_balancer_default_server_sockaddr) { ++ ngx_log_error(NGX_LOG_ERR, pc->log, 0, ++ "lua balancer: no peer set"); ++ ++ return NGX_ERROR; ++ } ++ ++ return rc; + } + + +-- +2.26.2 + + +From 941cd893573561574bc6a326d6306f1a30127293 Mon Sep 17 00:00:00 2001 +From: Thibault Charbonnier +Date: Tue, 17 Sep 2019 11:43:58 -0700 +Subject: [PATCH 3/3] refactor: used a simpler way to stash the balancer peer + data. + +--- + src/ngx_http_lua_balancer.c | 91 +++++++++---------------------------- + src/ngx_http_lua_common.h | 7 --- + 2 files changed, 22 insertions(+), 76 deletions(-) + +diff --git a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c +index 5c862d22..3ea1f067 100644 +--- a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c ++++ b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_balancer.c +@@ -411,9 +411,9 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) + ngx_http_request_t *r; + ngx_http_lua_ctx_t *ctx; + ngx_http_lua_srv_conf_t *lscf; +- ngx_http_lua_main_conf_t *lmcf; + ngx_http_lua_balancer_keepalive_item_t *item; + ngx_http_lua_balancer_peer_data_t *bp = data; ++ void *pdata; + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0, + "lua balancer: get peer, tries: %ui", pc->tries); +@@ -452,15 +452,13 @@ ngx_http_lua_balancer_get_peer(ngx_peer_connection_t *pc, void *data) + bp->keepalive = 0; + bp->total_tries++; + +- lmcf = ngx_http_get_module_main_conf(r, ngx_http_lua_module); +- +- /* balancer_by_lua does not support yielding and +- * there cannot be any conflicts among concurrent requests, +- * thus it is safe to store the peer data in the main conf. +- */ +- lmcf->balancer_peer_data = bp; ++ pdata = r->upstream->peer.data; ++ r->upstream->peer.data = bp; + + rc = lscf->balancer.handler(r, lscf, L); ++ ++ r->upstream->peer.data = pdata; ++ + if (rc == NGX_ERROR) { + return NGX_ERROR; + } +@@ -945,7 +943,6 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, + ngx_url_t url; + ngx_http_upstream_t *u; + ngx_http_lua_ctx_t *ctx; +- ngx_http_lua_main_conf_t *lmcf; + ngx_http_lua_balancer_peer_data_t *bp; + + if (r == NULL) { +@@ -971,18 +968,6 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, + return NGX_ERROR; + } + +- lmcf = ngx_http_get_module_main_conf(r, ngx_http_lua_module); +- +- /* we cannot read r->upstream->peer.data here directly because +- * it could be overridden by other modules like +- * ngx_http_upstream_keepalive_module. +- */ +- bp = lmcf->balancer_peer_data; +- if (bp == NULL) { +- *err = "no upstream peer data found"; +- return NGX_ERROR; +- } +- + ngx_memzero(&url, sizeof(ngx_url_t)); + + url.url.data = ngx_palloc(r->pool, addr_len); +@@ -1006,6 +991,8 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r, + return NGX_ERROR; + } + ++ bp = (ngx_http_lua_balancer_peer_data_t *) u->peer.data; ++ + if (url.addrs && url.addrs[0].sockaddr) { + bp->sockaddr = url.addrs[0].sockaddr; + bp->socklen = url.addrs[0].socklen; +@@ -1029,7 +1016,6 @@ ngx_http_lua_ffi_balancer_enable_keepalive(ngx_http_request_t *r, + { + ngx_http_upstream_t *u; + ngx_http_lua_ctx_t *ctx; +- ngx_http_lua_main_conf_t *lmcf; + ngx_http_lua_balancer_peer_data_t *bp; + + if (r == NULL) { +@@ -1055,17 +1041,7 @@ ngx_http_lua_ffi_balancer_enable_keepalive(ngx_http_request_t *r, + return NGX_ERROR; + } + +- lmcf = ngx_http_get_module_main_conf(r, ngx_http_lua_module); +- +- /* we cannot read r->upstream->peer.data here directly because +- * it could be overridden by other modules like +- * ngx_http_upstream_keepalive_module. +- */ +- bp = lmcf->balancer_peer_data; +- if (bp == NULL) { +- *err = "no upstream peer data found"; +- return NGX_ERROR; +- } ++ bp = (ngx_http_lua_balancer_peer_data_t *) u->peer.data; + + if (!ngx_http_lua_balancer_peer_set(bp)) { + *err = "no current peer set"; +@@ -1089,14 +1065,13 @@ ngx_http_lua_ffi_balancer_set_timeouts(ngx_http_request_t *r, long connect_timeout, long send_timeout, long read_timeout, char **err) { @@ -1085,7 +1217,7 @@ index e4ac57a..ddbbf8d 100644 if (r == NULL) { *err = "no request found"; -@@ -577,15 +1109,9 @@ ngx_http_lua_ffi_balancer_set_timeouts(ngx_http_request_t *r, +@@ -1121,15 +1096,9 @@ ngx_http_lua_ffi_balancer_set_timeouts(ngx_http_request_t *r, return NGX_ERROR; } @@ -1103,7 +1235,7 @@ index e4ac57a..ddbbf8d 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. */ -@@ -640,12 +1166,10 @@ ngx_http_lua_ffi_balancer_set_more_tries(ngx_http_request_t *r, +@@ -1184,12 +1153,10 @@ ngx_http_lua_ffi_balancer_set_more_tries(ngx_http_request_t *r, int count, char **err) { #if (nginx_version >= 1007005) @@ -1119,7 +1251,7 @@ index e4ac57a..ddbbf8d 100644 ngx_http_lua_balancer_peer_data_t *bp; if (r == NULL) { -@@ -671,13 +1195,7 @@ ngx_http_lua_ffi_balancer_set_more_tries(ngx_http_request_t *r, +@@ -1215,13 +1182,7 @@ ngx_http_lua_ffi_balancer_set_more_tries(ngx_http_request_t *r, return NGX_ERROR; } @@ -1134,7 +1266,7 @@ index e4ac57a..ddbbf8d 100644 #if (nginx_version >= 1007005) max_tries = r->upstream->conf->next_upstream_tries; -@@ -703,12 +1221,10 @@ int +@@ -1247,12 +1208,10 @@ int ngx_http_lua_ffi_balancer_get_last_failure(ngx_http_request_t *r, int *status, char **err) { @@ -1150,7 +1282,7 @@ index e4ac57a..ddbbf8d 100644 if (r == NULL) { *err = "no request found"; -@@ -733,13 +1249,7 @@ ngx_http_lua_ffi_balancer_get_last_failure(ngx_http_request_t *r, +@@ -1277,13 +1236,7 @@ ngx_http_lua_ffi_balancer_get_last_failure(ngx_http_request_t *r, return NGX_ERROR; } @@ -1166,7 +1298,7 @@ index e4ac57a..ddbbf8d 100644 if (r->upstream_states && r->upstream_states->nelts > 1) { state = r->upstream_states->elts; diff --git a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_common.h b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_common.h -index ed88f0a..97d1942 100644 +index 9ce6836a..9a4342df 100644 --- a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_common.h +++ b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_common.h @@ -240,13 +240,6 @@ struct ngx_http_lua_main_conf_s { @@ -1183,30 +1315,5 @@ index ed88f0a..97d1942 100644 ngx_chain_t *body_filter_chain; /* neither yielding nor recursion is possible in * body_filter_by_lua*, so there cannot be any races among -@@ -328,6 +321,10 @@ union ngx_http_lua_srv_conf_u { - #endif - - struct { -+ ngx_http_upstream_init_pt original_init_upstream; -+ ngx_http_upstream_init_peer_pt original_init_peer; -+ uintptr_t data; -+ - ngx_http_lua_srv_conf_handler_pt handler; - ngx_str_t src; - u_char *src_key; -diff --git a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_module.c b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_module.c -index fbeba12..49944c3 100644 ---- a/bundle/ngx_lua-0.10.21/src/ngx_http_lua_module.c -+++ b/bundle/ngx_lua-0.10.21/src/ngx_http_lua_module.c -@@ -1117,6 +1117,9 @@ ngx_http_lua_create_srv_conf(ngx_conf_t *cf) - * lscf->srv.ssl_session_fetch_src = { 0, NULL }; - * lscf->srv.ssl_session_fetch_src_key = NULL; - * -+ * lscf->balancer.original_init_upstream = NULL; -+ * lscf->balancer.original_init_peer = NULL; -+ * lscf->balancer.data = NULL; - * lscf->balancer.handler = NULL; - * lscf->balancer.src = { 0, NULL }; - * lscf->balancer.src_key = NULL; -- -2.34.1 +2.26.2 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 cfc25919986..00000000000 --- a/spec/02-integration/05-proxy/10-balancer/08-retries_spec.lua +++ /dev/null @@ -1,124 +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_size = 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) - end) - end) -end