[ISSUE #10722] [Bug] Lite Topic may not be cleaned up when message store timestamp is unavailable

[f1amingo]

Which Issue(s) This PR Fixes

• Fixes [Bug] Lite Topic may not be cleaned up when message store timestamp is unavailable #10722

Brief Description

Add a guard in isLiteTopicExpired to return true when getMessageStoreTimeStamp returns a non-positive value, preventing unreliable expiration judgment caused by an invalid store timestamp.

How Did You Test This Change?

Unit test added in AbstractLiteLifecycleManagerTest to verify that isLiteTopicExpired returns true when getMessageStoreTimeStamp returns 0.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Adds a defensive guard in AbstractLiteLifecycleManager#isLiteTopicExpired to return true (topic expired) when getMessageStoreTimeStamp returns a non-positive value, preventing unreliable expiration judgment and potential downstream issues in hasConsumerLag.

Overall Assessment: ✅ Approve

This is a clean, well-scoped bug fix with appropriate test coverage.

Analysis

Root cause: When getMessageStoreTimeStamp returns 0 or -1 (e.g., message physically deleted or temporarily unreadable), the original code proceeds to compute inactiveTime with an invalid base. While the arithmetic would coincidentally produce a very large inactiveTime, the real problem is that latestStoreTime = 0 would also be passed into hasConsumerLag(), potentially causing incorrect lag calculations.

Fix correctness: The guard if (latestStoreTime <= 0) return true is semantically correct — if the store timestamp is unavailable, treating the topic as expired triggers cleanup, which aligns with the issue's expectation. The pattern is consistent with the existing maxOffset <= 0 guard above it.

Test coverage: The new test case in AbstractLiteLifecycleManagerTest correctly verifies the guard condition by mocking getMessageStoreTimeStamp to return 0 and asserting isLiteTopicExpired returns true.

Minor Suggestion (non-blocking)

Consider adding a LOGGER.warn when this guard triggers, similar to the existing maxOffset <= 0 warning:

if (latestStoreTime <= 0) {
    LOGGER.warn("unexpected condition, store timestamp <= 0, {}, {}", lmqName, latestStoreTime);
    return true;
}

This would help diagnose production issues where the store layer fails to return valid timestamps.

Files Reviewed

• broker/src/main/java/org/apache/rocketmq/broker/lite/AbstractLiteLifecycleManager.java — guard logic ✅
• broker/src/test/java/org/apache/rocketmq/broker/lite/AbstractLiteLifecycleManagerTest.java — test coverage ✅

[codecov-commenter]

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 48.21%. Comparing base (00e45b8) to head (0b6fddd).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
...etmq/broker/lite/AbstractLiteLifecycleManager.java	95.65%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10723      +/-   ##
=============================================
- Coverage      48.32%   48.21%   -0.11%     
+ Complexity     13512    13485      -27     
=============================================
  Files           1380     1380              
  Lines         101091   101102      +11     
  Branches       13101    13106       +5     
=============================================
- Hits           48851    48750     -101     
- Misses         46279    46370      +91     
- Partials        5961     5982      +21     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fixes a bug where Lite Topic cleanup is blocked when getMessageStoreTimeStamp() returns an invalid value (≤ 0).

Findings

• [Info] AbstractLiteLifecycleManager.java:184 — When latestStoreTime <= 0, the subsequent System.currentTimeMillis() - latestStoreTime would produce an abnormally large inactiveTime, which should cause the topic to be considered expired. However, the semantics depend on whether 0 means "no data" or "epoch zero". Treating <= 0 as "unavailable → expired" is a reasonable and safe default.
• [Info] Test coverage is good — the new test case validates the storeTime == 0 path.

Suggestions

Clean, minimal fix with appropriate test coverage. LGTM.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Summary

Defensive fix with proper validation and test coverage. LGTM.

---

Automated review by github-manager-bot