unbridled-41 opened a new pull request, #5924:
URL: https://github.com/apache/rocketmq-dashboard/pull/5924
### Which Issue(s) This PR Fixes
Fixes #5923
### Problem / Evidence
`verifyTopicImport` answered one boolean for the whole cluster (`found`
became true as soon as any master matched) and the caller used it as "nothing
to do", so an import wrote nothing when the resource existed on some masters
but not others:
```java
// RocketMQAdminClientImpl (before)
boolean physicalExists = importing && verifyTopicImport(admin, brokerAddrs,
topicConfig);
if (!physicalExists) { for (String addr : brokerAddrs) {
admin.createAndUpdateTopicConfig(addr, topicConfig); } }
```
```
RocketMQAdminClientImplTest#importTopicCreatesTheTopicOnTheMastersThatLackItTest
Wanted but not invoked:
adminExt.createAndUpdateTopicConfig("10.0.0.2:10911", ...);
```
That is the state after a broker group is added to a cluster: the operator
re-imports the exported inventory to propagate the topics, every row already
exists on the old group, nothing is created on the new one, the row is still
marked ACTIVE and the endpoint reports success - and with
`autoCreateTopicEnable` (the RocketMQ default) the new broker serves an
auto-created topic with factory defaults instead of the imported
queues/perm/type. `verifyGroupImport` had the same shape for consumer groups.
### Root cause / Fix
Per-master state reduced to one boolean. Return the set of masters that do
not have a matching config yet and create exactly there; a config that differs
still refuses with 409, so "only register, never overwrite" is preserved for
the import path.
### Priority and scoring
**PRIORITY 58** — impact 24/40 (a silent partial configuration of the
cluster, reported as success, discovered later as lost configuration), blast
radius 14/20 (every import after a broker-group change), reproducibility 18/20
(deterministic broker interactions, pinned by the new tests), maintenance value
8/20.
**FIX_CONFIDENCE 82** — the topology set is already the loop's input; the
change keeps the conflict rule intact.
### Tests
`cd server && mvn -o -B -ntp test -Dtest='RocketMQAdminClientImplTest'` ->
`Tests run: 82, Failures: 0, Errors: 0`.
| Test | Before | After |
|---|---|---|
| `importTopicCreatesTheTopicOnTheMastersThatLackItTest` | FAIL (`Wanted but
not invoked`) | PASS |
| `importConsumerGroupCreatesTheGroupOnTheMastersThatLackItTest` | FAIL
(`Wanted but not invoked`) | PASS |
Each new case also asserts the master that already has a matching config is
not rewritten. `mvn -o -B -ntp test
-Dtest='RocketMQMetadataProviderTest,MetadataServiceTest'` -> `Tests run: 107,
Failures: 0, Errors: 0`; `checkstyle:check` passes.
### Risk
An import now performs the create RPC on the masters that lack the resource
(previously none), which is what the endpoint documents; a group/topic that
exists with a different config still fails with 409 before anything is written.
--
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]
