[oowl]

Related PRs

PR	What it does
openresty/lua-nginx-module#2536	C side: pool_size in the balancer keepalive FFI API, per-pool descriptors
#544	Lua side: balancer.enable_keepalive(idle_timeout?, max_requests?, pool_size?)
openresty/test-nginx#190	Fixes the ASan CI hang in HUP tests on Ubuntu 22.04. Independent of the feature. Merged
openresty/lua-nginx-module#2540	Bumps ngx_http_lua_version to 10033. Closed, not merged
#547	Requires ngx_lua 10033, bumps lua-resty-core to 0.1.36. Closed, not merged

Integration status:

• bugfix: send the HUP mode signals from perl instead of running kill(1). test-nginx#190 is merged.
• Both feature PRs now use upstream dependencies in CI; their temporary CI commits have been removed.
• feature: added the pool_size argument to the balancer keepalive FFI API. lua-nginx-module#2536 and feature: ngx.balancer: added the pool_size argument to enable_keepalive(). #544 still need to be integrated together because the FFI signature changes. Until #2536 lands on master, feature: ngx.balancer: added the pool_size argument to enable_keepalive(). #544's CI uses the old C signature and is expected to fail.
• The separate version-bump PRs change: bumped ngx_http_lua_version to 10033. lua-nginx-module#2540 and change: require ngx_http_lua_module 0.10.33 and bumped version to 0.1.36. #547 are closed without merging.

---

Summary

Adds an optional third argument pool_size to ngx.balancer.enable_keepalive():

balancer.enable_keepalive(idle_timeout?, max_requests?, pool_size?)

pool_size sets the maximum number of idle connections cached for the pool of the current connection. A pool is identified by the peer address, the host argument of set_current_peer() and the local address. When a pool is full, its own least recently used idle connection is closed. Without this, a busy pool evicts the idle connections of other pools from the upstream-wide balancer_keepalive cache. Omitting the argument or passing 0 keeps the current behaviour. pool_size never raises the balancer_keepalive total.

Depends on openresty/lua-nginx-module#2536, which changes the FFI signature of ngx_http_lua_ffi_balancer_enable_keepalive(). The two PRs should be merged together.

Docs

• enable_keepalive used to say the connection goes into "the pool specified by the pool option of set_current_peer". That option does not exist (it is a leftover of the original design). The text now describes the actual pool key: peer address, host (or proxy_ssl_name for HTTPS when host is nil), and local address. It also explains that all pools share the balancer_keepalive capacity.
• pool_size: the limit is checked against the value passed by the request whose connection is being inserted, so every request using the same pool should pass the same pool_size.

Tests

New in t/balancer-keepalive.t:

• TEST 29: argument validation for pool_size
• TEST 30: pool_size = 1 makes a pool evict its own oldest idle connection
• TEST 31: with balancer_keepalive 2, a full pool does not evict another pool's idle connection. This test fails if pool_size is 0.
• TEST 32: explicit pool_size = 0 behaves exactly like omitting it, i.e. the global LRU evicts the other pool's connection.
• TEST 33: pool_size also applies to pools split by SNI (host nil, name from proxy_ssl_name), and each request still gets its own SNI.

New in t/balancer-keepalive-localaddr.t:

• TEST 8: pool_size applies per local address (bind_to_local_addr). A full pool for one source address does not evict the other source address's connection.

TEST 33 fails if the C side ignores the host in the pool key, and localaddr TEST 8 fails if it ignores the local address (checked with deliberately broken builds).

Ran locally against nginx 1.31.8 + the lua-nginx-module branch, both as a normal --with-debug build and as an ASan build: t/balancer*.t and t/stream/balancer*.t all pass with no ASan reports. t/balancer-keepalive.t passed 20 runs in a row. Line length and tab checks pass.

CI

The temporary CI commit has been removed. CI now clones upstream openresty/lua-nginx-module master again. The feature and tests are unchanged.

The C-side dependency openresty/lua-nginx-module#2536 is still open. Until it lands on master, the upstream C function has the old FFI signature, so this CI configuration is expected to fail. Previous successful CI runs used the matching #2536 branch.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.