feature: implemented keepalive pooling in 'balancer_by_lua*'. (#38) #1908

zhuizhuhaomeng · 2021-08-18T05:43:31Z

Co-authored-by: Thibault Charbonnier thibaultcha@me.com

I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.

doujiang24 · 2021-08-31T02:10:16Z

src/ngx_http_lua_balancer.c

+                    item->host.len = host->len;
+
+                } else {
+                    item->host.data = ngx_pstrdup(c->pool, bp->addr_text);


show check if the return value is NULL.

doujiang24 · 2021-08-31T02:14:16Z

src/ngx_http_lua_balancer.c

+    ngx_sockaddr_t                 sockaddr;
+    ngx_str_t                      host;
+    u_char                         hostbuf[32];
+} ngx_http_lua_balancer_ka_item_t; /*balancer keepalive item*/


maybe ngx_http_lua_balancer_keepalive_cache_t can be better?
to align up with ngx_http_upstream_keepalive_cache_t.

doujiang24 · 2021-08-31T06:13:19Z

src/ngx_http_lua_balancer.c

    ngx_http_lua_balancer_peer_data_t  *bp;

    if (r == NULL) {
-        *err = "no request found";
+        p = ngx_snprintf(errbuf, *errbuf_size, "no request found");


why do we need this change?

Co-authored-by: Thibault Charbonnier <thibaultcha@me.com>

…ry exception

src/ngx_http_lua_balancer.c

xiaocang · 2021-09-03T16:05:58Z

README.markdown

+can open. The connections parameter should be set to a number small enough to
+let upstream servers process new incoming connections as well.
+
+This directive was first introduced in the `v0.10.19.5` release.


typo here?
v0.10.19.5 -> v0.10.19

xiaocang · 2021-09-03T16:06:10Z

doc/HttpLuaModule.wiki

+can open. The connections parameter should be set to a number small enough to
+let upstream servers process new incoming connections as well.
+
+This directive was first introduced in the <code>v0.10.19.5</code> release.


xiaocang · 2021-09-03T16:10:16Z

src/ngx_http_lua_balancer.c

@@ -520,22 +860,91 @@ ngx_http_lua_ffi_balancer_set_current_peer(ngx_http_request_t *r,

    if (ngx_parse_url(r->pool, &url) != NGX_OK) {
        if (url.err) {
-            *err = url.err;
+            p = ngx_snprintf(errbuf, *errbuf_size, "%s", url.err);


errbuf is undeclared before

doujiang24 · 2021-09-04T01:22:55Z

src/ngx_http_lua_balancer.c

+                    item->host.len = host->len;
+
+                } else {
+                    item->host.len = bp->addr_text->len;


it's better to move this line after line 680. just in case something changed in the feature even it doesn't for now.

doujiang24 · 2021-09-04T02:19:37Z

src/ngx_http_lua_balancer.c

+    if (n == -1 && ngx_socket_errno == NGX_EAGAIN) {
+        ev->ready = 0;
+
+        if (ngx_handle_read_event(c->read, 0) == NGX_OK) {


Suggested change

if (ngx_handle_read_event(c->read, 0) == NGX_OK) {

if (ngx_handle_read_event(c->read, 0) != NGX_OK) {

doujiang24 · 2021-09-04T02:28:29Z

src/ngx_http_lua_balancer.c

+        if (ngx_memn2cmp((u_char *) &item->sockaddr,
+                         (u_char *) sockaddr,
+                         item->socklen, socklen) == 0
+            && name->len == item->host.len


maybe we should move these length compare expressions at the front?
I'm not sure if the C compiler is clever enough to do such optimizations.

doujiang24 · 2021-09-04T02:31:53Z

src/ngx_http_lua_balancer.c

+
+    /*
+     * ssl name here may contain port, notably if derived from $proxy_host
+     * or $http_host; we have to strip it


better add an example of the ssl name value here.

mergify · 2021-09-20T10:04:29Z