lizhimins commented on PR #4813: URL: https://github.com/apache/rocketmq-dashboard/pull/4813#issuecomment-5806617800
Thanks for chasing this — the front-end half is right, but the back-end half does not actually fix the reported defect, so I am closing it. The problem is where the filter runs. `RocketMQMetadataProvider.listConsumerGroupsPage` (server/.../provider/apache/RocketMQMetadataProvider.java:266-288) does the paging itself with `groupMapper.selectPage(new Page<>(page, pageSize), query)`, so by the time `MetadataService.applySubscriptionModeFilter` sees the rows the slice is already taken. Filtering a slice, and shrinking the total by only the rows dropped from *that* slice, cannot produce a correct global total, and it cannot reach rows that live on later pages. Concretely, with the 30 Push + 2 Pop / pageSize 20 example from your own description: page 1 comes back with 20 Push rows, the filter removes all 20, and the total becomes 32 - 20 = 12. The pager now reports "共 12 个 Group" with a single page and the table is empty — the two Pop groups are unreachable, which is worse than the current behaviour (today they at least show up when they happen to land on the loaded page). The Javadoc on the new 6-arg overload also states "the filter is applied to the full filtered set before slicing so the reported total stays consistent with the rows", which is the opposite of what the code does (and of what the private helper's own Javadoc says two lines below). Neither new back-end test can catch this: both build a single page (`PageResult.of(List.of(pushGroup, popGroup), 2, 1, 20)`), where slice-then-filter and filter-then-slice are indistinguishable. Two smaller notes: the retained 5-arg `listConsumerGroupsPage` overload has no caller left in main or test after this change, and the subscription mode is stored in the `messageModel` column (`RocketMQMetadataProvider.java:311-314`), so it is available as a SQL predicate. A correct fix would thread `subscriptionMode` down into `MetadataProvider.listConsumerGroupsPage(...)` / `InstanceProvider.listConsumerGroupsPage(...)` so the predicate joins the `LambdaQueryWrapper` before `selectPage`, which is also how the CSV export path already behaves (`MetadataService.java:680-684` filters the full list, then slices). We are not taking that route right now either — #4773 proposed exactly that pushdown and is closed with the reasoning: at realistic group counts per instance the practical impact is small, while the pushdown means changing the provider SPI across every implementation. So this is closed as not planned, and no rework is expected. Nothing needs to change on your side; the front-end commit in this PR is fine as it stands. -- 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]
