lizhimins commented on PR #4773: URL: https://github.com/apache/rocketmq-dashboard/pull/4773#issuecomment-5806616585
Thanks for the thorough write-up — the diagnosis is accurate, and the shape of the fix is the right one: threading `subscriptionMode` down into the provider page query so the predicate joins the `LambdaQueryWrapper` before `selectPage`, with the total taken from the database count, is exactly how a filter on a server-paginated list has to work (and it matches what the CSV export path already does). We are not taking it, though, and I would rather say why explicitly than leave the PR hanging. What this fixes is a filtering inaccuracy on the consumer-group list: the Push/Pop selector filters the rows already loaded for the current server page while the pager reports the unfiltered total. In practice a single instance carries a small number of consumer groups, so the window where those two disagree is narrow, and the operator still reaches the groups by clearing the filter or paging. Against that, the change replaces signatures across the provider SPI (`InstanceProvider.listConsumerGroupsPage`, `MetadataProvider.listConsumerGroupsPage`, the Apache provider and its query wrapper) and touches 16 files — a wide contract change, carried by every provider implementation including the cloud ones, for a low-impact correction. So this is "not planned" rather than "wrong": the reasoning in the PR is sound, we just do not think the SPI churn is justified by what it buys today. Closing it, and no rework is expected on your side. If the subscription-mode filter ever becomes a correctness problem at realistic group counts, that is the moment to revisit the pushdown. The same applies to #4813, which attacked the same defect from the service layer — that one is closed too, with the specifics in its own thread. -- 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]
