You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "zjncs (via GitHub)" <gi...@apache.org> on 2026/09/08 03:55:31 UTC

[PR] fix(group): allow zero retry settings on update [rocketmq-dashboard]

zjncs opened a new pull request, #4123:
URL: https://github.com/apache/rocketmq-dashboard/pull/4123

   ### Motivation
   
   `UpdateConsumerGroupSettingsDTO` validates both retry fields with `@Positive`:
   
   ```java
   @NotNull(message = "retryQueueNums is required")
   @Positive(message = "retryQueueNums must be positive")
   private Integer retryQueueNums;
   @NotNull(message = "retryMaxTimes is required")
   @Positive(message = "retryMaxTimes must be positive")
   private Integer retryMaxTimes;
   ```
   
   But `0` is a legitimate broker-side value — `RocketMQAdminClientImpl` forwards these straight into `SubscriptionGroupConfig.setRetryQueueNums/setRetryMaxTimes`, where `retryQueueNums=0` disables retry queues and `retryMaxTimes=0` sends failed messages directly to the DLQ. The sibling `CreateConsumerGroupDTO` already accepts it:
   
   ```java
   @PositiveOrZero(message = "retryMaxTimes must be zero or positive")
   private Integer retryMaxTimes;
   ```
   
   Concrete failure: a group created via `POST /api/groups/create` with `retryMaxTimes: 0` (accepted) can never have its settings re-saved via the settings endpoint while keeping that value — the request fails bean validation with a 400 before reaching `MetadataService`.
   
   ### Modifications
   
   Both fields switch to `@PositiveOrZero` with matching messages, aligning the update path with the create path and the broker semantics. Negative values are still rejected.
   
   ### Verification
   
   New standalone `UpdateConsumerGroupSettingsDTOTest` (the controller test file is under review in another PR, so the validation is pinned in its own class, modeled on `AlertRuleRequestDTOTest`):
   - `shouldAcceptZeroRetrySettingsTest` — before the fix: fails with `retryQueueNums must be positive` / `retryMaxTimes must be positive`; after: passes.
   - `shouldRejectNegativeRetrySettingsTest` — negative values still produce the two violations.
   - Regression: `mvn -f server/pom.xml test -Dtest='UpdateConsumerGroupSettingsDTOTest,ConsumerGroupControllerTest,MetadataServiceTest'` → **Tests run: 59, Failures: 0, Errors: 0**.
   


-- 
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(group): allow zero retry settings on update [rocketmq-dashboard]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins merged PR #4123:
URL: https://github.com/apache/rocketmq-dashboard/pull/4123


-- 
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