[f1amingo]

Which Issue(s) This PR Fixes

• Fixes [Enhancement] Make broker-side subscription group max length configurable #11466

Brief Description

Adds subscriptionGroupMaxLength to BrokerConfig, retaining the existing default of 120 and capping the effective limit at 255. Broker-side single, batch, and automatic group creation use the configured value, while client and gRPC validation remain unchanged.

Callers that raise the limit must keep generated retry topics within 255 characters. Groups over 122 characters cannot use %DLQ%<group> because broker writes reject topics over 127 bytes.

How Did You Test This Change?

Ran TopicValidatorTest, AdminBrokerProcessorTest, and SubscriptionGroupManagerTest with JDK 8; all 121 tests passed.

[RockteMQ-AI]

LGTM. Clean implementation that adds broker-side configurability for subscription group max length while preserving backward compatibility.

Summary

• Added subscriptionGroupMaxLength to BrokerConfig (default 120, capped at 255)
• Introduced overloaded TopicValidator.validateGroup(String, int) with defensive bounds checking
• Updated 3 broker-side call sites (single create, batch create, auto-create) to use configured value
• Comprehensive test coverage (121 tests total across 3 test classes)

Design Notes

The implementation correctly preserves backward compatibility:

• Default value (120) matches the existing GROUP_MAX_LENGTH constant
• Client-side and gRPC validation remain unchanged (still use hardcoded 120)
• Only broker-side operations are affected

The cap at 255 (RETRY_OR_DLQ_TOPIC_MAX_LENGTH) is appropriate because retry/DLQ topics are constructed as RETRY<group> and %DLQ%<group> (5 chars + group length). As noted in the PR description, groups exceeding 122 characters cannot use DLQ topics due to the 127-byte broker limit on topic names.

Minor Observation

The PR description mentions that callers must ensure retry topics stay within 255 characters when raising the limit, but the code doesn't enforce this automatically. This is a reasonable design choice (caller responsibility), but worth documenting in BrokerConfig Javadoc if not already present.

Test Coverage

• ✅ Boundary tests (0, negative, 255, >255 values)
• ✅ Integration tests (AdminBrokerProcessor, SubscriptionGroupManager)
• ✅ Validation logic tests (TopicValidator)

All tests pass on JDK 8 as reported. Ready to merge.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.74%. Comparing base (57c2dc0) to head (5a8c3ad).

Files with missing lines	Patch %	Lines
.../java/org/apache/rocketmq/common/BrokerConfig.java	25.00%	3 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #11467      +/-   ##
=============================================
+ Coverage      49.72%   49.74%   +0.01%     
- Complexity     14366    14376      +10     
=============================================
  Files           1390     1390              
  Lines         103264   103273       +9     
  Branches       13504    13505       +1     
=============================================
+ Hits           51351    51376      +25     
+ Misses         45707    45705       -2     
+ Partials        6206     6192      -14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Adds subscriptionGroupMaxLength to BrokerConfig (default 120, capped at 255), making the broker-side subscription group name length configurable. All broker-side validation points (single create, batch create, auto-create) use the configured value, while client and gRPC validation remain unchanged for backward compatibility.

Findings

• [Info] The cap at 255 (RETRY_OR_DLQ_TOPIC_MAX_LENGTH) is correct — it prevents users from configuring a value that would break retry/DLQ topic generation. The Javadoc clearly warns that groups over 122 characters cannot use %DLQ%<group> due to the 127-byte broker write limit.
• [Info] The fallback logic in validateGroup(group, maxLength) — non-positive values use the default, values above 255 are capped — is defensive and prevents misconfiguration.
• [Info] Batch creation correctly rejects the entire batch if any group exceeds the limit (testUpdateAndCreateSubscriptionGroupListRejectsWholeBatchWhenOneTooLong). This is the right behavior — partial creation would be confusing.
• [Info] Test coverage is comprehensive: configurable length, cap at 255, fallback to default, blank/illegal validation, auto-create, single/batch creation. The tests verify both the validation result and the remark message content.

Suggestions

• Consider logging a warning at broker startup if subscriptionGroupMaxLength is set above 122, reminding operators that DLQ topics for groups in the range [123, 255] will fail. This makes the constraint visible in logs rather than only in documentation.
• The validateGroup(group, maxLength) overload shadows the local variable groupMaxLength with the parameter. Consider renaming the parameter to configuredMaxLength for clarity, or use this.groupMaxLength to make the shadowing explicit.

Overall this is a solid enhancement with good defensive design and comprehensive testing.

---

Automated review by github-manager-bot