unbridled-41 opened a new pull request, #4750:
URL: https://github.com/apache/rocketmq-dashboard/pull/4750
Fixes #4749.
## Problem
A consumer group that has never connected is reported as "connection
inventory unavailable" in the group list (`onlineInstances = -1`) while its own
detail view reports it as offline (`0`). Two answers for the same group.
- List path: `RocketMQMetadataProvider.resolveConsumerConnection`
(`provider/apache/RocketMQMetadataProvider.java:382-397`) classifies failures
with `isGroupNotOnline` (`:746-770`) and then sets
`vo.setOnlineInstances(connection.available() ? instances.size() : -1)`
(`:335`).
- `isGroupNotOnline` handles `CONSUMER_NOT_ONLINE` and
`BROADCAST_CONSUMPTION`, but not `TOPIC_NOT_EXIST` — the case the same file
documents at `:777-783`: rocketmq-tools locates a group through the
`%RETRY%<group>` topic route *before* any broker call, so a group whose retry
topic does not exist yet (never connected) fails with CODE 17. The progress and
subscription paths pre-create that topic for this reason
(`ensureRetryTopicExists`, `:638`/`:691`); the list path does not, per group.
- Detail path: `RocketMQAdminClientImpl.isConsumerNotOnline` (`:244-259`)
does grade it — `MqResponseCodes.hasResponseCode(exception,
ResponseCode.TOPIC_NOT_EXIST) && message.contains("%RETRY%")` (`:254-256`) —
and leaves `onlineInstances` at its `0` default.
The wrong `-1` is user-visible beyond the column:
`web/src/utils/consumerConnections.ts:28-35` renders the unknown label, and
`web/src/utils/consumerGroupDiagnostics.ts:302-320` turns it into a
`CONNECTION_STATUS_UNKNOWN` warning whose `else if` position masks the critical
`NO_ACTIVE_CLIENTS_WITH_LAG` finding for the same group.
## Root cause
The three-state connection resolution added in #4544 was taught about two of
the three "no live data" states; the CODE 17 retry-topic case described in the
same commit's comments was not extended to the list grading predicate.
## Fix
`isGroupNotOnline` now grades `TOPIC_NOT_EXIST` on a retry topic as "no live
data" too, mirroring `RocketMQAdminClientImpl.isConsumerNotOnline` (7 lines).
The three call sites all move in the intended direction: the list reports
offline (0), the progress read reports no stats instead of a 502, and the
subscription read tries the proxy before reporting an empty list — all states
the same file already treats as "not a connectivity failure" for the other
graded codes.
## Tests
Red before the fix:
```
$ cd server && mvn -B -ntp -o test -Dtest=RocketMQMetadataProviderTest
[ERROR] ...listConsumerGroupsShouldGradeAMissingRetryTopicRouteAsOfflineTest
<<< FAILURE!
but was: -1
[ERROR] Tests run: 45, Failures: 1, Errors: 0, Skipped: 0 # BUILD
FAILURE
```
After the fix:
```
$ mvn -B -ntp -o test
-Dtest=RocketMQMetadataProviderTest,RocketMQAdminClientImplTest,ProxyConsumerResolverTest
[INFO] Tests run: 45, Failures: 0, Errors: 0, Skipped: 0 --
RocketMQMetadataProviderTest
[INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0 --
ProxyConsumerResolverTest
[INFO] Tests run: 69, Failures: 0, Errors: 0, Skipped: 0 --
RocketMQAdminClientImplTest
[INFO] Tests run: 120, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
The new test uses a `MQClientException(ResponseCode.TOPIC_NOT_EXIST, "…
%RETRY%cg-brand-new")` and asserts `onlineInstances == 0` with an empty
instance list; the sibling test that pins the genuine "lookup failed" state
(`IllegalStateException`) still expects `-1` and still passes, so the two
states stay distinguishable.
## Duplicate check
Searched open PRs (205) and issues (214) for `onlineInstances`, `consumer
connection`, `retry topic`, `TOPIC_NOT_EXIST`, `unknown online`, plus the
commit index: nothing covers this grading. The nearest items are #4006
(`RocketMQClientProvider` lists connections as 502 when every group is offline
— different endpoint, error instead of a wrong value) and #4463 (producer
scans), plus the merged #4544 that introduced the three-state resolution.
## Base branch
Targets the development trunk `rocketmq-studio`. GitHub interprets closing
keywords only for pull requests that target the repository default branch, and
that is still `master`, so merging will not close #4749 automatically — it
needs to be closed by hand.
## Risk
Low, and in the direction of the existing design: the change only re-grades
one exception class that the detail path already grades this way. The
counter-state test (unexpected failure keeps `-1`) is untouched and green.
--
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]
