unbridled-41 commented on PR #5930:
URL:
https://github.com/apache/rocketmq-dashboard/pull/5930#issuecomment-6091380320
### Follow-up review of this fix: the import path must not apply the
creation defaults
Reviewing the first commit I found it changed the behaviour of the
**import** path, not just the create path. `createConsumerGroup(admin, group,
cluster, importing)` was applying `createConfigForMaster(...)` — the creation
defaults (queue counts, permission, retention...) — to *every* broker also when
`importing` was true. An import is documented to register a group that already
matches and to refuse to overwrite a different config (`verifyGroupImport`
throws 409); writing the defaults first meant an import of an existing,
differently-configured group silently rewrote its live settings, and the 409
guard never got to run.
Commit `039a0309` restores that contract in this branch (first commit
`8d2b1ebe` untouched, no force-push):
```java
if (importing) {
// An import only registers a group that already matches and refuses to
overwrite a
// different config (verifyGroupImport throws 409); it never applies the
creation
// defaults to a live group.
if (!verifyGroupImport(admin, brokerAddrs, config)) {
for (String addr : brokerAddrs) {
admin.createAndUpdateSubscriptionGroupConfig(addr, config);
}
}
} else {
for (String addr : brokerAddrs) {
admin.createAndUpdateSubscriptionGroupConfig(addr,
createConfigForMaster(admin, addr, config, group));
}
}
```
Two tests pin it in `RocketMQAdminClientImplTest`:
`importConsumerGroupShouldNotRewriteAnExistingMatchingGroupTest` (a matching
live group is left alone) and
`importConsumerGroupShouldRefuseToOverwriteADifferentConfigTest` (an import
over a different config still fails with the existing 409).
Verified at the current head (`039a0309`):
```
cd server && mvn -o -B -ntp test -Dtest='RocketMQAdminClientImplTest'
Tests run: 83, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
```
Branch `fix/group-create-preserve-settings`, HEAD `039a0309` (on top of
`8d2b1ebe`).
--
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]
