add comments to pop/ack/changInvisibleTime, No Code change

[winglechen]

约1200行注释，无代码修改

[RockteMQ-AI]

• [Positive] NettyRequestProcessor.java — Converting rejectRequest() to a default method is a clean refactoring. It eliminates boilerplate across ~20 processor classes and centralizes the "no rejection" default. Processors that need actual flow control can still override.

• [Positive] ConsumeQueue.correctMinOffset() — The reordered checks are more robust:

  • Checking lastMappedFile == null first avoids calling getMaxOffset() on a potentially empty queue.
  • The > (instead of >=) comparison correctly allows the boundary case where minLogicOffset == maxOffset (all entries consumed but file still present).
  • Explicitly resetting minLogicOffset=0, maxPhysicOffset=-1 when files are deleted prevents stale offset values.

• [Info] PR description — The description says "无代码修改" (no code changes), but this PR now includes interface refactoring and correctMinOffset() logic changes. Consider updating the description to reflect the full scope, which will help reviewers and future readers.

Verdict

The code changes are well-motivated and the new test provides good coverage. Please fix the duplicate import in AckMessageProcessor.java. Otherwise LGTM.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot (Incremental)

Summary

21 new commits since the previous review (2026-06-25), adding Javadoc and inline comments to 7 files: BrokerController, ConsumerOrderInfoManager, QueueLevelConsumerManager, QueueLevelConsumerOrderInfoLockManager, AckMessageProcessor, HookUtils, and TransRocksDBRecord. All changes remain documentation-only — no functional code modifications.

Findings

• [Critical] AckMessageProcessor.java — Duplicate import detected:

  import org.apache.rocketmq.broker.pop.PopConsumerService;
  import org.apache.rocketmq.broker.pop.PopConsumerService;

  This will cause a compilation warning or error depending on the compiler settings. Please remove the duplicate line.

• [Warning] QueueLevelConsumerOrderInfoLockManager.java — Typo in the class-level Javadoc:

  "When a consumer Pop s a batch"

  Should be "When a consumer Pops a batch" (missing 'p').

• [Positive] BrokerController.java — The Javadoc for registerMessageStoreHook() clearly documents the 4-step hook pipeline with proper <ol> ordering. Well structured.

• [Positive] QueueLevelConsumerManager.java — The inline comments in update(), needBlock(), and mergeOffsetConsumedCount() provide useful context for the orderly consumption logic. The @param attemptId addition fills a gap in the existing Javadoc.

• [Positive] HookUtils.java — The new Javadoc for sendMessageBack, handleScheduleMessage, checkBeforePutMessage, and handleLmqQuota consistently documents the hook pipeline steps. The SendMessageBackHook import is correctly added.

• [Positive] TransRocksDBRecord.java — The inline comments in checkTransRecordsStatus clarify the transaction check flow (checkTimes → delete, msgExt null → delete, immunity check → resolve, update checkTimes). Helpful for understanding the state machine.

• [Info] QueueLevelConsumerManager.java — Some inline comments describe obvious operations (e.g., // init orderInfo map, // create or merge orderInfo). While not incorrect, these add visual noise without much insight. Consider whether they add value beyond what the code already expresses.

Suggestions

1. Fix the duplicate import in AckMessageProcessor.java — this is the only actionable issue.
2. Fix the typo in QueueLevelConsumerOrderInfoLockManager.java Javadoc.
3. Consider consolidating the 21 small commits into fewer commits (e.g., squash by file or by logical group) before merge to keep the git history clean.

---

Automated review by github-manager-bot