lizhimins commented on PR #4973:
URL:
https://github.com/apache/rocketmq-dashboard/pull/4973#issuecomment-5806620987
This one needs to be redone rather than patched: it does not compile, and
the central metric measures the wrong thing.
1. Compile errors against the current branch:
- `MetadataService.precheckTopicQueueShrink` calls
`getTopicQueueStats(instanceId, topic)`; the method on this branch is
`getTopicStats` (`MetadataService.java:232`).
- It also calls `topicVO.getWriteQueueNums()`; `TopicVO` exposes `int
writeQueues` / `int readQueues` (`TopicVO.java:33-34`) — there is no
`writeQueueNums`, and the field is a primitive, so `== null` is not legal
either.
- `TopicQueueShrinkGuardEngine.java:73-74` compares `stat.getMinOffset()
== null` / `getMaxOffset() == null`, but
`TopicQueueStatsVO.minOffset/maxOffset` are primitive `long`
(`TopicQueueStatsVO.java:32-33`).
2. `maxOffset - minOffset` is not unconsumed backlog. Those offsets come
from `admin.examineTopicStats` (`MetadataService.java:259-266`) and describe
the broker's retained range — earliest non-deleted offset to latest offset. A
queue whose consumers are fully caught up but whose messages are still on disk
reports a large "backlog" and would be classified CRITICAL, blocking a safe
shrink. Unconsumed volume has to come from consumer-group offsets, i.e. the
`GET /api/groups/{name}/progress` → `QueueProgressVO` path.
3. `consumerGroupWithMaxLag` is hard-coded to the literal
`"POTENTIAL_SUBSCRIBERS"`, which reports an invented value as data;
`currentQueueNum` falls back to a magic 8; the drain estimate divides by a
magic 50 msg/s.
4. All six tests in `TopicQueueShrinkGuardEngineTest` are named `testXxx`
instead of the `...Test` suffix, and they build their own `TopicQueueStatsVO`
fixtures, so they cannot catch either the compile errors above or the
backlog-semantics problem.
5. The precheck is not wired to the write path that actually shrinks queues,
so it cannot guard anything; and `GET /api/topics/{name}/precheck-shrink` is
missing from `docs/api-spec.md`. The branch also currently conflicts with
`MetadataService.java`.
If you rework this, please base the risk assessment on consumer-group lag
per queue (progress API), keep the read-only GET, and wire the precheck into
the topic update flow so the guard is actually enforced.
--
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]
