[hanzhe-one]

Fixes #8004

Problem

#7986 made LeastActiveLoadBalance read Upstream#getInflight() instead of a private per-domain counter. Only DividePlugin (and the failover attempts in DefaultRetryStrategy) maintain that value, so on the other paths that call LoadbalancerUtils every upstream sits at the same baseline and leastActive degenerates to weighted random — a regression against the previous behaviour, where selections were at least accumulated.

The affected call sites named in the issue are the websocket plugin and apache dubbo gray routing.

Changes

shenyu-plugin-websocket

• WebSocketPlugin reserves the chosen upstream for leastActive when the proxied session is subscribed and releases it in doFinally, mirroring DividePlugin. The reservation is inside Mono.defer, so a session that is built but never subscribed cannot leak the count.

shenyu-plugin-apache-dubbo

• New InflightInvoker, a delegating Invoker that increases the upstream's count when the RPC starts and releases it exactly once: on result completion, or when the invocation throws before producing a result.
• ApacheDubboGrayLoadBalance wraps the selected invoker in it for leastActive only. Other algorithms keep the plain invoker.

shenyu-loadbalancer / docs

• The javadoc on LeastActiveLoadBalance and ShortestResponseLoadBalancer, and a RELEASE-NOTES entry, state which paths maintain the count and which do not.

Two decisions worth reviewing

The reservation is on invoke(), not on selection. Dubbo's cluster can select an invoker and then discard the pick (already tried, unavailable, or replaced by a reselect). A selection-time reservation would leak those counts permanently. invoke() is the only place that observes both the start and the end of the RPC.

The wrapper delegates equals and hashCode. AbstractClusterInvoker.doSelect keeps the returned invoker in its already-tried list, reselect and FailoverClusterInvoker dedupe with List.contains, sticky routing checks invokers.contains(stickyInvoker), and addInvalidateInvoker removes the selected invoker from validInvokers via BitList.remove. All of these compare with equals, and no Dubbo invoker overrides it, so they are identity comparisons. A wrapper that compared by its own identity would make failover retry a provider it already tried, silently disable sticky routing and stop unhealthy providers from being invalidated. Delegating equality keeps the wrapper transparent to all of them.

Not covered here

• shortestResponse remains weight-based outside the divide plugin. It needs the average elapsed time of successful requests as well as the outstanding count, and only DividePlugin.successResponseTrigger records that. Reserving inflight alone would not change the outcome, and connection lifetime is a poor proxy for WebSocket latency. This is stated in the javadoc and RELEASE-NOTES rather than silently left.
• The grpc picker, the tcp proxy and the java sdk build fresh Upstream objects on every call (convertUpstreamList), so a count kept on them could never be shared across requests. They are documented as weight-based for both algorithms.
• No integration or e2e coverage was added; both regressions are expressed in unit tests.

Validation

• ./mvnw -pl shenyu-loadbalancer,shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide test — 70 and 102 tests, 0 failures.
• ./mvnw -pl shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-websocket test — 18 tests, 0 failures.
• ./mvnw -pl shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-rpc/shenyu-plugin-dubbo/shenyu-plugin-apache-dubbo test — 57 tests, 0 failures.
• ./mvnw -pl shenyu-plugin/shenyu-plugin-httpclient -Dtest=RetryStrategyTest test — 25 tests, 0 failures.
• Checkstyle and RAT pass on every module touched.

New tests, each verified against the pre-fix source:

• WebSocketPluginTest#leastActiveShouldCountTheProxiedSession — fails without the fix (expected: <2> but was: <1>).
• WebSocketPluginTest#leastActiveShouldReleaseOnCompletion and #otherAlgorithmsShouldNotCountTheProxiedSession.
• ApacheDubboGrayLoadBalanceTest#leastActiveShouldCountTheRpcWhileItRuns — fails without the fix (expected: <2> but was: <1>); asserts the count is held while the future is incomplete and released on completion.
• ApacheDubboGrayLoadBalanceTest#countingWrapperShouldKeepTheIdentityOfTheWrappedInvoker — fails without the fix, since the unwrapped invoker is returned.
• ApacheDubboGrayLoadBalanceTest#leastActiveShouldReleaseWhenTheRpcThrows.
• ApacheDubboGrayLoadBalanceTest#otherAlgorithmsShouldReturnTheSelectedInvokerUnchanged.

