From 64f64f1224cb06fb86d6828e679f26df0b4ffe2a Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Mon, 31 Aug 2020 21:45:37 -0400 Subject: [PATCH 01/43] dont exit process, supposedly it will generate new processes the replace the old old processes --- ngx_http_upstream_serverlist.c | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 2fe5c45..23dc031 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -74,9 +74,6 @@ init_module(ngx_cycle_t *cycle); static ngx_int_t init_process(ngx_cycle_t *cycle); -static void -exit_process(ngx_cycle_t *cycle); - static void refresh_timeout_handler(ngx_event_t *ev); @@ -134,7 +131,7 @@ ngx_module_t ngx_http_upstream_serverlist_module = { init_process, /* init process */ NULL, /* init thread */ NULL, /* exit thread */ - exit_process, /* exit process */ + NULL, /* exit process */ NULL, /* exit master */ NGX_MODULE_V1_PADDING }; @@ -456,29 +453,6 @@ init_process(ngx_cycle_t *cycle) { return NGX_OK; } -static void -exit_process(ngx_cycle_t *cycle) { - main_conf *mcf = ngx_http_cycle_get_module_main_conf(cycle, - ngx_http_upstream_serverlist_module); - serverlist *sls = mcf->serverlists.elts; - service_conn *scs = mcf->service_conns.elts; - ngx_uint_t i; - - for (i = 0; i < mcf->serverlists.nelts; i++) { - if (sls[i].pool) { - ngx_destroy_pool(sls[i].pool); - sls[i].pool = NULL; - } - } - - for (i = 0; i < mcf->service_conns.nelts; i++) { - if (scs[i].peer_conn.connection) { - ngx_close_connection(scs[i].peer_conn.connection); - scs[i].peer_conn.connection = NULL; - } - } -} - static void empty_handler(ngx_event_t *ev) { ngx_log_debug(NGX_LOG_DEBUG_ALL, ev->log, 0, From 0808b1a9937c53100a53ed435d22b256cebc27c0 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Mon, 31 Aug 2020 21:59:35 -0400 Subject: [PATCH 02/43] use ngx_http_upstream_init_round_robin instead of init --- ngx_http_upstream_serverlist.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 23dc031..72d313f 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1087,9 +1087,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { return -1; } - init = uscf->peer.init_upstream ? uscf->peer.init_upstream - : ngx_http_upstream_init_round_robin; - ngx_memzero(&cf, sizeof cf); cf.name = "serverlist_init_upstream"; cf.cycle = (ngx_cycle_t *) ngx_cycle; @@ -1103,13 +1100,18 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { old_servers = uscf->servers; uscf->servers = new_servers; - if (init(&cf, uscf) != NGX_OK) { + if (ngx_http_upstream_init_round_robin(&cf, uscf) != NGX_OK) { + // see: https://github.com/GUI/nginx-upstream-dynamic-servers/pull/33/files + /* if you read the native code you can find out that all you need to do here is ngx_http_upstream_init_round_robin if you don't use other third party modules in the init process, + otherwise it may cause memory problem if you use keepalive in the upstream block (it reinitialize the keepalive queue, when remote close the connection 2 TTL later, it will crash) + */ ngx_log_error(NGX_LOG_ERR, log, 0, "upstream-serverlist: refresh upstream %V failed, rollback it", &uscf->host); // cf.pool = sl->pool; uscf->servers = old_servers; - init(&cf, uscf); + // this may not work if old servers do not exist? + ngx_http_upstream_init_round_robin(&cf, uscf); return -1; } From 25996a26efe30bd70fdc601cb5d1652ea147fb7e Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Mon, 31 Aug 2020 22:22:03 -0400 Subject: [PATCH 03/43] remove initialization of init var --- ngx_http_upstream_serverlist.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 72d313f..641a108 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1065,7 +1065,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { main_conf *mcf = ngx_http_cycle_get_module_main_conf(ngx_cycle, ngx_http_upstream_serverlist_module); ngx_http_upstream_srv_conf_t *uscf = sl->upstream_conf; - ngx_http_upstream_init_pt init = NULL; ngx_conf_t cf = {0}; ngx_array_t *new_servers = NULL; ngx_array_t *old_servers = uscf->servers; From 5ac28a533a6aa68c8d41e24f581706bfe06d671f Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 1 Sep 2020 21:29:19 -0400 Subject: [PATCH 04/43] testing order when loading directive --- ngx_http_upstream_serverlist.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 641a108..3678dc2 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -268,9 +268,6 @@ static char * serverlist_directive(ngx_conf_t *cf, ngx_command_t *cmd, void *dummy) { ngx_http_upstream_srv_conf_t *uscf = ngx_http_conf_get_module_srv_conf(cf, ngx_http_upstream_module); - main_conf *mcf = ngx_http_conf_get_module_main_conf(cf, - ngx_http_upstream_serverlist_module); - serverlist *sl = NULL; if (cf->args->nelts > 2) { ngx_conf_log_error(NGX_LOG_ERR, cf, 0, @@ -278,6 +275,10 @@ serverlist_directive(ngx_conf_t *cf, ngx_command_t *cmd, void *dummy) { return NGX_CONF_ERROR; } + main_conf *mcf = ngx_http_conf_get_module_main_conf(cf, + ngx_http_upstream_serverlist_module); + serverlist *sl = NULL; + sl = ngx_array_push(&mcf->serverlists); if (sl == NULL) { return NGX_CONF_ERROR; From 81ad622d02503484815afa17b7685a85c4fe7bc9 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 1 Sep 2020 21:40:48 -0400 Subject: [PATCH 05/43] testing order when loading directive --- ngx_http_upstream_serverlist.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 3678dc2..10d95fb 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -266,15 +266,14 @@ serverlist_service_directive(ngx_conf_t *cf, ngx_command_t *cmd, void *dummy) { static char * serverlist_directive(ngx_conf_t *cf, ngx_command_t *cmd, void *dummy) { - ngx_http_upstream_srv_conf_t *uscf = ngx_http_conf_get_module_srv_conf(cf, - ngx_http_upstream_module); - if (cf->args->nelts > 2) { ngx_conf_log_error(NGX_LOG_ERR, cf, 0, "upstream-serverlist: serverlist only need 0 or 1 args"); return NGX_CONF_ERROR; } + ngx_http_upstream_srv_conf_t *uscf = ngx_http_conf_get_module_srv_conf(cf, + ngx_http_upstream_module); main_conf *mcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_upstream_serverlist_module); serverlist *sl = NULL; From 11d91ff11c5f45c74074f9e3b5f02c7bf0dbe987 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Wed, 2 Sep 2020 07:26:08 -0400 Subject: [PATCH 06/43] testing order when loading directive --- ngx_http_upstream_serverlist.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 10d95fb..71bdd89 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -185,18 +185,18 @@ create_main_conf(ngx_conf_t *cf) { static char * serverlist_service_directive(ngx_conf_t *cf, ngx_command_t *cmd, void *dummy) { - main_conf *mcf = ngx_http_conf_get_module_main_conf(cf, - ngx_http_upstream_serverlist_module); - ngx_str_t *s = NULL; - ngx_uint_t i = 1; - ngx_int_t ret = -1; - if (cf->args->nelts <= 1) { ngx_conf_log_error(NGX_LOG_ERR, cf, 0, "upstream-serverlist: serverlist_service need at least 1 arg"); return NGX_CONF_ERROR; } + main_conf *mcf = ngx_http_conf_get_module_main_conf(cf, + ngx_http_upstream_serverlist_module); + ngx_str_t *s = NULL; + ngx_uint_t i = 1; + ngx_int_t ret = -1; + for (i = 1; i < cf->args->nelts; i++) { s = (ngx_str_t *)cf->args->elts + i; From 658fae366acba73eacbee52114fe43c73e729bc6 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Wed, 2 Sep 2020 07:32:44 -0400 Subject: [PATCH 07/43] testing order when loading directive --- ngx_http_upstream_serverlist.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 71bdd89..641a108 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -185,18 +185,18 @@ create_main_conf(ngx_conf_t *cf) { static char * serverlist_service_directive(ngx_conf_t *cf, ngx_command_t *cmd, void *dummy) { - if (cf->args->nelts <= 1) { - ngx_conf_log_error(NGX_LOG_ERR, cf, 0, - "upstream-serverlist: serverlist_service need at least 1 arg"); - return NGX_CONF_ERROR; - } - main_conf *mcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_upstream_serverlist_module); ngx_str_t *s = NULL; ngx_uint_t i = 1; ngx_int_t ret = -1; + if (cf->args->nelts <= 1) { + ngx_conf_log_error(NGX_LOG_ERR, cf, 0, + "upstream-serverlist: serverlist_service need at least 1 arg"); + return NGX_CONF_ERROR; + } + for (i = 1; i < cf->args->nelts; i++) { s = (ngx_str_t *)cf->args->elts + i; @@ -266,18 +266,18 @@ serverlist_service_directive(ngx_conf_t *cf, ngx_command_t *cmd, void *dummy) { static char * serverlist_directive(ngx_conf_t *cf, ngx_command_t *cmd, void *dummy) { - if (cf->args->nelts > 2) { - ngx_conf_log_error(NGX_LOG_ERR, cf, 0, - "upstream-serverlist: serverlist only need 0 or 1 args"); - return NGX_CONF_ERROR; - } - ngx_http_upstream_srv_conf_t *uscf = ngx_http_conf_get_module_srv_conf(cf, ngx_http_upstream_module); main_conf *mcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_upstream_serverlist_module); serverlist *sl = NULL; + if (cf->args->nelts > 2) { + ngx_conf_log_error(NGX_LOG_ERR, cf, 0, + "upstream-serverlist: serverlist only need 0 or 1 args"); + return NGX_CONF_ERROR; + } + sl = ngx_array_push(&mcf->serverlists); if (sl == NULL) { return NGX_CONF_ERROR; From c1a4e86a453aaaa5aaee815831ec73e2993ebef6 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Wed, 2 Sep 2020 09:21:58 -0400 Subject: [PATCH 08/43] add back exit process --- ngx_http_upstream_serverlist.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 641a108..f5bbb16 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -74,6 +74,9 @@ init_module(ngx_cycle_t *cycle); static ngx_int_t init_process(ngx_cycle_t *cycle); +static void +exit_process(ngx_cycle_t *cycle); + static void refresh_timeout_handler(ngx_event_t *ev); @@ -131,7 +134,7 @@ ngx_module_t ngx_http_upstream_serverlist_module = { init_process, /* init process */ NULL, /* init thread */ NULL, /* exit thread */ - NULL, /* exit process */ + exit_process, /* exit process */ NULL, /* exit master */ NGX_MODULE_V1_PADDING }; @@ -453,6 +456,29 @@ init_process(ngx_cycle_t *cycle) { return NGX_OK; } +static void +exit_process(ngx_cycle_t *cycle) { + main_conf *mcf = ngx_http_cycle_get_module_main_conf(cycle, + ngx_http_upstream_serverlist_module); + serverlist *sls = mcf->serverlists.elts; + service_conn *scs = mcf->service_conns.elts; + ngx_uint_t i; + + for (i = 0; i < mcf->serverlists.nelts; i++) { + if (sls[i].pool) { + ngx_destroy_pool(sls[i].pool); + sls[i].pool = NULL; + } + } + + for (i = 0; i < mcf->service_conns.nelts; i++) { + if (scs[i].peer_conn.connection) { + ngx_close_connection(scs[i].peer_conn.connection); + scs[i].peer_conn.connection = NULL; + } + } +} + static void empty_handler(ngx_event_t *ev) { ngx_log_debug(NGX_LOG_DEBUG_ALL, ev->log, 0, From d8876a3af634079b005ac00d461425655999f53b Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Wed, 2 Sep 2020 21:55:25 -0400 Subject: [PATCH 09/43] add note about sharing upstream across server names, remove exit_process again --- README.md | 2 ++ ngx_http_upstream_serverlist.c | 28 +--------------------------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index fe9191e..30a925a 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,8 @@ NOTE: One can use "Last-Modified" or "Etag" HTTP header in response to prevent wasted upstream refresh actions, Especially when thousands serverlists and upstreams configured. +NOTE: Different server names cannot share the same upstream right now, if they do, it will cause as sefgault at runtime + ## Directives ### serverlist_service * Syntax: `serverlist_service url=http://xxx/ [conf_dump_dir=dumped_dir/] [interval=5s] [timeout=2s] [concurrency=1];` diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index f5bbb16..641a108 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -74,9 +74,6 @@ init_module(ngx_cycle_t *cycle); static ngx_int_t init_process(ngx_cycle_t *cycle); -static void -exit_process(ngx_cycle_t *cycle); - static void refresh_timeout_handler(ngx_event_t *ev); @@ -134,7 +131,7 @@ ngx_module_t ngx_http_upstream_serverlist_module = { init_process, /* init process */ NULL, /* init thread */ NULL, /* exit thread */ - exit_process, /* exit process */ + NULL, /* exit process */ NULL, /* exit master */ NGX_MODULE_V1_PADDING }; @@ -456,29 +453,6 @@ init_process(ngx_cycle_t *cycle) { return NGX_OK; } -static void -exit_process(ngx_cycle_t *cycle) { - main_conf *mcf = ngx_http_cycle_get_module_main_conf(cycle, - ngx_http_upstream_serverlist_module); - serverlist *sls = mcf->serverlists.elts; - service_conn *scs = mcf->service_conns.elts; - ngx_uint_t i; - - for (i = 0; i < mcf->serverlists.nelts; i++) { - if (sls[i].pool) { - ngx_destroy_pool(sls[i].pool); - sls[i].pool = NULL; - } - } - - for (i = 0; i < mcf->service_conns.nelts; i++) { - if (scs[i].peer_conn.connection) { - ngx_close_connection(scs[i].peer_conn.connection); - scs[i].peer_conn.connection = NULL; - } - } -} - static void empty_handler(ngx_event_t *ev) { ngx_log_debug(NGX_LOG_DEBUG_ALL, ev->log, 0, From a7e7426604847ce1395e6539988c24e187bfe1a9 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 3 Sep 2020 01:44:39 -0400 Subject: [PATCH 10/43] update readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 30a925a..e95db5e 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ NOTE: One can use "Last-Modified" or "Etag" HTTP header in response to prevent wasted upstream refresh actions, Especially when thousands serverlists and upstreams configured. -NOTE: Different server names cannot share the same upstream right now, if they do, it will cause as sefgault at runtime +NOTE: Different server names cannot share the same upstream right now, if they do, it will cause as segfault at runtime. Will also segfault at runtime if you leave out the syntax in the config. ## Directives ### serverlist_service From 7d60a19c546776c09571ff9c292c5efc2a04d3da Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 3 Sep 2020 21:31:12 -0400 Subject: [PATCH 11/43] trying something out with main conf memory pool swaps --- ngx_http_upstream_serverlist.c | 43 +++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 641a108..8fe5996 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -18,6 +18,14 @@ #define CACHE_LINE_SIZE 128 #define DEFAULT_SERVERLIST_POOL_SIZE 1024 + +typedef struct +{ + ngx_queue_t queue; + ngx_pool_t *pool; + ngx_uint_t refer_num; /* to count the upstream that refers this memory pool*/ +} serverlist_pool_node_t; + typedef struct { ngx_pool_t *new_pool; ngx_pool_t *pool; @@ -807,6 +815,7 @@ get_one_line(u_char *buf, u_char *buf_end, ngx_str_t *line) { static ngx_array_t * get_servers(ngx_pool_t *pool, ngx_str_t *body, ngx_log_t *log) { ngx_int_t ret = -1; + // this is the pool that needs to be cleared ngx_array_t *servers = ngx_array_create(pool, 2, sizeof(ngx_http_upstream_server_t)); ngx_http_upstream_server_t *server = NULL; @@ -845,6 +854,7 @@ get_servers(ngx_pool_t *pool, ngx_str_t *body, ngx_log_t *log) { break; } + // this causes the memory leak when servers are never removed server = ngx_array_push(servers); ngx_memzero(server, sizeof *server); server->name = u.url; @@ -1071,7 +1081,33 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { // use mcf->pool, avoid coredump, will lead mem leak // new_servers = get_servers(sl->new_pool, body, log); - new_servers = get_servers(mcf->conf_pool, body, log); + /* + TODO: THE CONF POOL MUST BE CLEANED UP EVERYTIME THIS REFRESHES THE UPSTREAMS + */ + /* + pool_node = ngx_palloc( + mcf->conf_pool, sizeof(serverlist_pool_node_t)); + if (pool_node == NULL){ + ngx_log_error(NGX_LOG_ERR, ev->log, 0, + "upstream-serverlist: Could not create pool_node"); + goto exit; + } + pool_node->pool = mcf->conf_pool; + pool_node->refer_num = 0;*/ + + /* + + Try to add server a new pool + - if servers have changed + - swap new pool with old pool + - delete old pool + + //https://nginx.org/en/docs/dev/development_guide.html#pool + */ + + ngx_pool_t *tmp_pool = ngx_create_pool(DEFAULT_SERVERLIST_POOL_SIZE, log); + //new_servers = get_servers(mcf->conf_pool, body, log); + new_servers = get_servers(tmp_pool, body, log); if (new_servers == NULL || new_servers->nelts <= 0) { ngx_log_error(NGX_LOG_ERR, log, 0, "upstream-serverlist: parse serverlist %V failed", &sl->name); @@ -1079,6 +1115,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { } if (!upstream_servers_changed(uscf->servers, new_servers)) { + ngx_destroy_pool(tmp_pool); ngx_log_debug(NGX_LOG_INFO, log, 0, "upstream-serverlist: serverlist %V nothing changed",&sl->name); // once return -1, everything in the old pool will kept and the new pool @@ -1086,10 +1123,14 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { return -1; } + ngx_memzero(&cf, sizeof cf); cf.name = "serverlist_init_upstream"; cf.cycle = (ngx_cycle_t *) ngx_cycle; + // cf.pool = sl->new_pool; + mcf->conf_pool = tmp_pool; + cf.pool = mcf->conf_pool; cf.module_type = NGX_HTTP_MODULE; cf.cmd_type = NGX_HTTP_MAIN_CONF; From 0aede52cd1219d1f568d3afd198238915a671596 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Mon, 7 Sep 2020 23:35:52 -0400 Subject: [PATCH 12/43] try recreating main conf pool when refreshing upstreams --- ngx_http_upstream_serverlist.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 8fe5996..862904c 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1105,9 +1105,21 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { //https://nginx.org/en/docs/dev/development_guide.html#pool */ - ngx_pool_t *tmp_pool = ngx_create_pool(DEFAULT_SERVERLIST_POOL_SIZE, log); + + // create new temp main_conf with a new pool, copy info from existing conf except for the pool + cf.pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, log); + cf.ctx = mcf->conf_ctx; + + main_conf *tmp_mcf = create_main_conf(&cf); + tmp_mcf->conf_ctx= mcf->conf_ctx; + tmp_mcf->service_conns = mcf->service_conns; + tmp_mcf->serverlists = mcf->serverlists; + tmp_mcf->service_concurrency = mcf->service_concurrency; + tmp_mcf->service_url = mcf->service_url; + tmp_mcf->conf_dump_dir = mcf->conf_dump_dir; + //new_servers = get_servers(mcf->conf_pool, body, log); - new_servers = get_servers(tmp_pool, body, log); + new_servers = get_servers(tmp_mcf->conf_pool, body, log); if (new_servers == NULL || new_servers->nelts <= 0) { ngx_log_error(NGX_LOG_ERR, log, 0, "upstream-serverlist: parse serverlist %V failed", &sl->name); @@ -1115,7 +1127,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { } if (!upstream_servers_changed(uscf->servers, new_servers)) { - ngx_destroy_pool(tmp_pool); + ngx_destroy_pool(tmp_mcf->conf_pool); ngx_log_debug(NGX_LOG_INFO, log, 0, "upstream-serverlist: serverlist %V nothing changed",&sl->name); // once return -1, everything in the old pool will kept and the new pool @@ -1127,15 +1139,13 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_memzero(&cf, sizeof cf); cf.name = "serverlist_init_upstream"; cf.cycle = (ngx_cycle_t *) ngx_cycle; + - // cf.pool = sl->new_pool; - mcf->conf_pool = tmp_pool; - - cf.pool = mcf->conf_pool; + cf.pool = tmp_mcf->conf_pool; cf.module_type = NGX_HTTP_MODULE; cf.cmd_type = NGX_HTTP_MAIN_CONF; cf.log = ngx_cycle->log; - cf.ctx = mcf->conf_ctx; + cf.ctx = tmp_mcf->conf_ctx; old_servers = uscf->servers; uscf->servers = new_servers; @@ -1155,6 +1165,12 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { return -1; } + if (mcf->conf_pool != NULL) { + // destry old pool + ngx_destroy_pool(mcf->conf_pool); + mcf->conf_pool = NULL; + } + #if (NGX_HTTP_UPSTREAM_CHECK) if (ngx_http_upstream_check_update_upstream_peers(uscf, cf.pool) != NGX_OK) { From 648bdaeea3ec13fd037a4ff2e4af3b744fca3d89 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 01:35:47 -0400 Subject: [PATCH 13/43] some clean up --- ngx_http_upstream_serverlist.c | 35 ---------------------------------- 1 file changed, 35 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 862904c..fcf1313 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -18,14 +18,6 @@ #define CACHE_LINE_SIZE 128 #define DEFAULT_SERVERLIST_POOL_SIZE 1024 - -typedef struct -{ - ngx_queue_t queue; - ngx_pool_t *pool; - ngx_uint_t refer_num; /* to count the upstream that refers this memory pool*/ -} serverlist_pool_node_t; - typedef struct { ngx_pool_t *new_pool; ngx_pool_t *pool; @@ -1079,33 +1071,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_array_t *new_servers = NULL; ngx_array_t *old_servers = uscf->servers; - // use mcf->pool, avoid coredump, will lead mem leak - // new_servers = get_servers(sl->new_pool, body, log); - /* - TODO: THE CONF POOL MUST BE CLEANED UP EVERYTIME THIS REFRESHES THE UPSTREAMS - */ - /* - pool_node = ngx_palloc( - mcf->conf_pool, sizeof(serverlist_pool_node_t)); - if (pool_node == NULL){ - ngx_log_error(NGX_LOG_ERR, ev->log, 0, - "upstream-serverlist: Could not create pool_node"); - goto exit; - } - pool_node->pool = mcf->conf_pool; - pool_node->refer_num = 0;*/ - - /* - - Try to add server a new pool - - if servers have changed - - swap new pool with old pool - - delete old pool - - //https://nginx.org/en/docs/dev/development_guide.html#pool - */ - - // create new temp main_conf with a new pool, copy info from existing conf except for the pool cf.pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, log); cf.ctx = mcf->conf_ctx; From 098cbd7b304b710619fa82f17a94050ba7c0bd54 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 03:16:31 -0400 Subject: [PATCH 14/43] check for temp conf pool before trying to destroy it --- ngx_http_upstream_serverlist.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index fcf1313..3db9814 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1076,7 +1076,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { cf.ctx = mcf->conf_ctx; main_conf *tmp_mcf = create_main_conf(&cf); - tmp_mcf->conf_ctx= mcf->conf_ctx; + tmp_mcf->conf_ctx = mcf->conf_ctx; tmp_mcf->service_conns = mcf->service_conns; tmp_mcf->serverlists = mcf->serverlists; tmp_mcf->service_concurrency = mcf->service_concurrency; @@ -1092,7 +1092,11 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { } if (!upstream_servers_changed(uscf->servers, new_servers)) { - ngx_destroy_pool(tmp_mcf->conf_pool); + if (tmp_mcf->conf_pool != NULL) { + // destry temp pool + ngx_destroy_pool(tmp_mcf->conf_pool); + tmp_mcf->conf_pool = NULL; + } ngx_log_debug(NGX_LOG_INFO, log, 0, "upstream-serverlist: serverlist %V nothing changed",&sl->name); // once return -1, everything in the old pool will kept and the new pool From cd47391b2ba56cf86bb8923e6fe1dece9a1ecbc8 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 03:23:08 -0400 Subject: [PATCH 15/43] check for temp conf pool before trying to destroy it --- ngx_http_upstream_serverlist.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 3db9814..f00800f 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1134,12 +1134,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { return -1; } - if (mcf->conf_pool != NULL) { - // destry old pool - ngx_destroy_pool(mcf->conf_pool); - mcf->conf_pool = NULL; - } - #if (NGX_HTTP_UPSTREAM_CHECK) if (ngx_http_upstream_check_update_upstream_peers(uscf, cf.pool) != NGX_OK) { @@ -1150,6 +1144,12 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { #endif dump_serverlist(sl); + + if (mcf->conf_pool != NULL) { + // destry old pool + ngx_destroy_pool(mcf->conf_pool); + mcf->conf_pool = NULL; + } return 0; } From 06cb7bf14514c8c8c92c40fa337069876c07c60d Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 03:32:48 -0400 Subject: [PATCH 16/43] check for temp conf pool before trying to destroy it --- ngx_http_upstream_serverlist.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index f00800f..3ad3307 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1145,10 +1145,21 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { dump_serverlist(sl); - if (mcf->conf_pool != NULL) { - // destry old pool - ngx_destroy_pool(mcf->conf_pool); - mcf->conf_pool = NULL; + serverlist *old_sls = mcf->serverlists.elts; + service_conn *ols_scs = mcf->service_conns.elts; + ngx_uint_t i; + + for (i = 0; i < mcf->serverlists.nelts; i++) { + if (old_sls[i].pool) { + ngx_destroy_pool(sls[i].pool); + old_sls[i].pool = NULL; + } + } + for (i = 0; i < mcf->service_conns.nelts; i++) { + if (ols_scs[i].peer_conn.connection) { + ngx_close_connection(scs[i].peer_conn.connection); + ols_scs[i].peer_conn.connection = NULL; + } } return 0; } From 95a0f5014e4a38c2ebb4b3ff44078f37a62aa388 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 03:33:33 -0400 Subject: [PATCH 17/43] try to destroy serverlist conns and pools serperately --- ngx_http_upstream_serverlist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 3ad3307..c7cc542 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1151,13 +1151,13 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { for (i = 0; i < mcf->serverlists.nelts; i++) { if (old_sls[i].pool) { - ngx_destroy_pool(sls[i].pool); + ngx_destroy_pool(old_sls[i].pool); old_sls[i].pool = NULL; } } for (i = 0; i < mcf->service_conns.nelts; i++) { if (ols_scs[i].peer_conn.connection) { - ngx_close_connection(scs[i].peer_conn.connection); + ngx_close_connection(ols_scs[i].peer_conn.connection); ols_scs[i].peer_conn.connection = NULL; } } From 31299c4d56a9b92baabfb9e5902e5acfacdb228e Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 03:40:08 -0400 Subject: [PATCH 18/43] try to destroy serverlist conns and pools serperately --- ngx_http_upstream_serverlist.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index c7cc542..d717f1d 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1146,7 +1146,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { dump_serverlist(sl); serverlist *old_sls = mcf->serverlists.elts; - service_conn *ols_scs = mcf->service_conns.elts; ngx_uint_t i; for (i = 0; i < mcf->serverlists.nelts; i++) { @@ -1155,12 +1154,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { old_sls[i].pool = NULL; } } - for (i = 0; i < mcf->service_conns.nelts; i++) { - if (ols_scs[i].peer_conn.connection) { - ngx_close_connection(ols_scs[i].peer_conn.connection); - ols_scs[i].peer_conn.connection = NULL; - } - } return 0; } From f69c1f6d73d2ed299d11baf4aa0d8875b2c63261 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 05:03:41 -0400 Subject: [PATCH 19/43] need to init new server lists seperately into tmp conf --- ngx_http_upstream_serverlist.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index d717f1d..452b8a9 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1071,14 +1071,26 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_array_t *new_servers = NULL; ngx_array_t *old_servers = uscf->servers; - // create new temp main_conf with a new pool, copy info from existing conf except for the pool + // create new temp main_conf with a new pool and new serverlists, copy info from existing conf except for the pool and serverlist cf.pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, log); cf.ctx = mcf->conf_ctx; main_conf *tmp_mcf = create_main_conf(&cf); tmp_mcf->conf_ctx = mcf->conf_ctx; tmp_mcf->service_conns = mcf->service_conns; - tmp_mcf->serverlists = mcf->serverlists; + + serverlist *new_sl = NULL; + new_sl = ngx_array_push(&tmp_mcf->serverlists); + if (new_sl == NULL) { + ngx_log_error(NGX_LOG_ERR, log, 0, + "upstream-serverlist: temp serverlists conf %V failed", uscf->host); + return -1; + } + ngx_memzero(new_sl, sizeof *new_sl); + new_sl->upstream_conf = uscf; + new_sl->last_modified = -1; + new_sl->name = uscf->host; + tmp_mcf->service_concurrency = mcf->service_concurrency; tmp_mcf->service_url = mcf->service_url; tmp_mcf->conf_dump_dir = mcf->conf_dump_dir; @@ -1153,6 +1165,11 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_destroy_pool(old_sls[i].pool); old_sls[i].pool = NULL; } + + if (old_sls[i].new_pool) { + ngx_destroy_pool(old_sls[i].new_pool); + old_sls[i].new_pool = NULL; + } } return 0; } From eb98b5d4cb194cb8ed4e793cb4f8ba67563947e5 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 07:18:33 -0400 Subject: [PATCH 20/43] use new conns when refreshing upstream --- ngx_http_upstream_serverlist.c | 46 +++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 452b8a9..e1ae650 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -167,7 +167,7 @@ create_main_conf(ngx_conf_t *cf) { return NULL; } - if (ngx_array_init(&mcf->service_conns, cf->pool, 1, + if (ngx_array_init(&mcf->service_conns, cf->pool, 1, sizeof(service_conn)) != NGX_OK) { return NULL; } @@ -1071,14 +1071,15 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_array_t *new_servers = NULL; ngx_array_t *old_servers = uscf->servers; - // create new temp main_conf with a new pool and new serverlists, copy info from existing conf except for the pool and serverlist + // create new temp main_conf with a new pools, new service_conns and new serverlists, copy info from existing conf except for the pools, service_conns and serverlists cf.pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, log); cf.ctx = mcf->conf_ctx; main_conf *tmp_mcf = create_main_conf(&cf); tmp_mcf->conf_ctx = mcf->conf_ctx; - tmp_mcf->service_conns = mcf->service_conns; + + // create new server list serverlist *new_sl = NULL; new_sl = ngx_array_push(&tmp_mcf->serverlists); if (new_sl == NULL) { @@ -1095,6 +1096,39 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { tmp_mcf->service_url = mcf->service_url; tmp_mcf->conf_dump_dir = mcf->conf_dump_dir; + //create new conns + service_conn *new_sc = NULL; + new_sc = ngx_array_push(&tmp_mcf->service_conns); + ngx_memzero(new_sc, sizeof *new_sc); + new_sc->send.start = ngx_pcalloc(tmp_mcf->conf_pool, MAX_HTTP_REQUEST_SIZE); + if (new_sc->send.start == NULL) { + ngx_log_error(NGX_LOG_ERR, log, 0, + "upstream-serverlist: new allocate send buffer failed"); + return NGX_ERROR; + } + new_sc->send.end = new_sc->send.start + MAX_HTTP_REQUEST_SIZE; + new_sc->send.last = new_sc->send.pos = new_sc->send.start; + + new_sc->recv.start = ngx_pcalloc(tmp_mcf->conf_pool, ngx_pagesize); + if (new_sc->recv.start == NULL) { + ngx_log_error(NGX_LOG_ERR, log, 0, + "upstream-serverlist: new allocate recv buffer failed"); + return NGX_ERROR; + } + new_sc->recv.end = new_sc->recv.start + MAX_HTTP_REQUEST_SIZE; + new_sc->recv.last = new_sc->recv.pos = new_sc->recv.start; + + ngx_memzero(&new_sc->peer_conn, sizeof new_sc->peer_conn); + new_sc->peer_conn.data = NULL; + new_sc->peer_conn.log = log; + new_sc->peer_conn.log_error = NGX_ERROR_ERR; + new_sc->peer_conn.connection = NULL; + new_sc->peer_conn.get = ngx_event_get_peer; + new_sc->peer_conn.name = &tmp_mcf->service_url.host; + new_sc->peer_conn.sockaddr = &tmp_mcf->service_url.sockaddr.sockaddr; + new_sc->peer_conn.socklen = tmp_mcf->service_url.socklen; + + //new_servers = get_servers(mcf->conf_pool, body, log); new_servers = get_servers(tmp_mcf->conf_pool, body, log); if (new_servers == NULL || new_servers->nelts <= 0) { @@ -1158,6 +1192,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { dump_serverlist(sl); serverlist *old_sls = mcf->serverlists.elts; + service_conn *old_scs = mcf->service_conns.elts; ngx_uint_t i; for (i = 0; i < mcf->serverlists.nelts; i++) { @@ -1170,6 +1205,11 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_destroy_pool(old_sls[i].new_pool); old_sls[i].new_pool = NULL; } + + if (old_scs[i].peer_conn.connection) { + ngx_close_connection(old_scs[i].peer_conn.connection); + old_scs[i].peer_conn.connection = NULL; + } } return 0; } From 7b16ce175d04f00f8a2d04c55f6a6cd6bfa6aff5 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 07:57:18 -0400 Subject: [PATCH 21/43] dont free old cons for now, worker process keeps getting killed with bad free, prob need reference counting --- ngx_http_upstream_serverlist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index e1ae650..68eccb6 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1205,11 +1205,11 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_destroy_pool(old_sls[i].new_pool); old_sls[i].new_pool = NULL; } - + /* if (old_scs[i].peer_conn.connection) { ngx_close_connection(old_scs[i].peer_conn.connection); old_scs[i].peer_conn.connection = NULL; - } + }*/ } return 0; } From aae33c22f92c4fcbb611790cdab56448ff111c98 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 07:58:17 -0400 Subject: [PATCH 22/43] dont free old cons for now, worker process keeps getting killed with bad free, prob need reference counting --- ngx_http_upstream_serverlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 68eccb6..826ab58 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1192,7 +1192,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { dump_serverlist(sl); serverlist *old_sls = mcf->serverlists.elts; - service_conn *old_scs = mcf->service_conns.elts; + //service_conn *old_scs = mcf->service_conns.elts; ngx_uint_t i; for (i = 0; i < mcf->serverlists.nelts; i++) { From 203b403c3c041c665bb041454bf5ebd3b2643ead Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 22:22:08 -0400 Subject: [PATCH 23/43] dump new_sl, free old servers and old conns --- ngx_http_upstream_serverlist.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 826ab58..321b1e2 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1189,10 +1189,10 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { } #endif - dump_serverlist(sl); + dump_serverlist(new_sl); serverlist *old_sls = mcf->serverlists.elts; - //service_conn *old_scs = mcf->service_conns.elts; + service_conn *old_scs = mcf->service_conns.elts; ngx_uint_t i; for (i = 0; i < mcf->serverlists.nelts; i++) { @@ -1205,12 +1205,25 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_destroy_pool(old_sls[i].new_pool); old_sls[i].new_pool = NULL; } - /* + if (old_scs[i].peer_conn.connection) { ngx_close_connection(old_scs[i].peer_conn.connection); old_scs[i].peer_conn.connection = NULL; - }*/ + } + } + + if (old_servers != NULL) { + // destry old old_servers + ngx_array_destroy(old_servers); + old_servers = NULL; } + + if (mcf->conf_pool != NULL) { + // destry old conf_pool + ngx_destroy_pool(mcf->conf_pool); + mcf->conf_pool = NULL; + } + return 0; } From c638fa19e26d81cd1ea466486e9fec390779bdf2 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 22:46:03 -0400 Subject: [PATCH 24/43] just try to free old servers --- ngx_http_upstream_serverlist.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 321b1e2..e35b4eb 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1192,7 +1192,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { dump_serverlist(new_sl); serverlist *old_sls = mcf->serverlists.elts; - service_conn *old_scs = mcf->service_conns.elts; + //service_conn *old_scs = mcf->service_conns.elts; ngx_uint_t i; for (i = 0; i < mcf->serverlists.nelts; i++) { @@ -1206,10 +1206,10 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { old_sls[i].new_pool = NULL; } - if (old_scs[i].peer_conn.connection) { + /*if (old_scs[i].peer_conn.connection) { ngx_close_connection(old_scs[i].peer_conn.connection); old_scs[i].peer_conn.connection = NULL; - } + }*/ } if (old_servers != NULL) { @@ -1218,11 +1218,12 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { old_servers = NULL; } + /* if (mcf->conf_pool != NULL) { // destry old conf_pool ngx_destroy_pool(mcf->conf_pool); mcf->conf_pool = NULL; - } + }*/ return 0; } From a98607aad9d1c159c4ac5f848dc11bf92f1ed03b Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 22:55:20 -0400 Subject: [PATCH 25/43] just try to free old servers --- ngx_http_upstream_serverlist.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index e35b4eb..c69c993 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1189,6 +1189,8 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { } #endif + + ngx_shmtx_unlock(&new_sl->dump_file_lock); dump_serverlist(new_sl); serverlist *old_sls = mcf->serverlists.elts; From f6a96f07cea43aa038dd2a6a3397e20235074166 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Tue, 8 Sep 2020 23:08:01 -0400 Subject: [PATCH 26/43] just try to free old servers --- ngx_http_upstream_serverlist.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index c69c993..e77b085 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1190,7 +1190,22 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { #endif - ngx_shmtx_unlock(&new_sl->dump_file_lock); + ngx_shm_t shm = {0}; + shm.size = CACHE_LINE_SIZE * tmp_mcf->serverlists.nelts; + shm.log = log; + ngx_str_set(&shm.name, "upstream-serverlist-shared-zone"); + if (ngx_shm_alloc(&shm) != NGX_OK) { + return -1; + } + for (uint i = 0; i < tmp_mcf->serverlists.nelts; i++) { + serverlist *temp_sl = (serverlist *)tmp_mcf->serverlists.elts + i; + ngx_int_t ret = ngx_shmtx_create(&temp_sl->dump_file_lock, + (ngx_shmtx_sh_t *)(shm.addr + CACHE_LINE_SIZE * i), NULL); + if ( ret != NGX_OK) { + return -1; + } + } + dump_serverlist(new_sl); serverlist *old_sls = mcf->serverlists.elts; From 3c410a3da6a088adaa9b4385a8fe692c2f01711b Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 04:04:55 -0400 Subject: [PATCH 27/43] trying to figure out reinit and destruction of serverlistcons in refresh_upstreams --- ngx_http_upstream_serverlist.c | 51 +++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index e77b085..51a46bc 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1129,6 +1129,33 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { new_sc->peer_conn.socklen = tmp_mcf->service_url.socklen; + ngx_uint_t blocksize = 0; + if (tmp_mcf->serverlists.nelts >= tmp_mcf->service_concurrency) { + blocksize = (tmp_mcf->serverlists.nelts + (tmp_mcf->service_concurrency - 1)) + / tmp_mcf->service_concurrency; + } else { + blocksize = 1; + } + + new_sc->serverlists_start = ngx_min(tmp_mcf->serverlists.nelts, + 0 + blocksize); + new_sc->serverlists_end = ngx_min(tmp_mcf->serverlists.nelts, + new_sc->serverlists_start + blocksize); + new_sc->serverlists_curr = new_sc->serverlists_start; + + for (int i = 0; i < tmp_mcf->service_conns.nelts; i++) { + service_conn *tmp_sc = (service_conn *)tmp_mcf->service_conns.elts + i; + tmp_sc->timeout_timer.handler = refresh_timeout_handler; + tmp_sc->timeout_timer.log = log; + tmp_sc->timeout_timer.data = tmp_sc; + tmp_sc->refresh_timer.handler = connect_to_service; + tmp_sc->refresh_timer.log = log; + tmp_sc->refresh_timer.data = tmp_sc; + if ((ngx_uint_t)tmp_sc->serverlists_start < tmp_mcf->serverlists.nelts) { + ngx_add_timer(&tmp_sc->refresh_timer, random_interval_ms()); + } + } + //new_servers = get_servers(mcf->conf_pool, body, log); new_servers = get_servers(tmp_mcf->conf_pool, body, log); if (new_servers == NULL || new_servers->nelts <= 0) { @@ -1189,7 +1216,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { } #endif - ngx_shm_t shm = {0}; shm.size = CACHE_LINE_SIZE * tmp_mcf->serverlists.nelts; shm.log = log; @@ -1209,10 +1235,15 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { dump_serverlist(new_sl); serverlist *old_sls = mcf->serverlists.elts; - //service_conn *old_scs = mcf->service_conns.elts; + service_conn *old_scs = mcf->service_conns.elts; ngx_uint_t i; for (i = 0; i < mcf->serverlists.nelts; i++) { + /*if (old_scs[i].peer_conn.connection) { + ngx_close_connection(old_scs[i].peer_conn.connection); + old_scs[i].peer_conn.connection = NULL; + }*/ + if (old_sls[i].pool) { ngx_destroy_pool(old_sls[i].pool); old_sls[i].pool = NULL; @@ -1222,19 +1253,25 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_destroy_pool(old_sls[i].new_pool); old_sls[i].new_pool = NULL; } - - /*if (old_scs[i].peer_conn.connection) { - ngx_close_connection(old_scs[i].peer_conn.connection); - old_scs[i].peer_conn.connection = NULL; - }*/ } + /*if (old_sls != NULL) { + ngx_array_destroy(old_sls); + old_sls = NULL; + }*/ + if (old_servers != NULL) { // destry old old_servers ngx_array_destroy(old_servers); old_servers = NULL; } + if (old_scs != NULL) { + // destroy oll conds + ngx_array_destroy(old_scs); + old_scs = NULL; + } + /* if (mcf->conf_pool != NULL) { // destry old conf_pool From 35e07a18758cb8f1e888960642e5dc91257eebb5 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 04:47:28 -0400 Subject: [PATCH 28/43] fix compile issues --- ngx_http_upstream_serverlist.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 51a46bc..c3d899b 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1143,7 +1143,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { new_sc->serverlists_start + blocksize); new_sc->serverlists_curr = new_sc->serverlists_start; - for (int i = 0; i < tmp_mcf->service_conns.nelts; i++) { + for (ngx_uint_t i = 0; i < tmp_mcf->service_conns.nelts; i++) { service_conn *tmp_sc = (service_conn *)tmp_mcf->service_conns.elts + i; tmp_sc->timeout_timer.handler = refresh_timeout_handler; tmp_sc->timeout_timer.log = log; @@ -1223,7 +1223,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { if (ngx_shm_alloc(&shm) != NGX_OK) { return -1; } - for (uint i = 0; i < tmp_mcf->serverlists.nelts; i++) { + for (ngx_uint_t i = 0; i < tmp_mcf->serverlists.nelts; i++) { serverlist *temp_sl = (serverlist *)tmp_mcf->serverlists.elts + i; ngx_int_t ret = ngx_shmtx_create(&temp_sl->dump_file_lock, (ngx_shmtx_sh_t *)(shm.addr + CACHE_LINE_SIZE * i), NULL); @@ -1236,9 +1236,8 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { serverlist *old_sls = mcf->serverlists.elts; service_conn *old_scs = mcf->service_conns.elts; - ngx_uint_t i; - for (i = 0; i < mcf->serverlists.nelts; i++) { + for (ngx_uint_t i = 0; i < mcf->serverlists.nelts; i++) { /*if (old_scs[i].peer_conn.connection) { ngx_close_connection(old_scs[i].peer_conn.connection); old_scs[i].peer_conn.connection = NULL; @@ -1268,7 +1267,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { if (old_scs != NULL) { // destroy oll conds - ngx_array_destroy(old_scs); + ngx_array_destroy(&mcf->service_conns); old_scs = NULL; } From b0888b923816492fc214039887f7bd31e09b646e Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:01:59 -0400 Subject: [PATCH 29/43] try close connections --- ngx_http_upstream_serverlist.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index c3d899b..11de4ef 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1069,7 +1069,8 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_http_upstream_srv_conf_t *uscf = sl->upstream_conf; ngx_conf_t cf = {0}; ngx_array_t *new_servers = NULL; - ngx_array_t *old_servers = uscf->servers; + ngx_array_t *new_service_conns = NULL; + // create new temp main_conf with a new pools, new service_conns and new serverlists, copy info from existing conf except for the pools, service_conns and serverlists cf.pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, log); @@ -1189,9 +1190,12 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { cf.log = ngx_cycle->log; cf.ctx = tmp_mcf->conf_ctx; - old_servers = uscf->servers; + ngx_array_t *old_servers = uscf->servers; uscf->servers = new_servers; + ngx_array_t *old_service_conns = mcf->service_conns; + + if (ngx_http_upstream_init_round_robin(&cf, uscf) != NGX_OK) { // see: https://github.com/GUI/nginx-upstream-dynamic-servers/pull/33/files /* if you read the native code you can find out that all you need to do here is ngx_http_upstream_init_round_robin if you don't use other third party modules in the init process, @@ -1238,10 +1242,10 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { service_conn *old_scs = mcf->service_conns.elts; for (ngx_uint_t i = 0; i < mcf->serverlists.nelts; i++) { - /*if (old_scs[i].peer_conn.connection) { + if (old_scs[i].peer_conn.connection) { ngx_close_connection(old_scs[i].peer_conn.connection); old_scs[i].peer_conn.connection = NULL; - }*/ + } if (old_sls[i].pool) { ngx_destroy_pool(old_sls[i].pool); From ad637c3ed6b22decbaeb9e4e2c0e306f80a9006c Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:07:33 -0400 Subject: [PATCH 30/43] try close connections --- ngx_http_upstream_serverlist.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 11de4ef..4a0aaee 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1068,9 +1068,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_http_upstream_serverlist_module); ngx_http_upstream_srv_conf_t *uscf = sl->upstream_conf; ngx_conf_t cf = {0}; - ngx_array_t *new_servers = NULL; - ngx_array_t *new_service_conns = NULL; - + ngx_array_t *new_servers = NULL; // create new temp main_conf with a new pools, new service_conns and new serverlists, copy info from existing conf except for the pools, service_conns and serverlists cf.pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, log); @@ -1193,7 +1191,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_array_t *old_servers = uscf->servers; uscf->servers = new_servers; - ngx_array_t *old_service_conns = mcf->service_conns; + ngx_array_t *old_service_conns = &mcf->service_conns; if (ngx_http_upstream_init_round_robin(&cf, uscf) != NGX_OK) { @@ -1269,10 +1267,10 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { old_servers = NULL; } - if (old_scs != NULL) { + if (old_service_conns != NULL) { // destroy oll conds - ngx_array_destroy(&mcf->service_conns); - old_scs = NULL; + ngx_array_destroy(old_service_conns); + old_service_conns = NULL; } /* From 350aa58fa29ff71b2d1a2e4b7464e156887c98bc Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:09:40 -0400 Subject: [PATCH 31/43] try close connections --- ngx_http_upstream_serverlist.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 4a0aaee..5fedd3f 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1240,10 +1240,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { service_conn *old_scs = mcf->service_conns.elts; for (ngx_uint_t i = 0; i < mcf->serverlists.nelts; i++) { - if (old_scs[i].peer_conn.connection) { - ngx_close_connection(old_scs[i].peer_conn.connection); - old_scs[i].peer_conn.connection = NULL; - } if (old_sls[i].pool) { ngx_destroy_pool(old_sls[i].pool); @@ -1256,11 +1252,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { } } - /*if (old_sls != NULL) { - ngx_array_destroy(old_sls); - old_sls = NULL; - }*/ - if (old_servers != NULL) { // destry old old_servers ngx_array_destroy(old_servers); From 6b023f56cbce56ca5bbdef5e04116d3cb7d1e620 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:10:25 -0400 Subject: [PATCH 32/43] try close connections --- ngx_http_upstream_serverlist.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 5fedd3f..a8e2863 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1237,8 +1237,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { dump_serverlist(new_sl); serverlist *old_sls = mcf->serverlists.elts; - service_conn *old_scs = mcf->service_conns.elts; - + for (ngx_uint_t i = 0; i < mcf->serverlists.nelts; i++) { if (old_sls[i].pool) { From 38f4d785d3c77ea5fdca898c1bc0a8fdbd3fdd5b Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:12:36 -0400 Subject: [PATCH 33/43] try close connections --- ngx_http_upstream_serverlist.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index a8e2863..8ba116d 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1237,7 +1237,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { dump_serverlist(new_sl); serverlist *old_sls = mcf->serverlists.elts; - + for (ngx_uint_t i = 0; i < mcf->serverlists.nelts; i++) { if (old_sls[i].pool) { @@ -1257,11 +1257,11 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { old_servers = NULL; } - if (old_service_conns != NULL) { + /*if (old_service_conns != NULL) { // destroy oll conds ngx_array_destroy(old_service_conns); old_service_conns = NULL; - } + }*/ /* if (mcf->conf_pool != NULL) { From 64f6cf65ec57658488d5034912234fe095e1f7e9 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:13:13 -0400 Subject: [PATCH 34/43] try close connections --- ngx_http_upstream_serverlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 8ba116d..9f2feeb 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1191,7 +1191,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_array_t *old_servers = uscf->servers; uscf->servers = new_servers; - ngx_array_t *old_service_conns = &mcf->service_conns; + //ngx_array_t *old_service_conns = &mcf->service_conns; if (ngx_http_upstream_init_round_robin(&cf, uscf) != NGX_OK) { From 2dbaea349643119c677e600d9c0736360f705949 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:16:14 -0400 Subject: [PATCH 35/43] try close connections --- ngx_http_upstream_serverlist.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 9f2feeb..8cd5470 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1142,7 +1142,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { new_sc->serverlists_start + blocksize); new_sc->serverlists_curr = new_sc->serverlists_start; - for (ngx_uint_t i = 0; i < tmp_mcf->service_conns.nelts; i++) { + /*for (ngx_uint_t i = 0; i < tmp_mcf->service_conns.nelts; i++) { service_conn *tmp_sc = (service_conn *)tmp_mcf->service_conns.elts + i; tmp_sc->timeout_timer.handler = refresh_timeout_handler; tmp_sc->timeout_timer.log = log; @@ -1153,7 +1153,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { if ((ngx_uint_t)tmp_sc->serverlists_start < tmp_mcf->serverlists.nelts) { ngx_add_timer(&tmp_sc->refresh_timer, random_interval_ms()); } - } + }*/ //new_servers = get_servers(mcf->conf_pool, body, log); new_servers = get_servers(tmp_mcf->conf_pool, body, log); @@ -1191,7 +1191,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_array_t *old_servers = uscf->servers; uscf->servers = new_servers; - //ngx_array_t *old_service_conns = &mcf->service_conns; + ngx_array_t *old_service_conns = &mcf->service_conns; if (ngx_http_upstream_init_round_robin(&cf, uscf) != NGX_OK) { @@ -1257,11 +1257,11 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { old_servers = NULL; } - /*if (old_service_conns != NULL) { + if (old_service_conns != NULL) { // destroy oll conds ngx_array_destroy(old_service_conns); old_service_conns = NULL; - }*/ + } /* if (mcf->conf_pool != NULL) { From a31077cfae87eb6f4481ac4c9a4360ca7fbda01f Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:22:27 -0400 Subject: [PATCH 36/43] try close connections --- ngx_http_upstream_serverlist.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 8cd5470..039f3f0 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1126,7 +1126,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { new_sc->peer_conn.name = &tmp_mcf->service_url.host; new_sc->peer_conn.sockaddr = &tmp_mcf->service_url.sockaddr.sockaddr; new_sc->peer_conn.socklen = tmp_mcf->service_url.socklen; - + /* ngx_uint_t blocksize = 0; if (tmp_mcf->serverlists.nelts >= tmp_mcf->service_concurrency) { @@ -1140,9 +1140,9 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { 0 + blocksize); new_sc->serverlists_end = ngx_min(tmp_mcf->serverlists.nelts, new_sc->serverlists_start + blocksize); - new_sc->serverlists_curr = new_sc->serverlists_start; + new_sc->serverlists_curr = new_sc->serverlists_start;*/ - /*for (ngx_uint_t i = 0; i < tmp_mcf->service_conns.nelts; i++) { + for (ngx_uint_t i = 0; i < tmp_mcf->service_conns.nelts; i++) { service_conn *tmp_sc = (service_conn *)tmp_mcf->service_conns.elts + i; tmp_sc->timeout_timer.handler = refresh_timeout_handler; tmp_sc->timeout_timer.log = log; @@ -1153,7 +1153,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { if ((ngx_uint_t)tmp_sc->serverlists_start < tmp_mcf->serverlists.nelts) { ngx_add_timer(&tmp_sc->refresh_timer, random_interval_ms()); } - }*/ + } //new_servers = get_servers(mcf->conf_pool, body, log); new_servers = get_servers(tmp_mcf->conf_pool, body, log); From 3398022d838298d1afb81eb860196385a169619f Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:23:53 -0400 Subject: [PATCH 37/43] try close connections --- ngx_http_upstream_serverlist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 039f3f0..ad5bce5 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1126,7 +1126,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { new_sc->peer_conn.name = &tmp_mcf->service_url.host; new_sc->peer_conn.sockaddr = &tmp_mcf->service_url.sockaddr.sockaddr; new_sc->peer_conn.socklen = tmp_mcf->service_url.socklen; - /* + ngx_uint_t blocksize = 0; if (tmp_mcf->serverlists.nelts >= tmp_mcf->service_concurrency) { @@ -1140,7 +1140,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { 0 + blocksize); new_sc->serverlists_end = ngx_min(tmp_mcf->serverlists.nelts, new_sc->serverlists_start + blocksize); - new_sc->serverlists_curr = new_sc->serverlists_start;*/ + new_sc->serverlists_curr = new_sc->serverlists_start; for (ngx_uint_t i = 0; i < tmp_mcf->service_conns.nelts; i++) { service_conn *tmp_sc = (service_conn *)tmp_mcf->service_conns.elts + i; From 080395a68a69e29e3a25d3ec247888eba2419aa2 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:31:59 -0400 Subject: [PATCH 38/43] try close connections --- ngx_http_upstream_serverlist.c | 57 +++++++++++++++++----------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index ad5bce5..b7596c9 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1127,34 +1127,6 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { new_sc->peer_conn.sockaddr = &tmp_mcf->service_url.sockaddr.sockaddr; new_sc->peer_conn.socklen = tmp_mcf->service_url.socklen; - - ngx_uint_t blocksize = 0; - if (tmp_mcf->serverlists.nelts >= tmp_mcf->service_concurrency) { - blocksize = (tmp_mcf->serverlists.nelts + (tmp_mcf->service_concurrency - 1)) - / tmp_mcf->service_concurrency; - } else { - blocksize = 1; - } - - new_sc->serverlists_start = ngx_min(tmp_mcf->serverlists.nelts, - 0 + blocksize); - new_sc->serverlists_end = ngx_min(tmp_mcf->serverlists.nelts, - new_sc->serverlists_start + blocksize); - new_sc->serverlists_curr = new_sc->serverlists_start; - - for (ngx_uint_t i = 0; i < tmp_mcf->service_conns.nelts; i++) { - service_conn *tmp_sc = (service_conn *)tmp_mcf->service_conns.elts + i; - tmp_sc->timeout_timer.handler = refresh_timeout_handler; - tmp_sc->timeout_timer.log = log; - tmp_sc->timeout_timer.data = tmp_sc; - tmp_sc->refresh_timer.handler = connect_to_service; - tmp_sc->refresh_timer.log = log; - tmp_sc->refresh_timer.data = tmp_sc; - if ((ngx_uint_t)tmp_sc->serverlists_start < tmp_mcf->serverlists.nelts) { - ngx_add_timer(&tmp_sc->refresh_timer, random_interval_ms()); - } - } - //new_servers = get_servers(mcf->conf_pool, body, log); new_servers = get_servers(tmp_mcf->conf_pool, body, log); if (new_servers == NULL || new_servers->nelts <= 0) { @@ -1194,6 +1166,35 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { ngx_array_t *old_service_conns = &mcf->service_conns; + + ngx_uint_t blocksize = 0; + if (tmp_mcf->serverlists.nelts >= tmp_mcf->service_concurrency) { + blocksize = (tmp_mcf->serverlists.nelts + (tmp_mcf->service_concurrency - 1)) + / tmp_mcf->service_concurrency; + } else { + blocksize = 1; + } + + new_sc->serverlists_start = ngx_min(tmp_mcf->serverlists.nelts, + 0 + blocksize); + new_sc->serverlists_end = ngx_min(tmp_mcf->serverlists.nelts, + new_sc->serverlists_start + blocksize); + new_sc->serverlists_curr = new_sc->serverlists_start; + + for (ngx_uint_t i = 0; i < tmp_mcf->service_conns.nelts; i++) { + service_conn *tmp_sc = (service_conn *)tmp_mcf->service_conns.elts + i; + tmp_sc->timeout_timer.handler = refresh_timeout_handler; + tmp_sc->timeout_timer.log = log; + tmp_sc->timeout_timer.data = tmp_sc; + tmp_sc->refresh_timer.handler = connect_to_service; + tmp_sc->refresh_timer.log = log; + tmp_sc->refresh_timer.data = tmp_sc; + if ((ngx_uint_t)tmp_sc->serverlists_start < tmp_mcf->serverlists.nelts) { + ngx_add_timer(&tmp_sc->refresh_timer, random_interval_ms()); + } + } + + if (ngx_http_upstream_init_round_robin(&cf, uscf) != NGX_OK) { // see: https://github.com/GUI/nginx-upstream-dynamic-servers/pull/33/files /* if you read the native code you can find out that all you need to do here is ngx_http_upstream_init_round_robin if you don't use other third party modules in the init process, From f2e025fecdd318ab46f5cbc46b8b026ac509f036 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 10 Sep 2020 05:55:51 -0400 Subject: [PATCH 39/43] try to destroy old server listst --- ngx_http_upstream_serverlist.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index b7596c9..938a131 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1164,8 +1164,7 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { uscf->servers = new_servers; ngx_array_t *old_service_conns = &mcf->service_conns; - - + ngx_array_t *old_serverlists = &mcf->serverlists; ngx_uint_t blocksize = 0; if (tmp_mcf->serverlists.nelts >= tmp_mcf->service_concurrency) { @@ -1259,11 +1258,17 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { } if (old_service_conns != NULL) { - // destroy oll conds + // destroy old conns ngx_array_destroy(old_service_conns); old_service_conns = NULL; } + if (old_serverlists != NULL) { + // destroy old old_serverlists + ngx_array_destroy(old_serverlists); + old_serverlists = NULL; + } + /* if (mcf->conf_pool != NULL) { // destry old conf_pool From a4374903d589d1e9e80aa87f677ffa863f89c40f Mon Sep 17 00:00:00 2001 From: Cinque McFarlane-Blake <1519017+cinquemb@users.noreply.github.com> Date: Wed, 16 Sep 2020 01:17:22 +0000 Subject: [PATCH 40/43] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e95db5e..6ecb5e5 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ NOTE: One can use "Last-Modified" or "Etag" HTTP header in response to prevent wasted upstream refresh actions, Especially when thousands serverlists and upstreams configured. -NOTE: Different server names cannot share the same upstream right now, if they do, it will cause as segfault at runtime. Will also segfault at runtime if you leave out the syntax in the config. +NOTE: Will also segfault at runtime if you leave out the syntax for serverlist upstream in the config. ## Directives ### serverlist_service From 9bff1aef0d4c05f4062908663d1f294238eaad0a Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Thu, 1 Oct 2020 23:11:34 -0400 Subject: [PATCH 41/43] try destroying old pool in next success upstream refresh iteration --- ngx_http_upstream_serverlist.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 938a131..dae8225 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -48,10 +48,12 @@ typedef struct { typedef struct { ngx_http_conf_ctx_t *conf_ctx; ngx_pool_t *conf_pool; + ngx_pool_t *prev_conf_pool; ngx_array_t service_conns; ngx_array_t serverlists; ngx_uint_t service_concurrency; + ngx_int_t conf_pool_count; ngx_url_t service_url; ngx_str_t conf_dump_dir; } main_conf; @@ -180,6 +182,8 @@ create_main_conf(ngx_conf_t *cf) { mcf->service_concurrency = DEFAULT_SERVICE_CONCURRENCY; mcf->conf_ctx = cf->ctx; mcf->conf_pool = cf->pool; + mcf->conf_pool_count = 0; + return mcf; } @@ -1076,6 +1080,9 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { main_conf *tmp_mcf = create_main_conf(&cf); tmp_mcf->conf_ctx = mcf->conf_ctx; + + //copy over previous count + tmp_mcf->conf_pool_count = mcf->conf_pool_count; // create new server list @@ -1269,13 +1276,17 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { old_serverlists = NULL; } - /* - if (mcf->conf_pool != NULL) { - // destry old conf_pool - ngx_destroy_pool(mcf->conf_pool); - mcf->conf_pool = NULL; - }*/ + if (tmp_mcf->conf_pool_count > 0){ + //destry previous pool + if (tmp_mcf->prev_conf_pool != NULL) { + // destry old conf_pool + ngx_destroy_pool(tmp_mcf->prev_conf_pool); + tmp_mcf->prev_conf_pool = NULL; + } + } + tmp_mcf->prev_conf_pool = mcf->conf_pool; + tmp_mcf->conf_pool_count++; return 0; } From aa59f7244e15713c2454e42100a9aa9f26b8d941 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Fri, 2 Oct 2020 03:29:34 -0400 Subject: [PATCH 42/43] try to destroy new_pool when done with it in recv_from_service --- ngx_http_upstream_serverlist.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index dae8225..9c87dd1 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1558,8 +1558,13 @@ recv_from_service(ngx_event_t *ev) { // the pool is NULL at first run. ngx_destroy_pool(sl->pool); } + sl->pool = sl->new_pool; - sl->new_pool = NULL; + + if (sl->new_pool != NULL) { + ngx_destroy_pool(sl->new_pool); + sl->new_pool = NULL; + } exit: if (sc->serverlists_curr + 1 >= sc->serverlists_end) { From c879e2b6b78d14a962989c34434d638bc34a4466 Mon Sep 17 00:00:00 2001 From: cinquemb <1519017+cinquemb@users.noreply.github.com> Date: Fri, 2 Oct 2020 04:03:47 -0400 Subject: [PATCH 43/43] trying to create new ctx in refresh_upstream using new pool --- ngx_http_upstream_serverlist.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ngx_http_upstream_serverlist.c b/ngx_http_upstream_serverlist.c index 9c87dd1..9337c61 100644 --- a/ngx_http_upstream_serverlist.c +++ b/ngx_http_upstream_serverlist.c @@ -1076,10 +1076,16 @@ refresh_upstream(serverlist *sl, ngx_str_t *body, ngx_log_t *log) { // create new temp main_conf with a new pools, new service_conns and new serverlists, copy info from existing conf except for the pools, service_conns and serverlists cf.pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, log); - cf.ctx = mcf->conf_ctx; + + ngx_http_conf_ctx_t *ctx = NULL; + ctx = ngx_pcalloc(cf.pool, sizeof(ngx_http_conf_ctx_t)); + if (ctx == NULL) { + return -1; + } + ctx = mcf->conf_ctx; + cf.ctx = ctx; main_conf *tmp_mcf = create_main_conf(&cf); - tmp_mcf->conf_ctx = mcf->conf_ctx; //copy over previous count tmp_mcf->conf_pool_count = mcf->conf_pool_count;