Conversation
🤖 Automated Review by RockteMQ-AIReview SummaryThis PR modifies 2 file(s) with +71/-5 lines ( Overall: COMMENT — Automated deep analysis was unavailable during this review pass. The PR structure appears valid. Recommendations
🔍 This is an automated review. Maintainers should verify findings before acting on them. Reply with |
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Summary
Fixes a subtle bug where ConsumerLagResolver.UNKNOWN (-1) sentinel values were being silently summed into currentTotalLag and projectedTotalLag, producing misleading negative totals. The new aggregateResetPreviewLag helper correctly propagates the UNKNOWN sentinel, and a new warning is surfaced when any queue has unavailable lag.
The approach is sound — early-return with UNKNOWN sentinel propagation is the correct pattern. The warning message is clear and actionable. Test coverage for the UNKNOWN propagation path is included.
LGTM.
Automated review by github-manager-bot
Which Issue(s) This PR Fixes
Brief Description
Apache reset-offset preview previously computed queue lag with
Math.max(0, brokerOffset - consumerOffset). That conflicts with Studio's establishedConsumerLagResolver.UNKNOWN(-1) contract for RocketMQ 5.x gRPC/POP progress where a negative broker difference means lag cannot be determined.This change reuses the existing unknown-lag contract for current and projected preview lag. Aggregate current/projected backlog is also unknown when any contributing queue is unknown, rather than summing fabricated zeroes. A warning tells operators that affected backlog totals are unavailable.
Reset target, offset delta, rewind/fast-forward counts, and
allowResetsemantics are unchanged when offset lookup succeeds.Red / Green Verification
Baseline:
master@d50ffecc9d7e8f8f46da64198831bd7952e6974e.Fail-before, Java 21:
brokerOffset=100,consumerOffset=120, and a valid reset target at 80.RocketMQAdminClientImplTest: 65 tests, exactly 1 failure.currentLag/currentTotalLag=-1, but current master returned healthy0; the other 64 tests passed.Green, Java 21:
RocketMQAdminClientImplTest: 66/66 passed with current-unknown and projected-unknown coverage.RocketMQAdminClientImplTest,MetadataServiceTest,ConsumerGroupControllerTest,GroupMutationPlanTest: 143/143 passed.mvn -B -ntp -DskipTests package: BUILD SUCCESS.git diff --check: clean.Compatibility / Risk
No API schema, frontend, dependency, reset execution, or known-zero/positive lag behavior changes. The existing UI already renders negative lag as unavailable (
-), so no frontend change is required.Only lag values that were previously clamped from an unresolved negative difference to zero now preserve the established unknown sentinel. The reset can still proceed when its offset target is otherwise complete; the preview simply stops presenting unavailable backlog as healthy.
AI-assisted source audit, implementation and regression authoring; fail-before and green verification were executed locally against the stated baseline.