[ISSUE #10549] Fix lite topic reset offset: memory leak, FIFO block bypass, and offset-0 reset failure

[f1amingo]

Which Issue(s) This PR Fixes

• Fixes [Bug] Lite topic reset offset: memory leak, FIFO block bypass, and offset-0 reset failure #10549

Brief Description

This PR fixes three related bugs in lite topic offset reset:

1. Memory leak in resetOffsetTable: removeResetOffset() now cleans up empty inner map entries after removing a queueId, preventing stale key accumulation.

2. FIFO block bypass on reset: When useServerSideResetOffset is enabled and a reset offset is pending, isFifoBlocked() returns false immediately, allowing consumption to proceed from the reset position.

3. Reset to offset 0 silently skipped: Changed resetOffset > 0 to resetOffset >= 0 in ResetOffsetByTimeCommand so that offset 0 is a valid reset target.

Additionally, a new eraseResetOffset(topic, group, queueId) method is introduced and called during lite topic removal in AbstractLiteLifecycleManager to clean up orphaned reset offset entries.

How Did You Test This Change?

• Added unit test testEraseResetOffset in ConsumerOffsetManagerTest verifying precise removal and empty-map cleanup.
• Added unit test testIsFifoBlocked_hasResetOffset in PopLiteMessageProcessorTest verifying FIFO bypass when reset is pending.
• Existing tests continue to pass.

[RockteMQ-AI]

Review by github-manager-bot

Summary

This PR fixes three related bugs in lite topic offset reset: memory leak in resetOffsetTable (empty inner maps not cleaned up), FIFO block bypass when a server-side reset offset is pending, and offset-0 being silently skipped during reset. All three fixes are well-scoped and include appropriate unit tests.

Findings

• [Info] ConsumerOffsetManager.java:466-480 — The computeIfPresent lambda in both queryThenEraseResetOffset and eraseResetOffset performs a side-effect (map.remove(queueId)) on the inner ConcurrentHashMap within the outer map's atomic computation. This is a well-known pattern and works correctly because the inner map is also concurrent, but consider adding a brief comment explaining the intentional side-effect for future maintainers.

• [Info] ConsumerOffsetManager.java:477-481 — The new eraseResetOffset(topic, group, queueId) method is clean and consistent with the updated queryThenEraseResetOffset. The return-null-from-lambda pattern to remove the outer key when the inner map becomes empty is idiomatic ConcurrentHashMap usage.

• [Info] PopLiteMessageProcessor.java:314-317 — The isFifoBlocked bypass correctly checks hasOffsetReset(lmqName, group, 0) with hardcoded queueId=0, which is appropriate for lite topics (single queue). The early return false allows consumption to proceed from the reset position instead of being blocked by FIFO ordering — this is the correct behavior when a server-side reset is pending.

• [Info] ResetOffsetByTimeCommand.java:143 — Changing resetOffset > 0 to resetOffset >= 0 is correct. searchOffset can legitimately return 0 (e.g., when the timestamp maps to the very first message), and silently skipping it was a bug.

• [Info] AbstractLiteLifecycleManager.java:208 — The eraseResetOffset(lmqName, group, 0) call is correctly placed before removeConsumerOffset in deleteLmq, ensuring orphaned reset offset entries are cleaned up when a lite topic is removed.

Suggestions

• Consider adding a brief inline comment in queryThenEraseResetOffset explaining why the lambda has a side-effect on the inner map, to prevent future refactoring from inadvertently breaking the cleanup logic.
• The eraseResetOffset method could benefit from a null/empty check on topic/group parameters (similar to assignResetOffset's validation) for defensive programming, though this is optional since the caller already provides valid values.

Verdict

The fixes are correct, well-tested, and minimal. LGTM.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fixes three related bugs in lite topic offset reset:

1. Memory leak in resetOffsetTable — empty inner maps not cleaned up after removing entries
2. FIFO block bypass — when useServerSideResetOffset is enabled and a reset offset is pending, consumption should proceed immediately
3. Reset to offset 0 silently skipped — boundary condition > 0 changed to >= 0

Findings

Bug 1: Memory leak fix

• [Info] ConsumerOffsetManager.java:469-477 — queryThenEraseResetOffset now cleans up empty inner maps via computeIfPresent. This prevents stale key accumulation in resetOffsetTable.
• [Info] ConsumerOffsetManager.java:480-486 — New eraseResetOffset(topic, group, queueId) method provides a clean API for removing reset offset entries without querying.
• [Info] AbstractLiteLifecycleManager.java:208 — Called during deleteLmq to clean up orphaned reset offset entries when a lite topic is removed. Good integration.

Bug 2: FIFO block bypass

• [Info] PopLiteMessageProcessor.java:314-316 — When useServerSideResetOffset is enabled and a pending reset exists for queue 0, isFifoBlocked returns false immediately, bypassing the consumerOrderInfoManager.checkBlock call. This allows consumption to proceed from the reset position.

Bug 3: Offset 0 boundary

• [Info] ResetOffsetByTimeCommand.java:143 — Simple > 0 → >= 0 fix. Offset 0 is a valid reset target (beginning of topic).

Suggestions

• [Warning] ConsumerOffsetManager.java:471-473 — The computeIfPresent lambda returns null to remove the entry when the inner map is empty. This is correct per ConcurrentMap semantics (returning null from computeIfPresent removes the mapping), but worth noting that this relies on the inner map being a ConcurrentMap that supports this pattern. The test testEraseResetOffset verifies this correctly.
• [Info] PopLiteMessageProcessor.java:314 — The FIFO bypass is hardcoded to queue 0. This matches the lite topic model where each lite topic uses a single queue, but if the model ever changes to multi-queue, this would need updating. Consider adding a brief comment explaining why queue 0 is assumed.
• [Info] Tests are well-structured. testEraseResetOffset verifies both individual removal and empty-map cleanup. testIsFifoBlocked_hasResetOffset correctly verifies that checkBlock is never called when a reset is pending.

Verdict

Well-structured multi-bug fix with focused changes and proper test coverage. Each fix is independent and addresses a real issue. LGTM.

---

Automated review by github-manager-bot