[Aias00]

Both regressions are real and the fix is the right shape.

Verification I ran locally

• Reverting the two changed main sources and dropping InflightInvoker leaves exactly the three discriminating cases red: WebSocketPluginTest.leastActiveShouldCountTheProxiedSession, ApacheDubboGrayLoadBalanceTest.countingWrapperShouldKeepTheIdentityOfTheWrappedInvoker and #leastActiveShouldCountTheRpcWhileItRuns. With the patch: websocket 18/18, apache-dubbo 57/57, checkstyle:check clean on both modules.
• The gate matches the selection in both paths: WebSocketPlugin.doExecute picks upstream with ruleHandle.getLoadBalance() and ApacheDubboGrayLoadBalance picks it with dubboRuleHandle.getLoadBalance(), so the counter is maintained exactly on the paths whose selection reads it. No other caller is silently affected.
• The Upstream being counted in the dubbo path is the one LoadbalancerUtils.getForNoExchange returned from UpstreamCacheManager, i.e. the same object LeastActiveLoadBalance reads — not a per-invocation copy. That is the correct pairing.
• Reserving on invoke() instead of on selection is the right call for the reasons you give, and AtomicBoolean makes the release exactly-once across all three exits (throw, null result, completion). The equals/hashCode delegation is genuinely load-bearing — invokers.contains(selected) and indexOf in the test pin the BitList.remove / sticky-routing behaviour.

Needs doing before merge
The branch is CONFLICTING against master; please rebase.

Nits

• WebSocketPlugin increments before doFinally is attached; if that call ever threw, the count would leak. Attaching first and incrementing after (or Mono.usingWhen) closes the window.
• In the dubbo path, when upstream was chosen by leastActive but invokerGrays comes back empty, the code falls back to dubboSelect(invokers, ...) with no reservation at all. Nothing leaks, but that one branch is an exception to "every request that went through leastActive selection is counted" — worth a comment so the next reader does not read it as an oversight.
• InflightInvoker has no toString, so retry/invalidation log lines will name the wrapper instead of the provider; getUrl() is delegated so this is cosmetic.

[Aias00]

The increment runs before doFinally is attached. It cannot throw today, but attaching the release first and incrementing afterwards (or using Mono.usingWhen) removes the ordering assumption entirely — as written, any throw between these two lines leaks a permanent count for that upstream.

[Aias00]

When upstream was picked by leastActive but the gray filter matches no invoker, this fallback selects from the full list and reserves nothing, so the count never moves even though the shenyu-side selection read it. Not a leak, but it is the one branch that breaks the "counted for the whole RPC" invariant the rest of the change establishes — a one-line comment saying the pick is discarded would keep it from reading as an oversight.

[Aias00]

The Dubbo side reads well: counting at invoke rather than at selection is the right choice, because a selection that the cluster later discards never gets released, and delegating equals/hashCode to the wrapped invoker is the detail that keeps Dubbo's already-tried and connectivity-invalidation sets working — that is the part most wrappers get wrong.

I have one question about the WebSocket half, and one nit on the Dubbo wrapper.

WebSocket: does the returned Mono really stay pending for the session lifetime?

The count is released in doFinally on the Mono returned by WebSocketService.handleRequest. Shenyu's bean is a plain HandshakeWebSocketService (WebSocketPluginConfiguration:94), and HandshakeWebSocketService.handleRequest delegates to requestUpgradeStrategy.upgrade(...), whose Mono completes when the upgrade handshake is done. The proxied session then runs inside the container, driven by ShenyuWebSocketHandler.handle(session)'s own subscription, not by the Mono the plugin holds. If that is so, a WebSocket that stays open for an hour is counted for a few milliseconds, and least-active still sees nothing from the WebSocket path.

