RockteMQ-AI commented on code in PR #2543:
URL: 
https://github.com/apache/rocketmq-dashboard/pull/2543#discussion_r3836265759


##########
server/src/main/java/org/apache/rocketmq/studio/instance/InstanceController.java:
##########
@@ -47,6 +48,15 @@ public Result<List<InstanceVO>> listInstances(
         return Result.ok(instanceService.listInstances(type, search));
     }
 
+    @GetMapping("/page")
+    public Result<PageResult<InstanceVO>> listInstancesPage(

Review Comment:
   **[Info]** The existing `GET /` endpoint (line 43) still returns the full 
unpaginated list. Consider whether both endpoints are needed long-term, or if 
the old one should be deprecated once the frontend fully migrates to `/page`. 
Having two endpoints with overlapping behavior can lead to maintenance burden.



##########
server/src/main/java/org/apache/rocketmq/studio/instance/InstanceService.java:
##########
@@ -107,6 +109,30 @@ public List<InstanceVO> listInstances(InstanceType type, 
String search) {
         return sorted;
     }
 
+    public PageResult<InstanceVO> listInstances(InstanceType type, String 
search, int page,
+                                                int pageSize) {
+        validateListPagination(page, pageSize);
+        String normalizedSearch = search == null || search.isBlank() ? null : 
search.trim();
+        long total = instanceRepository.count(type, normalizedSearch);
+        if (total == 0) {

Review Comment:
   **[Performance]** This method calls `listInstances(type, normalizedSearch)` 
which loads ALL instances into memory (including remote resource-count fan-out 
for every instance) and then paginates in-memory with `subList`. This defeats 
the purpose of pagination — for a deployment with hundreds of instances, every 
page request still triggers N remote API calls.
   
   Consider paginating at the database level instead:
   1. Use `instanceRepository` with LIMIT/OFFSET (or MyBatis-Plus `Page`) to 
fetch only the current page of instances
   2. Then do the resource-count fan-out only for those instances on the 
current page
   
   This would reduce both DB load and remote API calls from O(N) to O(pageSize) 
per request.



-- 
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]

Reply via email to