| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Hi @hyperxpro could you check this PR. Thanks in advance |
Sorry, something went wrong.
|
Please rebase this PR with main - the commit history is messed up. |
Sorry, something went wrong.
…ermit-starved In LoadBalance.ROUND_ROBIN mode each request is pinned to a resolved IP via RoundRobinPartitionKey(base, IP), but the connection permit is per host (maxConnectionsPerHost). With a finite cap, a request pinned to IP_B that could not acquire a permit polled only its own per-IP key in the HTTP/2 registry and never found a sibling connection already open on IP_A: off the event loop it spun the full connectTimeout and then failed with the permit exception. Evolve the HTTP/2 registry (ChannelManager.http2Connections) from a flat exact-key map into a base-keyed nested map (base host -> per-IP key -> channel), so a permit-starved request can find any active, non-draining sibling-IP connection for the same host and multiplex onto it instead of stalling and failing. Multiplexing onto an existing HTTP/2 connection takes no permit, so this is safe; the fallback is confined to the permit-starved waitForHttp2Connection path, leaving the happy path strictly per IP so round-robin load keeps spreading. - RoundRobinPartitionKey: expose getBaseKey(). - ChannelManager: nested registry grouped by base key; register inside compute() and remove inside computeIfPresent() prune on the same outer bucket (closes the orphan race); add pollHttp2SiblingConnection(baseKey). - NettyRequestSender: in the permit-starved path, fall back to a sibling-IP connection when the exact per-IP poll misses (round-robin only). - Update LoadBalance.ROUND_ROBIN and waitForHttp2Connection docs. - Tests: new ChannelManagerHttp2SiblingLookupTest, RoundRobinPartitionKeyTest getBaseKey coverage, two BasicHttp2Test integration tests (permit-starved sibling reuse + spreading regression guard); flatten the reflective http2Connections() helpers in the two HTTP/2 regression tests for the nested map.
In LoadBalance.ROUND_ROBIN mode the per-host connection permit was released immediately after ALPN (as in DEFAULT mode), so it only throttled connection establishment, not the steady-state count. Because each request pins to a different IP and opens its own connection, a host could accumulate up to one connection per resolved IP regardless of maxConnectionsPerHost -- the cap was effectively not enforced. Hold the per-host permit for the HTTP/2 connection's lifetime in round-robin mode (as HTTP/1.1 already does) instead of releasing it post-ALPN. The per-host semaphore then bounds the number of live connections: at most maxConnectionsPerHost (K) connections are opened, spread across K of the resolved IPs, and a request pinned to a further IP fails to acquire a permit and multiplexes onto a sibling-IP connection (AsyncHttpClient#2214) rather than opening another. DEFAULT mode is unchanged: one connection per host, released post-ALPN so concurrent requests multiplex onto it instead of self-starving. This builds on the sibling-IP reuse landed for AsyncHttpClient#2214 -- without it a capped request would stall connectTimeout and then fail instead of multiplexing. - NettyConnectListener: registerHttp2AndManageSemaphore holds the permit for the connection lifetime when the partition key is a RoundRobinPartitionKey, else releases it immediately (both the TLS and h2c paths). - LoadBalance.ROUND_ROBIN: document the enforced cap and that the K connections are held open (kept alive by PING), so they stay pinned to K IPs; set the cap at least as large as the resolved-IP count for full per-IP spreading. - BasicHttp2Test: assert maxConnectionsPerHost caps live connections (3 IPs, cap=2 -> at most 2 connections) while all requests still succeed. Note: a live round-robin HTTP/2 connection now also occupies a global maxConnections slot for its lifetime.
…ng (#2248) Motivation: #2217 changed round-robin mode to hold the per-host connection permit for the lifetime of an HTTP/2 connection, ensuring `maxConnectionsPerHost` limits live connections. However, after a GOAWAY, the connection is immediately removed from the registry and marked as draining, while its permit is only released when the channel finally closes after all in-flight streams complete. During this drain window, the connection can no longer serve requests but still occupies both its per-host permit (and, with a combined limiter, a global connection permit). As a result, new requests cannot reuse the draining connection or acquire a new permit, eventually timing out with `TooManyConnectionsPerHostException`. With `maxConnectionsPerHost=1`, even same-host redirects fail, and under a combined limiter a draining connection can block new connections to unrelated hosts. `DEFAULT` mode does not have this issue because it releases the permit immediately after ALPN negotiation. Modification: Release the connection permit when draining begins instead of waiting for channel close. `Http2ConnectionState` now exposes a once-only permit release hook (`setPermitRelease`/`releasePermitOnce`) backed by an `AtomicReference` latch. In round-robin mode, `NettyConnectListener` installs this hook and also registers a `closeFuture` listener that invokes it. The GOAWAY handler in `ChannelManager` invokes the same hook after marking the connection as draining and removing it from the registry. Whichever path executes first releases the permit exactly once, preventing double releases from over-incrementing the semaphore. `DEFAULT` mode remains unchanged. The change also avoids repeatedly reading `future.getPartitionKey()` in `registerHttp2AndManageSemaphore` and documents the connection-lifetime permit semantics in `LoadBalance`. Result: A GOAWAY now immediately frees the draining connection's per-host and global connection permits, allowing a replacement connection to be established without waiting for existing streams to finish. This restores the expected behavior during server rolling restarts while preserving the connection cap. The change is covered by unit tests for drain-time permit release, double-release prevention, close-without-GOAWAY, combined-limiter behavior, concurrent once-only release, and an end-to-end HTTP/2 GOAWAY scenario that previously failed with `TooManyConnectionsPerHostException`.
| Back | FazBrowse Home | New Git URL |
Fixes #2214.
Two HTTP/2 robustness fixes for LoadBalance.ROUND_ROBIN, on top of the feature
branch (requests-round-robin-per-host):
stalling for connectTimeout and then failing (Round-robin: reuse a sibling-IP HTTP/2 connection when the per-IP connection is permit-starved #2214).
in round-robin mode (it previously only throttled connection establishment).
Two commits:
1. Reuse a sibling-IP HTTP/2 connection when permit-starved (#2214)
In round-robin mode each request is pinned to a resolved IP via
RoundRobinPartitionKey(base, IP), but the connection permit is per host. With a
finite maxConnectionsPerHost, a request pinned to IP_B that couldn't acquire a
permit polled only its own per-IP key in the HTTP/2 registry, never found a
connection already open on IP_A, and — off the event loop — spun the full
connectTimeout before failing with the permit exception.
exact-key map into a base-keyed nested map (base host → per-IP key → channel): single source of truth, O(1) exact lookup, sibling lookup scoped to
the host. Register runs inside compute(baseKey, …) and removal inside
computeIfPresent(baseKey, …) on the same outer bucket, so a concurrent
insert/prune can't orphan an entry.
pollHttp2SiblingConnection(baseKey) — any active, non-draining connection to
the same host on a sibling IP — and multiplexes onto it. Multiplexing onto an
existing HTTP/2 connection takes no permit, so this is safe. The fallback is
confined to the permit-starved path; the happy path stays strictly per IP, so
round-robin spreading is unaffected.
2. Enforce maxConnectionsPerHost for round-robin HTTP/2
The per-host permit was released immediately after ALPN (as in DEFAULT mode), so
it only throttled connection establishment. Because each request pins to a
different IP and opens its own connection, a host could accumulate up to one
connection per resolved IP regardless of the cap — it wasn't enforced.
(as HTTP/1.1 already does). The per-host semaphore then bounds live connections:
at most maxConnectionsPerHost (K) connections, spread across K of the IPs; a
request pinned to a further IP fails to acquire a permit and multiplexes onto a
sibling (via the fix above) rather than opening another. DEFAULT mode is
unchanged (one connection per host, released post-ALPN so concurrent requests
multiplex onto it).
Behavior & compatibility
mode. When it's < #resolved IPs, the cap wins over full spreading: K
connections are held open (kept alive by PING) and stay pinned to K IPs. Set it
≥ #IPs for full per-IP spreading. The default is unlimited, so this only
affects clients that configure a finite per-host cap.
slot for its lifetime.
Testing
active hit, draining/dead/closed skipped, base mismatch ignored, empty-inner-map
pruning.
multi-IP) — completes by multiplexing instead of stalling;
RoundRobinSendTypeTest pass; the two reflective http2Connections() test
helpers were updated to flatten the nested map.
Both fixes were adversarially verified: reverting the change makes the sibling
reuse test stall-then-fail, and the cap test report expected <= 2, got 3.
Out of scope
layer's responsibility (round-robin's IP cooldown is only a short-lived dampener).