The tests cannot settle this, because webSocketService is mocked to return Mono.never() — that pins the assumption rather than the real service. Can you confirm the lifetime, and if it is the handshake, move the decrement to where the session actually ends (e.g. ShenyuWebSocketHandler.handle returning session.receive().then() with a doFinally, or an explicit session.close() hook)? A note in the PR body on how this was verified against a real connection would be enough for me.

InflightInvoker.released is per-invoker, not per-invocation

Invoker is a reusable object by contract — Dubbo's own RpcStatus counts per invocation for exactly this reason. If the same wrapper instance is invoked twice, the second call increments and then hits released.compareAndSet(false, true) == false, so its count is never returned. In the current call path withInflight builds a fresh wrapper per select, so it does not happen today, but the guard would be more obviously correct created inside invoke and captured by the callback.

[Aias00]

HandshakeWebSocketService.handleRequest completes when the upgrade handshake finishes, not when the proxied session ends, so this doFinally may fire immediately after the handshake. The tests mock Mono.never(), which cannot distinguish the two. Please confirm against a real connection.

[hanzhe-one]

Thanks for the review — the WebSocket point was correct and it invalidated that half of the change. Pushed 66010e914.

WebSocket lifetime: you were right, the count was on the handshake.

I traced it in the bytecode rather than trusting the tests. HandshakeWebSocketService.handleRequest returns initAttributes(...).flatMap(...) over RequestUpgradeStrategy.upgrade, and reactor-netty's HttpServerOperations.withWebsocketSupport returns

Mono.from(handshakerResult).doOnEach(signal -> { ...; publisher.subscribe(new WebsocketSubscriber(ops, ctx)); })

so the Mono is over the handshake promise, and ShenyuWebSocketHandler.handle is subscribed separately inside doOnEach, never chained onto it. The returned Mono therefore completed at handshake time and the count was released milliseconds later. A session open for an hour was counted for a few milliseconds, exactly as you described.

The fix wraps the WebSocketHandler instead of the returned Mono: InflightWebSocketHandler.handle(session) increments on subscription and releases on that Mono's termination, which is when the session ends. Your Mono.never() observation was also right — the old tests pinned the assumption rather than the behaviour. The new tests take the handler out of the mocked service, let the service return Mono.empty() to stand for a completed handshake, and assert the count is still held afterwards; then they drive the handler and assert the increment and the release.

Verification against a real connection: I did not run one — this environment cannot open a local socket, which is why NettyHttpClientPluginTest fails here on unmodified code too. So the evidence is the bytecode path above plus a test that now fails if the count is attached to the handshake instead of the session (leastActiveShouldCountTheSessionAndNotTheHandshake fails on the previous commit with expected: <2> but was: <1>). If you would rather see a live-connection check before merge, say so and I will arrange one rather than claim it.

InflightInvoker.released is now per invocation. You are right that an Invoker is reusable, and the shared flag would have silently lost the second call's count. The guard is created inside invoke and captured by the completion callback, matching how RpcStatus counts per invocation. leastActiveShouldReleaseEveryInvocationOnAReusedWrapper covers it and fails on the previous commit.

Increment-before-doFinally ordering. Fixed by construction: the handler attaches the release first and increments after, and a handler that throws or returns nothing releases before propagating, so no ordering assumption is left.

Dubbo fallback branch. Added the comment you asked for — the gray pick is discarded because it matched no provider, so that upstream is not carrying the request.

toString. Delegated, so log lines name the provider.

The one open item is the real-connection verification above; everything else is addressed in 66010e914.

[Aias00]

Approved. Counting inflight for websocket sessions (InflightWebSocketHandler counts connected sessions, not handshakes) and dubbo gray-route invocations makes least-active balancing accurate for those transports. Wrapping the invoker in InflightInvoker (delegating equals/hashCode) avoids breaking Dubbo retry/cluster comparisons. RELEASE-NOTES updated. Good work.