Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: get ngx_array_t element number error #198

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix: get ngx_array_t element number error #198

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3642c1
Fix: get ngx_array_t element number error
Jan 23, 2018
0c2c67e
Use nginx memory pool for server peers alloc/free
Jan 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 25 additions & 15 deletions src/ngx_http_upsync_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ typedef struct {

typedef struct {
ngx_uint_t upstream_num;

ngx_http_upsync_server_t *upsync_server;

ngx_pool_t *pool; // worker common pool, can't be destroyed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

更换为pool 的优点呢？有没有一种极端的情况，内存一直被吞噬（在增删server 比较频繁的场景下），pool 内存释放是以块为单位？

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

删除向pool释放的内存，还能被使用，减少向系统申请、释放内存数。
没打算提交pool的修改，因为通过首次upstream有增删控制，去replace_peers已经很好了。
主要是修复bug：
image

我把这个删了，再提交一个PR。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

欢迎任何可能的修改哈，探讨选取最优的；还有很多待优化的地方 :-);
@seven-eleven 谢谢

} ngx_http_upsync_main_conf_t;


Expand Down Expand Up @@ -835,20 +836,18 @@ ngx_http_upsync_add_peers(ngx_cycle_t *cycle,
// FIXME: until backup is fully implemented this causes crashes
// on startup with nodes set backup=1. Let them in for now

peer = ngx_calloc(sizeof(ngx_http_upstream_rr_peer_t),
cycle->log);
peer = ngx_pcalloc(upsync_ctx->pool, sizeof(ngx_http_upstream_rr_peer_t));
if (peer == NULL) {
goto invalid;
}

if ((saddr = ngx_calloc(len, cycle->log)) == NULL) {
if ((saddr = ngx_pcalloc(upsync_ctx->pool, len)) == NULL) {
goto invalid;
}
ngx_memcpy(saddr, server->addrs->sockaddr, len);
peer->sockaddr = saddr;

if ((namep = ngx_calloc(server->addrs->name.len,
cycle->log)) == NULL) {
if ((namep = ngx_pcalloc(upsync_ctx->pool, server->addrs->name.len)) == NULL) {
goto invalid;
}
ngx_memcpy(namep, server->addrs->name.data,
Expand Down Expand Up @@ -1009,7 +1008,7 @@ ngx_http_upsync_diff_filter(ngx_cycle_t *cycle,
return;
}

ngx_memzero(flag_array.elts, sizeof(ngx_uint_t) * flag_array.size);
ngx_memzero(flag_array.elts, sizeof(ngx_uint_t) * flag_array.nalloc);
flags = (ngx_uint_t*)flag_array.elts;
}

Expand Down Expand Up @@ -1189,13 +1188,12 @@ ngx_http_upsync_replace_peers(ngx_cycle_t *cycle,
if (peers) {

for (i = 0; i < peers->number; i++) {
peer = ngx_calloc(sizeof(ngx_http_upstream_rr_peer_t),
cycle->log);
peer = ngx_pcalloc(upsync_ctx->pool, sizeof(ngx_http_upstream_rr_peer_t));
if (peer == NULL) {
goto invalid;
}

if ((saddr = ngx_calloc(len, cycle->log)) == NULL) {
if ((saddr = ngx_pcalloc(upsync_ctx->pool, len)) == NULL) {
goto invalid;
}
ngx_memcpy(saddr, tmp_peer[i].sockaddr, len);
Expand All @@ -1205,8 +1203,7 @@ ngx_http_upsync_replace_peers(ngx_cycle_t *cycle,
peer->name.len = tmp_peer[i].name.len;
peer->server.len = tmp_peer[i].name.len;

if ((namep = ngx_calloc(tmp_peer[i].name.len,
cycle->log)) == NULL) {
if ((namep = ngx_pcalloc(upsync_ctx->pool, tmp_peer[i].name.len)) == NULL) {
goto invalid;
}
ngx_memcpy(namep, tmp_peer[i].name.data, tmp_peer[i].name.len);
Expand Down Expand Up @@ -2182,6 +2179,7 @@ ngx_http_upsync_create_main_conf(ngx_conf_t *cf)

upmcf->upstream_num = NGX_CONF_UNSET_UINT;
upmcf->upsync_server = NGX_CONF_UNSET_PTR;
upmcf->pool = NULL;

return upmcf;
}
Expand Down Expand Up @@ -2451,6 +2449,18 @@ ngx_http_upsync_init_process(ngx_cycle_t *cycle)
if (upsync_ctx == NULL) {
return NGX_OK;
}

if (upsync_ctx->pool == NULL) {
pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, ngx_cycle->log);
if (pool == NULL) {
ngx_log_error(NGX_LOG_ERR, cycle->log, 0,
"upsync_init_process: create main conf pool error, "
"server no enough memory");
return NGX_ERROR;
}
upsync_ctx->pool = pool;
}

upsync_server = upsync_ctx->upsync_server;

for (i = 0; i < upsync_ctx->upstream_num; i++) {
Expand Down Expand Up @@ -3285,20 +3295,20 @@ ngx_http_upsync_del_delay_delete(ngx_event_t *event)
while (peer != NULL) {
saddr = peer->sockaddr;
if (saddr != NULL) {
ngx_free(saddr);
ngx_pfree(upsync_ctx->pool, saddr);
saddr = NULL;
}

namep = peer->name.data;
if (namep != NULL) {
ngx_free(namep);
ngx_pfree(upsync_ctx->pool, namep);
namep = NULL;
}

pre_peer = peer;
peer = peer->next;

ngx_free(pre_peer);
ngx_pfree(upsync_ctx->pool, pre_peer);
}

ngx_queue_remove(&delay_event->queue);
Expand Down