[ISSUE #10432] Optimize RocksDB ConsumeQueue iterator for sequential …#10433
Open
echooymxq wants to merge 1 commit into
Open
[ISSUE #10432] Optimize RocksDB ConsumeQueue iterator for sequential …#10433echooymxq wants to merge 1 commit into
echooymxq wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|
oss-sentinel-ai
approved these changes
Jun 8, 2026
oss-sentinel-ai
left a comment
There was a problem hiding this comment.
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
multiGetcalls with a single iterator scan. For sequential reads, this reduces I/O overhead significantly. - [Good] AbstractRocksDBStorage.java:325-400 — Changed callback from
BiConsumertoIteratorCallbackwith 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
- Consider adding a benchmark to quantify the performance improvement for typical range query sizes.
- 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
Contributor
Author
|
@lizhimins help review it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which Issue(s) This PR Fixes