zjncs opened a new pull request, #11121:
URL: https://github.com/apache/rocketmq/pull/11121
## Motivation
`cleanTopicByUnRegisterRequests` dereferences
`brokerAddrTable.get(brokerName)` without a null check while iterating
`reducedBroker`:
```java
if (this.brokerAddrTable.get(brokerName).isEnableActingMaster()) {
```
A brokerName lands in `reducedBroker` exactly when the request loop found
**no** `BrokerData` for it (or the group still has addresses left). If a topic
still carries a `queueData` for such a brokerName, this line throws an NPE. The
sibling `isNoMasterExists` null-checks the very same lookup, this call site
just forgot to.
The stale state is reachable and self-propagating: `unRegisterBroker(Set)`
mutates `brokerAddrTable`/`brokerLiveTable` in the request loop and only cleans
`topicQueueTable` afterwards in `cleanTopicByUnRegisterRequests`, all inside
one try/catch. Any exception thrown mid-batch (e.g. this very NPE) is swallowed
by the catch, so the queueData cleanup for the not-yet-iterated topics is
silently skipped while the broker data is already gone. From then on every
later unregister request for such a brokerName (channel-destroy resubmits are
common during connection flapping) re-triggers the NPE and aborts its whole
batch again.
The abort has real consequences for the remaining requests of the batch: the
write-perm wipe for masterless acting-master groups never happens (clients keep
writing to a group without a master) and `notifyMinBrokerIdChanged` is skipped.
## Modification
Hoist the `brokerAddrTable.get(brokerName)` lookup and skip the perm wipe
when the broker data is already gone, mirroring the null check in
`isNoMasterExists`.
## Test Evidence
New test `testUnregisterBrokerWithStaleQueueData` registers two
acting-master groups serving the same topics, removes one group's `BrokerData`
to simulate the state an aborted batch leaves behind, and unregisters both
groups' masters in one batch. It asserts the surviving group's queueData is
wiped to read-only.
```
docker exec rmq-build mvn -q -pl namesrv test
-Dtest='RouteInfoManagerTest#testUnregisterBrokerWithStaleQueueData'
-Dsurefire.failIfNoSpecifiedTests=true
```
Before the fix:
```
java.lang.NullPointerException: Cannot invoke
"BrokerData.isEnableActingMaster()" because the return value of
"Map.get(Object)" is null
at
RouteInfoManager.cleanTopicByUnRegisterRequests(RouteInfoManager.java:676)
Tests run: 1, Failures: 1, Errors: 0 (expected: 4 but perm stayed
read-write)
```
After the fix:
```
docker exec rmq-build mvn -q -pl namesrv test -Dtest='RouteInfoManagerTest'
-Dsurefire.failIfNoSpecifiedTests=true
Tests run: 13, Failures: 0, Errors: 0, Skipped: 0
```
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]
