iloveeyjafjalla opened a new pull request, #11357:
URL: https://github.com/apache/rocketmq/pull/11357
### Which Issue(s) This PR Fixes
- Fixes #11356
### Brief Description
`AdminSendMessage` resolved the write queue of a message group with
`Math.floorMod(shardingKey.hashCode(), queues.size())`, while the gRPC data
plane resolves it with Guava's
consistent hash (`SendMessageActivity.java:414`). The two functions disagree
for the same group and queue count —
for message group `groupA-fifo` over 3 writable queues, `floorMod` picks
queue 2 and `consistentHash` picks
queue 0 — so a message injected through the admin surface could land on a
queue that the group's producers never
write to. Per-group FIFO order only holds within a single queue, so the
group's messages could be consumed out
of order, and the RPC still answered `OK`, so nothing told the operator the
group was split.
`selectQueue`'s javadoc states the intent ("A FIFO message must keep going
to the queue its message group hashes
to") and `docs/proxy-admin.md` §D4 documents this path as "system properties
honoured for timer / FIFO messages",
so the mapping is meant to match the data plane.
The change makes the admin path use the same hash function the data plane
uses. Nothing else changes: the
no-message-group branch keeps spreading over the writable queues.
### How Did You Test This Change?
New unit test
`ProxyAdminGrpcServiceTest#adminSendMessageRoutesMessageGroupToTheDataPlaneQueueTest`:
it captures
the `QueueSelector` the RPC hands to `MessagingProcessor#sendMessage`, runs
it against a mocked `MessageQueueView`
with 3 writable queues, and asserts the selected queue is the one the data
plane's formula picks for that group.
It also asserts the two formulas differ for the test key, so the test cannot
silently pass if the old mapping
comes back.
Results on Temurin JDK 8 (same JDK as CI), base `9ea2ccdc2` plus this patch:
- before the fix, the new test fails: `expected:<...queueId=0...> but
was:<...queueId=2...>`
- after the fix: `Tests run: 1, Failures: 0, Errors: 0, Skipped: 0`
- whole class: `Tests run: 61, Failures: 0, Errors: 0, Skipped: 0` (`mvn -B
-ntp -pl proxy -am test -Dtest=ProxyAdminGrpcServiceTest`)
This change was prepared with AI assistance (analysis, patch and test
drafting); the diff and every result above
were reproduced locally by running the module's tests against the base
commit and the patched tree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
