[AxisNow-Dev]

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

This is the stream counterpart of openresty/lua-nginx-module#2544. It now follows the design #2544 settled on: the direct handoff from b965a88 there, with the fixes that came after it.

With a connection pool backlog, setkeepalive() posts the resumption of the next queued operation so that it takes the idle connection. A connect() made before that event runs calls ngx_stream_lua_get_keepalive_peer() first and takes the connection. The resumed operation then finds no idle connection. Resumed operations skip the backlog check in ngx_stream_lua_socket_tcp_connect_helper(), so it opens a new one, and the pool can grow up to pool_size + backlog. Since #415 every freed slot posts its own operation, which makes this much more frequent.

Final design

A pool keeps two counts:

• connections is the slots handed out: active sockets, cached idle connections and posted connect operations.
• waiting is the operations still queued in wait_connect_op, which hold no slot.

The backlog check is waiting >= backlog. Posted operations move to their own posted_connect_op queue, which replaces the next_connect_op cursor.

When setkeepalive() returns a connection while someone waits, the connection does not go to cache. It is attached to the head waiter (op->item, with item->owner pointing back), and the waiter takes it with ngx_stream_lua_socket_tcp_reuse_peer() when its resumption runs. New connects look at cache only, so they cannot take it.

Every place that frees a slot goes through ngx_stream_lua_socket_tcp_release_slot(), whether or not a connection comes with it. If someone waits, it resumes the head waiter with the slot and the connection, if any. Otherwise it puts the connection in cache, or decrements connections when there is no connection.

A connection closed while it waits for its owner only clears the owner's item; the owner then connects in its slot. An operation resumed with an empty slot first takes the head of cache when it runs, and gives its spare slot back if it gets one.

Commits

1. 0dabcb0 bugfix: new connects took the idle connection handed to a queued connect, which then went past the pool size. The first approach: a new connect() skipped every idle connection while any operation waited. Replaced by the second commit.
2. 89ecad5 bugfix: hold idle connections for resumed connects in a reserved queue. Moved the connection handed to a resumed operation to a reserved queue, out of reach of new connects. Replaced by d701ffa.
3. 36e8635 bugfix: cancelling a queued connect resumed another one beyond the pool size. Cancelling an operation still queued, which holds no slot, resumed the next waiter. The rule stays in the final design: cancelling a queued operation only decrements waiting.
4. aa0ba51 bugfix: a resumed connect cancelled after its reserved idle connection closed left the freed slot to nobody. A gap in the reserved queue. The final design covers this case through release_slot() without a connection, and test 64 stays.
5. d701ffa change: hand cosocket connections directly to waiting operations. The final design above, ported from b965a88, fbbcf41 and 3e21572 in #2544. It also brings these changes:
   • A queued or posted socket stays busy connecting, and its pool key is written only once the busy checks pass.
   • A failed connect closes a partly set up peer and releases its slot once.
   • A connection made before the pool existed only replaces an idle connection in a full pool.
   • A posted operation cancelled before it runs releases its slot.
6. b284c76 tests: cover the cosocket connect queue handoff in the stream module. Tests 65 to 88 in t/068-socket-keepalive.t, and a new t/196-socket-keepalive-queue.t, ported from #2544. Two kinds of http case needed changes:
   • The cases that end a subrequest before its connect runs use a timer that exits instead (tests 74 and 85).
   • The internal redirect cases have no stream counterpart. Test 81 instead resumes a waiter in the preread phase. t/196 runs its handoff, cancellation and timeout checks in the preread and content phases, and keeps the fixed-seed stress case.
7. f8bc411 bugfix: a connect resumed with an empty slot opened a new connection next to an idle one. 12e9595 from #2544, with test 89. As there, it can be dropped on its own.

Tests

• On my machine, tests 65 to 89 passed five runs out of five over HTTP/1.1 and over HTTP/2, which only changes the request that drives the stream server. They also passed under valgrind, with no errors from this module. t/196 passes, including a run with TEST_NGINX_SOCKET_POOL_ROUNDS=60.
• Tests 82 to 89 cover the same cases as tests 85 to 92 in #2544. Each fails when the stream branch it covers is broken by a one-line change; I checked twelve such changes. I did not run this check on tests 65 to 81.
• I also ran the full t/068 with memcached, against this branch and against aa0ba51, the reserved queue. Tests 1 to 64 fail the same way with both, for reasons in my local setup: tests 3 to 9, 11, 12, 25, 33, 45, 53 and 54. On aa0ba51, 13 of the new tests fail: 65 to 69, 71, 72, 75 to 77, 86, 87 and 89.
• I tested against nginx 1.31.8, in an OpenResty build with this module at current master.

Contributed by AxisNow.