zjncs opened a new pull request, #11122:
URL: https://github.com/apache/rocketmq/pull/11122
## Motivation
Querying the topics of a cluster that the nameserver does not know (typo in
`mqadmin topicList -c`, or a cluster whose brokers all unregistered, which
drops the cluster key from `clusterAddrTable`) currently produces two problems:
1. `RouteInfoManager.getTopicsByCluster` dereferences the result of
`clusterAddrTable.get(cluster)` without a null check and NPEs on the for-loop;
the nested catch swallows it and every query logs a full stack trace.
2. `DefaultRequestProcessor.getTopicsByCluster` then encodes the empty
`TopicList` and answers `SUCCESS`, so the client cannot distinguish "cluster
has no topics" from "cluster does not exist".
## Modification
- `RouteInfoManager.getTopicsByCluster` returns `null` for an unknown
cluster, mirroring how `pickupTopicRouteData` signals a missing topic.
- `DefaultRequestProcessor.getTopicsByCluster` maps the null result to
`SYSTEM_ERROR` with a `cluster[...] not exist` remark, so clients (e.g.
`mqadmin topicList -c`) get a clear error instead of an empty list.
## Test Evidence
```
docker exec rmq-build mvn -q -pl namesrv test
-Dtest='RouteInfoManagerTest#testGetTopicsByClusterNotExisted'
-Dsurefire.failIfNoSpecifiedTests=true
docker exec rmq-build mvn -q -pl namesrv test
-Dtest='DefaultRequestProcessorGetTopicsByClusterTest'
-Dsurefire.failIfNoSpecifiedTests=true
```
Before the fix:
```
java.lang.NullPointerException: Cannot invoke "Set.iterator()" because
"brokerNameSet" is null
at RouteInfoManager.getTopicsByCluster(RouteInfoManager.java:1031)
RouteInfoManagerTest#testGetTopicsByClusterNotExisted: expected null but was
an empty TopicList (Tests run: 1, Failures: 1)
DefaultRequestProcessorGetTopicsByClusterTest#testGetTopicsByClusterNotExisted:
expected SYSTEM_ERROR but got SUCCESS (Tests run: 2, Failures: 1)
```
After the fix:
```
docker exec rmq-build mvn -q -pl namesrv test
-Dtest='DefaultRequestProcessorGetTopicsByClusterTest,RouteInfoManagerTest'
-Dsurefire.failIfNoSpecifiedTests=true
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0 - in
DefaultRequestProcessorGetTopicsByClusterTest
Tests run: 13, Failures: 0, Errors: 0, Skipped: 0 - in RouteInfoManagerTest
```
Note: the processor assertions live in a new test class because
`RequestProcessorTest`'s setUp uses a `Field.modifiers` reflection hack that
cannot run on JDK 12+ (verified pre-existing on develop with the container's
JDK 17/21).
No associated issue (self-discovered during a namesrv self-audit).
--
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]
