caigy commented on code in PR #6778:
URL: https://github.com/apache/rocketmq/pull/6778#discussion_r1201740693
##########
controller/src/main/java/org/apache/rocketmq/controller/BrokerHeartbeatManager.java:
##########
@@ -63,4 +64,10 @@ void onBrokerHeartbeat(final String clusterName, final
String brokerName, final
* Check whether broker active
*/
boolean isBrokerActive(final String clusterName, final String brokerName,
final Long brokerId);
+
+ /**
+ * Count the number of active brokers in each broker-set of each cluster
+ * @return active brokers count
+ */
+ Map<String/*cluster*/, Map<String/*broker-set*/, Long/*active broker
num*/>> getActiveBrokersNum();
Review Comment:
It's unnecessary to use type `Long` for the number of brokers, `Integer` is
enough.
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/heartbeat/DefaultBrokerHeartbeatManager.java:
##########
@@ -173,4 +175,15 @@ public boolean isBrokerActive(String clusterName, String
brokerName, Long broker
return false;
}
+ @Override
+ public Map<String, Map<String, Long>> getActiveBrokersNum() {
+ Map<String, Map<String, Long>> map = new HashMap<>();
+ this.brokerLiveTable.keySet().forEach(id -> {
+ map.computeIfAbsent(id.getClusterName(), k -> new HashMap<>());
+ map.get(id.getClusterName()).compute(id.getBrokerName(), (broker,
num) ->
+ num == null ? 0L : num + 1L
+ );
+ });
+ return map;
+ }
Review Comment:
- Whether a broker is 'active' should be checked by `isBrokerActive()`,
rather than just checking if it is in`brokerLiveTable`.
- IMO this method provides a dynamic view of `brokerLiveTable`. As the
returned data structure is used only in metrics instead of heat beat of
brokers, it seems inappropriate to add methods in the interface
`BrokerHeartbeatManager`. Placing it in metric-related packages may be more
appropriate.
--
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]
