[ISSUE #10577] Keep reentrant POP receives broker sticky

[Aias00]

Summary

Fixes #10577.

This change keeps reentrant orderly POP receive requests broker-sticky when the request carries an attemptId.

Motivation

Reentrant orderly POP depends on broker-local order state. If a retry with the same attemptId asks for a specific broker but the selector silently falls back to another broker, the retry can lose access to the original broker-local OrderInfo and leave the FIFO queue blocked until invisible time expires.

Changes

• Pass a broker-sticky flag into ReceiveMessageQueueSelector only for FIFO receive requests carrying attemptId.
• When sticky mode is enabled and the requested broker is unavailable, return null instead of falling back to another broker.
• Preserve the existing fallback behavior for non-sticky receive requests.
• Extend ReceiveMessageActivityTest to cover both fallback and sticky-missing-broker behavior.

Verification

mvn -pl proxy -Dtest=ReceiveMessageActivityTest test \
  -DfailIfNoTests=false -Djacoco.skip=true \
  -Dspotbugs.skip=true -Dcheckstyle.skip=true -Drat.skip=true

Tests run: 9, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

mvn -pl proxy -DskipTests -Dspotbugs.skip=true -Drat.skip=true compile

Checkstyle violations: 0
BUILD SUCCESS

git diff --check is clean.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fixes broker selection for reentrant orderly POP receives by adding a brokerSticky mode to ReceiveMessageQueueSelector. When a FIFO retry carries an attemptId, the selector now stays pinned to the target broker instead of silently falling back to another broker — which would lose access to broker-local OrderInfo and block the FIFO queue.

Findings

• [Info] ReceiveMessageActivity.java:236-238 — When brokerSticky=true and the broker is not found, select() returns null. Callers must handle this gracefully (e.g., return an error to the client rather than NPE). The PR description implies this is the intended behavior, so it looks correct.
• [Info] The condition fifo && request.hasAttemptId() is well-scoped — it only activates sticky mode for reentrant FIFO requests, preserving the existing fallback path for all other cases.

Suggestions

• Minor: consider adding a brief Javadoc on the brokerSticky parameter in the two-arg constructor to clarify the contract (returns null when the target broker is unavailable, rather than falling back).

Overall this is a clean, focused fix with good test coverage. LGTM.

---

Automated review by github-manager-bot

[codecov-commenter]

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 48.20%. Comparing base (577b89f) to head (a998906).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
...proxy/grpc/v2/consumer/ReceiveMessageActivity.java	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10652      +/-   ##
=============================================
- Coverage      48.32%   48.20%   -0.13%     
+ Complexity     13490    13473      -17     
=============================================
  Files           1380     1380              
  Lines         101031   101097      +66     
  Branches       13092    13104      +12     
=============================================
- Hits           48825    48730      -95     
- Misses         46247    46374     +127     
- Partials        5959     5993      +34     

☔ 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

Adds broker-sticky behavior to ReceiveMessageQueueSelector for reentrant POP receives: when a message is FIFO and has an attemptId, the selector will stick to the same broker even if the preferred broker's queue is not available (returns null instead of falling through to random selection).

Findings

• [Warning] ReceiveMessageActivity.java:239-242 — The new logic: if brokerSticky is true and the broker-name lookup returns null, it still returns null instead of falling through to getRandomOne(). This means for FIFO messages with attemptId, if the preferred broker is unavailable, the receive will get a null queue. Verify that the caller handles this null gracefully (e.g., retries or returns an empty response rather than NPE).
• [Info] ReceiveMessageActivity.java:145 — The fifo && request.hasAttemptId() condition is a reasonable trigger for broker stickiness. FIFO ordering requires the same broker to maintain message ordering guarantees.
• [Info] Backward-compatible constructor ReceiveMessageQueueSelector(String brokerName) delegates to the new constructor with brokerSticky=false, preserving existing behavior for non-FIFO cases.
• [Info] Tests cover both the sticky and non-sticky paths.

Suggestions

Consider adding a null check or comment at the call site where select() returns null for the sticky case, to make it explicit that this is intentional (not a bug).

Verdict

Reasonable change for FIFO ordering guarantees. The main concern is ensuring callers handle the null queue case properly.

---

Automated review by github-manager-bot

[Aias00]

Addressed the review feedback in a998906f3:

• Documented the broker-sticky null-return contract in ReceiveMessageQueueSelector Javadoc.
• Added a call-site comment clarifying that reentrant FIFO POP must stay on the original broker instead of falling back.
• Added coverage for FIFO receive with attemptId to verify the sticky selector is passed into popMessage and returns no queue when the original broker is unavailable.

Local verification:
mvn -pl proxy -Dtest=ReceiveMessageActivityTest test passed with 10 tests; git diff --check passed.