[ISSUE #10405] Route orderly retry messages to topic brokers#10624
Open
btlqql wants to merge 1 commit into
Open
[ISSUE #10405] Route orderly retry messages to topic brokers#10624btlqql wants to merge 1 commit into
btlqql wants to merge 1 commit into
Conversation
RockteMQ-AI
approved these changes
Jul 18, 2026
RockteMQ-AI
left a comment
Contributor
There was a problem hiding this comment.
Review by github-manager-bot
LGTM. Well-designed fix for orderly retry message routing.
Summary
Fixes #10405 by routing orderly retry messages to the original broker first (via RECONSUME_WHEN), with fallback to other brokers on the same topic. Also fixes a property mutation bug by using deepCopyProperties instead of direct reference sharing.
Key Changes
OrderlyRetryMessageSender— New class that implements smart retry routing: prefers the original broker, falls back to other topic brokers. Thread interruption handling is correct.deepCopyProperties— Fixes a subtle bug whereclearPropertyon the retry message would mutate the original message's properties map (shared reference).ConsumeMessagePopOrderlyService— SamedeepCopyPropertiesfix applied consistently.
Positive Notes
- Comprehensive test coverage including edge cases (single broker, all brokers failing, interrupted thread)
- Clean separation of concerns with the new
OrderlyRetryMessageSenderclass RETRY_QUEUE_ID = 0is correct since broker-created retry topics always use queue 0- Consistent fix applied to both push and pop orderly services
Automated review by github-manager-bot
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
Brief Description
When an orderly consumer reaches
maxReconsumeTimes, both the regular and POP orderly services publish the retry message with the default producer. Because the%RETRY%topic may not have publish route data yet, producer fallback can select a broker fromTBW102that does not host the original business topic. This creates retry topics on unrelated brokers and makes retry-topic placement inconsistent with the consumed topic.This change:
OrderlyRetryMessageSenderfor both orderly consumption paths;PROPERTY_RETRY_TOPICwhen a retry message is consumed again;The change is internal to the orderly retry path and introduces no public API changes.
How Did You Test This Change?
mvn -o -B -ntp -pl client -am -Dtest=OrderlyRetryMessageSenderTest,ConsumeMessageOrderlyServiceTest,ConsumeMessagePopOrderlyServiceTest -Dsurefire.failIfNoSpecifiedTests=false testResult:
BUILD SUCCESS; 27 tests run, 0 failures, 0 errors, 0 skipped. The Maven lifecycle also completed Checkstyle with 0 violations and SpotBugs with 0 findings for the affected reactor modules.The new tests cover original-broker preference, current-route filtering, cross-broker failover with fresh message copies, retry-message business-topic lookup, unavailable/empty route fallback, refusal to leave a known topic route, and interruption handling.