lizhimins commented on PR #4544: URL: https://github.com/apache/rocketmq-dashboard/pull/4544#issuecomment-5758994617
Merged with a few maintainer adjustments that complete the `-1` sentinel rollout. Your core change is unchanged: the three-state resolution in `ProxyConsumerResolver` (discovery failure / proxy lookup failure → unavailable, `CONSUMER_NOT_ONLINE` or an observed empty set → a trustworthy zero) and the `onlineInstances = -1` sentinel match the unknown-lag contract we already ship, and `GroupDetailToolHandler` now answers `UNKNOWN` instead of "backlog with no online consumer". What was added on top, and why: **1. The catalog schema rejected the sentinel — this one was a hard runtime failure.** `server/src/main/resources/tool-catalog/tools/group.yaml` declared `minimum: 0` for `rmq.group.detail`'s `onlineInstances`. `ToolValidationFilter` runs `ToolSchemaValidator.validateOutput` on **every** tool result, and it throws `IllegalStateException` on a violation — so the first time a group's connection inventory was unavailable, `rmq.group.detail` would have failed outright instead of reporting the unknown state. Verified locally by reverting the schema and watching the contract test fail with `Tool output validation failed for rmq.group.detail: [/onlineInstances: must have a minimum value of 0]`. Fixed by allowing and documenting the sentinel in all three schema sites (`rmq.group.list` items, `rmq.group.detail`, and `rmq.group.detail`'s `configurations[]` items), plus `make catalog-generate` so `rmqctl/internal/catalog/catalog_gen.go`'s digest stays in sync — CI does not run on this branch, so a stale digest would have landed silently. `GroupListItem` / `GroupDetailOutput` and `docs/api-spec.md` now document the sentinel too, and `ToolOutputSchemaContractTest` carries `-1` samples for both tools. **2. The sentinel leaked into three display paths.** - `MetadataService.buildConsumerGroupCsv` printed the raw `-1` in the `Online Instances` column; it now uses `onlineInstancesText(...)`, mirroring the existing `lagText(...)` treatment of the unknown-lag sentinel in the same row builder. - `web/src/services/consumerService.ts` had the same problem in the browser-side export. Note the exported cell was not even a bare `-1`: `escapeCsvCell` formula-guards leading `-`, so the column contained `'-1`. - The `在线客户端` column's `sorter` was still `(a.onlineInstances ?? 0) - (b.onlineInstances ?? 0)`, which sorts an unavailable group *before* a group with zero clients — i.e. as the healthiest possible answer. It now uses a sort key that pushes unavailable to the end, matching `lagSortValue` in the adjacent `总堆积量` column. Since the table cell, the two `Statistic` formatters and both CSV exports all need the same rule, the helpers moved to `web/src/utils/consumerConnections.ts` (`UNKNOWN_ONLINE_INSTANCES` / `isOnlineInstancesAvailable` / `formatOnlineInstances` / `onlineInstancesSortValue`), deliberately shaped like the existing `web/src/utils/consumerLag.ts` so the two sentinels read the same way. **3. Test naming.** `detailMarksHealthUnknownWhenConsumerConnectionsAreUnavailable` → `...UnavailableTest`; our convention is that test method names end with `Test`. Every one of those four changes has a mutation-sensitive test: reverting the schema, the backend CSV formatting, the frontend CSV formatting or the sorter each turns a test red (contract test, `MetadataServiceTest`, `consumerService.test.ts`, `ConsumerPage.test.tsx` respectively). One item left alone on purpose, and one note: - We kept your treatment of a `CONSUMER_NOT_ONLINE` heartbeat-syncer discovery as a trustworthy zero. Your code comment states the trade-off explicitly, which is the right way to record it. - Unrelated to this PR: the browser-side export still prints the raw `totalLag`, so an unknown lag reaches that CSV as `'-1` while the backend export renders `unknown`. That inconsistency predates this change and will be handled separately. -- 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]
