[ISSUE 365] fix queryMessagesByTopic#364
Conversation
oss-sentinel-ai
left a comment
There was a problem hiding this comment.
Review: Approved ✅
PR: #364 — [ISSUE 365] fix queryMessagesByTopic
Type: Bug fix (1 file, +2)
Assessment
Fixes message query by topic functionality in MessageServiceImpl. Adds missing logic for topic-based message queries.
Verdict
✅ Clean, targeted fix.
🤖 Automated review by oss-sentinel-ai
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot
Summary
Fixes duplicate message results when querying by topic. The root cause was that start offset was not being updated after each pull() call, causing the same messages to be fetched repeatedly.
Findings
-
[Info]
MessageServiceImpl.java:373— Addingstart = pullResult.getNextBeginOffset()after thepull()call is the correct fix. Without this, thestartvariable remained at its initial value, causing the while loop to re-pull from the same offset on each iteration. -
[Warning] The fix is minimal (2 lines added) but the PR description could be more detailed. The issue (#365) title mentions 'queryMessagesByTopic 重复' but the body doesn't explain the root cause or reproduction steps. Consider updating the PR description for future maintainability.
-
[Info] No test was added for this fix. Consider adding a unit test that verifies
queryMessagesByTopicreturns distinct results when multiple pages are pulled.
Suggestions
- Add a brief explanation in the PR body or a code comment about why the offset update was missing.
- Consider adding a regression test to prevent this from recurring.
Verdict
Correct bug fix. The missing offset update was clearly a logic error. Low risk.
Automated review by github-manager-bot
What is the purpose of the change
根据topic查询消息重复
fix #365
Brief changelog
XX
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily. Notice,
it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyleto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.