[ISSUE #10432] Optimize RocksDB ConsumeQueue iterator for sequential …

[echooymxq]

Which Issue(s) This PR Fixes

• Fixes [Enhancement] Optimize RocksDB ConsumeQueue iterator for sequential reads #10432

[codecov-commenter]

Codecov Report

❌ Patch coverage is 58.33333% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.96%. Comparing base (7e0cec3) to head (9c0bf8f).

Files with missing lines	Patch %	Lines
...rocketmq/common/config/AbstractRocksDBStorage.java	0.00%	6 Missing ⚠️
...rocketmq/store/queue/RocksDBConsumeQueueTable.java	76.47%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10433      +/-   ##
=============================================
- Coverage      48.04%   47.96%   -0.09%     
+ Complexity     13305    13280      -25     
=============================================
  Files           1377     1377              
  Lines         100613   100604       -9     
  Branches       12992    12988       -4     
=============================================
- Hits           48341    48252      -89     
- Misses         46354    46418      +64     
- Partials        5918     5934      +16     

☔ 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.

[oss-sentinel-ai]

Review by github-manager-bot

Summary

This PR optimizes RocksDB ConsumeQueue range queries by replacing multiGet with iterator-based sequential reads, which is more efficient for contiguous key ranges.

Findings

• [Good] RocksDBConsumeQueueTable.java:128-152 — Replaced N individual multiGet calls with a single iterator scan. For sequential reads, this reduces I/O overhead significantly.
• [Good] AbstractRocksDBStorage.java:325-400 — Changed callback from BiConsumer to IteratorCallback with boolean return, enabling early termination when offsets become non-contiguous.
• [Good] AbstractRocksDBStorage.java:360-363 — Conditional readahead: only set 4MB readahead for unbounded scans, avoiding over-prefetch for bounded ranges.
• [Good] AbstractRocksDBStorage.java:395 — Added iterator.status() call to detect iteration errors.
• [Good] RocksDBConsumeQueueStoreTest.java:139-188 — Added tests for continuous offsets and early termination at gaps.

Suggestions

1. Consider adding a benchmark to quantify the performance improvement for typical range query sizes.
2. The early termination logic (currentOffset != expectedOffset) assumes ConsumeQueue offsets must be continuous — this is correct per RocketMQ semantics but worth documenting.

Verdict

✅ Approved — Well-designed optimization that leverages RocksDB iterator for sequential access patterns.

---

Automated review by github-manager-bot

[echooymxq]

@lizhimins help review it.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Optimizes RocksDB ConsumeQueue reads by replacing N individual multiGet calls with a single sequential iterator scan, adds early termination support via IteratorCallback, and applies conditional readahead sizing.

Findings

• [Critical] AbstractRocksDBStorage.java:325 — Replacing BiConsumer with IteratorCallback (returning boolean) enables early termination. This is a good API improvement for bounded scans.
• [Critical] AbstractRocksDBStorage.java:360-363 — Conditional readahead: only sets 4MB readahead for unbounded scans. For bounded range scans, RocksDB's default readahead avoids over-prefetching. This is a smart optimization for sequential ConsumeQueue access patterns.
• [Warning] AbstractRocksDBStorage.java:395-398 — The iterator.status() call after the loop is good practice to detect RocksDB errors during iteration. However, the current code silently ignores the status. Consider logging or throwing if status() indicates an error.
• [Warning] ConsumeQueue.java:266-280 — The new rangeQuery stops at the first gap in offsets (continuity check). This is a behavioral change from the old multiGet approach which would return nulls for missing entries. Verify that all callers handle this correctly — specifically, that no caller relies on getting null entries for gaps.
• [Info] AbstractRocksDBStorage.java:771-775 — IteratorCallback as a nested functional interface is fine. Consider making it a top-level interface if it's used outside AbstractRocksDBStorage in the future.

Suggestions

• Add a comment or log warning when iterator.status() returns a non-OK status, to aid debugging of RocksDB errors in production.
• The continuity check in rangeQuery is semantically correct for ConsumeQueue but document this assumption clearly for future maintainers.

Verdict: Well-designed optimization. The sequential scan approach is significantly more efficient than N point lookups for ConsumeQueue access patterns. The conditional readahead and early termination are good improvements.

---

Automated review by github-manager-bot