iloveeyjafjalla opened a new pull request, #11359:
URL: https://github.com/apache/rocketmq/pull/11359
### Which Issue(s) This PR Fixes
- Fixes #11358
### Brief Description
In LOCAL mode (`-pm local`, proxy embedded in the broker process) the Proxy
Admin RPCs that relay to a client
never observe the client's answer:
`ProxyAdminGrpcService#relayConsumerRunningInfo` /
`relayConsumeMessageDirectly` wait on the future carried by the
`ProxyRelayRequest` they write into the client's
channel, and `ClusterProxyRelayService#bridgeToCaller` hands the relay
outcome back to that caller. The local
implementation does not — it only synthesizes a remoting response into the
broker's response table, which is
what the broker's mqadmin path needs, but for a `ProxyRelayRequest` there is
no remoting waiter, so the
in-process caller's future is never completed.
Effect in LOCAL mode: `GetConsumerRunningInfo`, `PrintThreadStackTrace` and
`VerifyMessage` stall for the full
relay timeout (`grpcProxyRelayRequestTimeoutInSeconds + 1`, 6 s by default)
and then fail; the
`GetConsumerRunningInfo` fallback additionally reports a wrong cause
("client ... is not connected to this
cluster" for a connected remoting client).
The change:
- bridges the relay future to the caller future in `LocalProxyRelayService`,
keeping the existing
remoting-response synthesis for plain `RemotingCommand`s (the mqadmin path
is unchanged);
- moves the bridging helper from `ClusterProxyRelayService` into
`AbstractProxyRelayService` so both modes share
one implementation instead of two copies.
### How Did You Test This Change?
Added to `LocalProxyRelayServiceTest`:
- `testProcessGetConsumerRunningInfoBridgesInProcessCaller` /
`testProcessConsumeMessageDirectlyBridgesInProcessCaller`:
build a `ProxyRelayRequest` with a caller future, complete the relay
future with the client's answer, assert
the caller future observes that exact result.
-
`testProcessGetConsumerRunningInfoPropagatesRelayFailureToInProcessCaller`: an
exceptional relay completion
reaches the caller future as well.
The two pre-existing tests (`testProcessGetConsumerRunningInfo`,
`testProcessConsumeMessageDirectly`, plain
`RemotingCommand` + broker remoting server) still pass, i.e. the mqadmin
path still gets its synthesized
response.
Results on Temurin JDK 8 (same JDK as CI), base `9ea2ccdc2` plus this patch:
- before the fix: `testProcessGetConsumerRunningInfoBridgesInProcessCaller`
and
`testProcessConsumeMessageDirectlyBridgesInProcessCaller` fail (`Expecting
value to be true but was false` —
the caller future stays incomplete)
- after the fix: `LocalProxyRelayServiceTest — Tests run: 5, Failures: 0,
Errors: 0, Skipped: 0`
- related classes re-run: `GrpcClientChannelTest` and
`ProxyAdminGrpcServiceTest` pass.
`ProxyChannelTest#testWriteAndFlush` fails in this environment both with
and without the patch (it also fails
on the base commit with these changes stashed), so that failure is
pre-existing and unrelated to this change.
This change was prepared with AI assistance (analysis, patch and test
drafting); the diff and every result above
were reproduced locally by running the module's tests against the base
commit and the patched tree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
