RockteMQ-AI commented on code in PR #3019: URL: https://github.com/apache/rocketmq-dashboard/pull/3019#discussion_r3920252335
########## server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterTopologySummaryVO.java: ########## @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.rocketmq.studio.cluster.broker; + +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.NoArgsConstructor; + +import java.util.List; + +@Data +@Builder +@NoArgsConstructor +@AllArgsConstructor +public class ClusterTopologySummaryVO { + private int totalClusters; + private int healthyClusters; + private int warningClusters; + private int errorClusters; + private int offlineClusters; + private int totalBrokers; + private int runningBrokers; + private int readonlyBrokers; + private int maintenanceBrokers; + private int totalNameServers; + private int healthyNameServers; + private int unhealthyNameServers; + private int totalProxies; + private int healthyProxies; + private int unhealthyProxies; + private long totalTpsIn; + private long totalTpsOut; + private double maxDiskUsage; Review Comment: **[Minor: Redundant fields]** The `issues` list is embedded directly in the summary VO, and `criticalIssueCount` / `warningIssueCount` are also stored as separate fields. These counts can be derived from `issues.size()` filtered by severity. This is not a bug, but it introduces a consistency risk — if someone adds an issue to the list without incrementing the counter (or vice versa), the UI will show conflicting numbers. Consider either (a) computing the counts at serialization time via `@JsonIgnore` getters, or (b) documenting the invariant clearly. ########## server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterService.java: ########## @@ -122,7 +121,24 @@ private List<ClusterVO> probeRegistryEntry(NameserverRegistryVO entry) { public List<ClusterVO> listClusters(String instanceId) { log.info("Listing clusters for instance: {}", instanceId); - List<ClusterVO> discovered = clusterProvider.discoverClusters(instanceId); + return discoverClusters(instanceId); + } + + public ClusterTopologySnapshotVO getTopologySnapshot(String instanceId) { + List<ClusterVO> clusters = discoverClusters(instanceId); + return ClusterTopologySnapshotVO.builder() + .clusters(clusters) + .summary(summarizeTopology(clusters)) Review Comment: **[Performance]** `getTopologySnapshot()` calls `discoverClusters(instanceId)` once and passes the result to `summarizeTopology()`, which is good. However, the frontend's auto-refresh loop (every 15s) now triggers a full cluster discovery on each tick. Consider adding a short-lived cache (e.g. 30s TTL) for the topology snapshot to avoid hammering the NameServer/Broker probe path on rapid refreshes, especially for large clusters. -- 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]
