You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "zjncs (via GitHub)" <gi...@apache.org> on 2026/09/08 04:32:25 UTC

[PR] fix(tools): skip master-address comparison in consumed() when the routed broker has no master [rocketmq]

zjncs opened a new pull request, #11069:
URL: https://github.com/apache/rocketmq/pull/11069

   ### Motivation
   
   `DefaultMQAdminExtImpl.consumed()` resolves the master address of each broker referenced by the consume-stats offset table:
   
   ```java
   String addr = NetworkUtil.convert2IpString(brokerData.getBrokerAddrs().get(MixAll.MASTER_ID));
   ```
   
   `examineConsumeStats` accepts entries from brokers whose `selectBrokerAddr()` resolved to a slave (master down, stale route). For such a broker `getBrokerAddrs().get(MixAll.MASTER_ID)` returns null, and `NetworkUtil.convert2IpString(null)` → `string2SocketAddress(null)` crashes on `addr.lastIndexOf(":")`.
   
   The NPE aborts the whole track listing: `queryMsgById -i ...` → `printMsg` → `messageTrackDetail` → `consumed(msg, group)` fails for **all** groups, not just the group served by the unreachable broker. The sibling `consumedConcurrent` already uses the raw address with a null-safe comparison direction, showing the null case was considered elsewhere.
   
   ### Modifications
   
   Skip the store-host comparison when the master address is null, mirroring the null-safe structure of the sibling method.
   
   ### Verification
   
   Fail-before (new test `testConsumedWithSlaveOnlyBrokerInRoute`, run against the unpatched code — a spy of `DefaultMQAdminExtImpl` returning one offset-table entry for `broker1` whose route registers only brokerId 1):
   
   ```
   Tests run: 1, Failures: 0, Errors: 1 -- DefaultMQAdminExtImplTest
   java.lang.NullPointerException: Cannot invoke "String.lastIndexOf(String)" because "addr" is null
   ```
   
   Pass-after:
   
   ```
   Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 -- DefaultMQAdminExtImplTest#testConsumedWithSlaveOnlyBrokerInRoute
   ```
   
   Note: the pre-existing `testConsumeMessageDirectly` in this class fails on JDK 21 in my local container with `Mockito cannot mock this class: java.net.InetAddress` — verified to fail identically on a clean develop checkout (before my changes), so it is an environment limitation, not a regression of this PR.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org