unbridled-41 opened a new pull request, #5930:
URL: https://github.com/apache/rocketmq-dashboard/pull/5930
### Which Issue(s) This PR Fixes
Fixes #5929
### Problem / Evidence
`createConsumerGroup` writes a fresh config with the creation defaults for
every non-import call (`importing` is only true for the import endpoints):
```java
// before
config.setConsumeEnable(true); config.setConsumeBroadcastEnable(true);
config.setRetryQueueNums(1);
...
boolean physicalExists = importing && verifyGroupImport(admin, brokerAddrs,
config);
if (!physicalExists) { for (String addr : brokerAddrs) {
admin.createAndUpdateSubscriptionGroupConfig(addr, config); } }
```
so calling create for a group that already exists on that broker reset
`retryQueueNums` to 1, re-enabled consumption and broadcast consumption, and
turned ordered consumption back to concurrent. Reachable from a resubmitted
create form, and by design from the AI upsert, which calls create whenever the
metadata table has no row - including for a physically existing group that was
never imported.
```
RocketMQAdminClientImplTest#createConsumerGroupKeepsTheExistingBrokerSettingsOfAnAlreadyPresentGroupTest
expected: 5
but was: 1
```
The update path already does the opposite and documents it: "Read every
broker before writing so missing or unreadable configurations cannot cause a
partial update or fall back to creation defaults" / "Preserve each broker's
other settings; zero is an explicit retry limit".
### Root cause / Fix
The create path had no notion of "already there". Pre-read the master's
config, apply the retry limit and - only when the request names a delivery
order type - the ordered flag, and keep everything else; absent groups and
absent configs keep the creation defaults. The import path is unchanged.
### Priority and scoring
**PRIORITY 52** — impact 20/40 (silent reset of a live group's consumption
switches and retry queues, i.e. a disabled group starts consuming again), blast
radius 10/20 (any re-create of an existing group), reproducibility 18/20
(deterministic broker interaction, pinned by the new test), maintenance value
8/20 (aligns two paths that should agree).
**FIX_CONFIDENCE 80** — the pattern already exists in `updateConsumerGroup`;
the ordering of the read is the only new logic.
### Tests
`cd server && mvn -o -B -ntp test -Dtest='RocketMQAdminClientImplTest'` ->
`Tests run: 81, Failures: 0, Errors: 0`.
| Test | Before | After |
|---|---|---|
|
`createConsumerGroupKeepsTheExistingBrokerSettingsOfAnAlreadyPresentGroupTest`
| FAIL (`retryQueueNums` expected 5, got 1) | PASS |
The existing ordered/unordered create cases
(`createConsumerGroupPropagatesAnOrderedDeliveryOrderTypeToTheBrokerTest`,
`createConsumerGroupKeepsAnUnorderedDeliveryOrderTypeConcurrentTest`) still
pass - they cover the absent-group path. `checkstyle:check` passes.
### Risk
The create path now performs one extra `examineSubscriptionGroupConfig` per
master; a master that cannot be read falls back to the creation defaults
exactly as before, and the import path keeps its own semantics.
--
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]
