You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "btlqql (via GitHub)" <gi...@apache.org> on 2026/09/21 06:29:54 UTC
[PR] fix(message): resolve hostname-registered brokers in the offset msgId guard [rocketmq-dashboard]
btlqql opened a new pull request, #4683:
URL: https://github.com/apache/rocketmq-dashboard/pull/4683
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Which Issue(s) This PR Fixes
- Fixes #4554
### Brief Description
`BrokerTopologyGuards.validatedBrokerAddr` decided whether the broker embedded in a physical-offset
msgId belongs to the selected instance with an exact `Set.contains` against the addresses from
`examineBrokerClusterInfo()`. Those addresses are the strings the brokers registered with, while
`MessageDecoder.decodeMessageId` always yields a numeric address (`InetAddress.getByAddress`, no
reverse lookup). On a cluster whose brokers register with a hostname (`brokerIP1=broker-0`, the style
of the bundled `deploy/rocketmq/conf/broker-0.conf`), the two never match, so the guard rejected every
offset id and the lookup returned an empty result.
The guard now falls back to comparing the resolved addresses (same port required) before rejecting, so
a numeric address that resolves to a registered hostname endpoint is accepted. Unresolvable or
unrelated endpoints are still rejected, and the exact-match fast path is unchanged.
### How Did You Test This Change?
The regression test fails on the unmodified base (only the new test fails; the three negative
controls already pass), and passes with the fix:
```
$ cd server && mvn -B -ntp test -Dtest=BrokerTopologyGuardsTest # before the fix
[ERROR] Tests run: 4, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 2.329 s <<< FAILURE! -- in org.apache.rocketmq.studio.provider.apache.BrokerTopologyGuardsTest
[ERROR] org.apache.rocketmq.studio.provider.apache.BrokerTopologyGuardsTest.acceptsAnOffsetIdWhoseBrokerIsRegisteredWithAHostname -- Time elapsed: 0.037 s <<< FAILURE!
at org.apache.rocketmq.studio.provider.apache.BrokerTopologyGuardsTest.acceptsAnOffsetIdWhoseBrokerIsRegisteredWithAHostname(BrokerTopologyGuardsTest.java:43)
[ERROR] Tests run: 4, Failures: 1, Errors: 0, Skipped: 0
[INFO] BUILD FAILURE
```
```
$ cd server && mvn -B -ntp test -Dtest=BrokerTopologyGuardsTest,RocketMQMessageProviderTest,RocketMQDLQProviderTest
[INFO] You have 0 Checkstyle violations.
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.provider.apache.BrokerTopologyGuardsTest
[INFO] Tests run: 39, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.provider.apache.RocketMQDLQProviderTest
[INFO] Tests run: 54, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.provider.apache.RocketMQMessageProviderTest
[INFO] Tests run: 97, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
The test drives the guard through its public entry points with a fixture `ClusterInfo` (a broker
registered as `localhost:10911`): the offset id built from the resolved address is accepted and
`validatedBrokerAddr` returns that numeric address, while an address outside the topology, a
non-matching port and an unresolvable hostname are still rejected. `RocketMQMessageProviderTest` and
`RocketMQDLQProviderTest` cover the two consumers of the guard and stay green.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added or updated for non-trivial changes, test methods named `...Test`
- [x] New UI text has both Chinese and English entries under `web/src/i18n/` (no UI text changed)
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit checks)
- [x] New source files carry the ASF license header
- [x] Documentation touched where behaviour changed (no user-facing docs for this guard)
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(message): resolve hostname-registered brokers in the offset msgId guard [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4683:
URL: https://github.com/apache/rocketmq-dashboard/pull/4683#issuecomment-6049518952
This pull request has been automatically closed because there has been no activity for 7 days. If you would like to continue working on this, please reopen the PR with updated changes.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(message): resolve hostname-registered brokers in the offset msgId guard [rocketmq-dashboard]
Posted by "btlqql (via GitHub)" <gi...@apache.org>.
btlqql commented on PR #4683:
URL: https://github.com/apache/rocketmq-dashboard/pull/4683#issuecomment-5762421267
Reworked onto `4c697f07`. Blocking item 2 answered with option (b), and the "at most once per call" half of option (a) as well.
**The seam.** Resolution now goes through `BrokerHostResolver`, a single-method interface (`DEFAULT = InetAddress::getByName`), plus `BrokerHostResolver.caching(delegate)` — a memoizing wrapper that remembers failures as well as successes, so an unresolvable registered host is not retried either. `RocketMQMessageProvider.perQueryHostResolver()` and the equivalent in `RocketMQDLQProvider` build one caching resolver per query/resend, which is the exact case you named: one `queryByMsgId` evaluates the guard twice (`:150` before `viewMessage`, then the decoded-offset fallback), and now the second evaluation compares instead of resolving again. Both entry points take the resolver as a parameter, so tests inject a fake and **count** lookups — the bound is asserted, not described.
**The javadoc** states both requested things: the accepted set widens from "the literal registered string" to "whatever the registered host currently resolves to", and the expected DNS cost with its bound. It also says plain what you flagged — the platform lookup has no wall-clock timeout of its own, since `InetAddress` exposes no knob and the JVM/OS resolver decides — which is the second reason the seam exists: a deployment wanting a hard bound or a cross-request cache supplies its own resolver without touching the guard. Failure direction is unchanged: an unresolvable registered host matches nothing, so the id stays rejected.
**Nice-to-have:** `rejectsAnOffsetIdWhenTheRegisteredHostnameCannotBeResolved` no longer does real DNS — the class now uses `FakeBrokerHostResolver` throughout, so no test in it touches the network.
**Mutation check** — resolved-address fallback removed (pre-fix exact-string behaviour), tests untouched:
```
[ERROR] Tests run: 110, Failures: 6, Errors: 0, Skipped: 0
[ERROR] BrokerTopologyGuardsTest.acceptsAnOffsetIdWhoseBrokerIsRegisteredWithAHostnameTest:51
[ERROR] BrokerTopologyGuardsTest.acceptsAnIdWhoseAddressMatchesOneOfSeveralRegisteredEndpointsTest:64
[ERROR] BrokerTopologyGuardsTest.rejectsAnOffsetIdWhenTheRegisteredHostnameCannotBeResolvedTest:116
[ERROR] BrokerTopologyGuardsTest.reusesOneResolutionAcrossRepeatedEvaluationsOfTheSameGuardTest:76
[ERROR] RocketMQMessageProviderTest.queryByMsgIdAcceptsAHostnameRegisteredBrokerAndLooksItUpOnceTest:524
[ERROR] RocketMQDLQProviderTest.resendSelectedMessagesLooksUpAHostnameRegisteredBrokerOnceForAllSelectedIdsTest:454
```
Whole suite on a host with MySQL 8 reachable: 3160 tests, 0 failures, 0 errors (3145 on the untouched head).
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(message): resolve hostname-registered brokers in the offset msgId guard [rocketmq-dashboard]
Posted by "yyqdbngt (via GitHub)" <gi...@apache.org>.
yyqdbngt commented on PR #4683:
URL: https://github.com/apache/rocketmq-dashboard/pull/4683#issuecomment-5911729543
Blocking item 2 is answered with option (b) on the current head: the `BrokerHostResolver` seam with a `caching` wrapper built per query (failures memoized as well), lookup counts asserted in the tests, and the javadoc stating the widened accepted set and the expected DNS cost with its bound. @lizhimins a re-review when convenient please.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(message): resolve hostname-registered brokers in the offset msgId guard [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4683: fix(message): resolve hostname-registered brokers in the offset msgId guard
URL: https://github.com/apache/rocketmq-dashboard/pull/4683
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org