[ISSUE #10446] Support batch deletion of topics and subscription groups in broker

[wang-jiahua]

Which Issue(s) This PR Fixes

Fixes #10446

Brief Description

Adds two new batch admin APIs to AdminBrokerProcessor whose names follow the existing *_LIST convention (e.g. UPDATE_AND_CREATE_TOPIC_LIST, UPDATE_AND_CREATE_SUBSCRIPTIONGROUP_LIST):

• DELETE_TOPIC_IN_BROKER_LIST
• DELETE_SUBSCRIPTIONGROUP_LIST

Bulk metadata cleanup (e.g. tenant decommissioning, retention purge, cluster migration) currently requires N round-trips to delete N items and triggers N persist() calls on the broker config files. These new APIs allow doing the same in 1 RPC + 1 persist + 1 messageStore.deleteTopics(Set) call.

Note: removing the synchronized modifier from AdminBrokerProcessor#deleteTopic is out of scope here and is already being discussed in #9997 / #9998. This PR keeps synchronized as-is for both the existing deleteTopic and the new deleteTopicList, so the lock semantics match the rest of the existing updateAndCreate* family.

Changes

• RequestCode: add DELETE_TOPIC_IN_BROKER_LIST and DELETE_SUBSCRIPTIONGROUP_LIST.
• New request bodies:
  • DeleteTopicListRequestBody { List<String> topicList }
  • DeleteSubscriptionGroupListRequestBody { List<String> groupNameList; boolean cleanOffset }
• TopicConfigManager#deleteTopicConfigList(List<String>) and SubscriptionGroupManager#deleteSubscriptionGroupConfigList(List<String>): aggregate persist() once per batch.
• AdminBrokerProcessor:
  • Extract collectPopRetryTopics(...) shared helper used by both single and batch paths.
  • Add deleteTopicList(...) and deleteSubscriptionGroupList(...) (both synchronized, consistent with existing single APIs) with input dedup (preserving order), system-topic / blank validation, one-shot persist, and one-shot messageStore.deleteTopics(...) call.
• MQClientAPIImpl: add deleteTopicInBrokerList(...) and deleteSubscriptionGroupList(...) client helpers; both use an explicit null check (instead of assert) so callers get a deterministic MQClientException(SYSTEM_ERROR, ...) when invokeSync returns null.

Compatibility

• Single-item DELETE_TOPIC_IN_BROKER / DELETE_SUBSCRIPTIONGROUP request codes and methods are unchanged; existing clients are not affected.
• New *_LIST request codes are additive; older brokers will fall through to getUnknownCmdResponse, so older clients/brokers continue to work as today.

How Did You Test This Change?

New unit tests added in AdminBrokerProcessorTest:

• testDeleteTopicListInBroker — covers empty input, system-topic rejection, and the success path.
• testDeleteTopicListBatchPersist — verifies that:
  • the manager-level deleteTopicConfigList is invoked once (so persist() runs once for the whole batch),
  • the singular deleteTopicConfig is not called on the batch path,
  • messageStore.deleteTopics(...) is invoked once with the batched set,
  • duplicate inputs are deduplicated.
• testDeleteSubscriptionGroupList — covers empty input and the success path with cleanOffset=true.

Local verification:

mvn -pl remoting,client,broker -Dspotbugs.skip=true validate
# 0 Checkstyle violations across all three modules

mvn -pl broker -Dcheckstyle.skip -Dspotbugs.skip=true -Djacoco.skip=true \
  -Dtest='AdminBrokerProcessorTest#testDeleteTopic+testDeleteTopicListInBroker+testDeleteTopicListBatchPersist+testDeleteSubscriptionGroup+testDeleteSubscriptionGroupList+testDeleteWithPopRetryTopic' \
  test
# Tests run: 6, Failures: 0, Errors: 0, Skipped: 0

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds broker/admin support for batch deletion of topics and subscription groups via new remoting request codes and request bodies, plus client APIs and broker-side handling.

Changes:

• Introduce new request bodies and request codes for batch-delete operations.
• Implement broker processor logic to delete topic/group lists (including dedup + optional cleanup behaviors).
• Add persistence helpers for batch config deletion and tests covering the new admin paths.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/body/DeleteTopicListRequestBody.java	Adds request body for batch topic deletion.
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/body/DeleteSubscriptionGroupListRequestBody.java	Adds request body for batch subscription-group deletion (with cleanOffset flag).
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RequestCode.java	Adds new request codes for batch delete operations.
client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java	Adds client APIs to invoke the new batch delete broker requests.
broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java	Adds unit tests for batch deletion request handling and persistence behavior.
broker/src/main/java/org/apache/rocketmq/broker/topic/TopicConfigManager.java	Adds batch topic-config deletion with single persist/update.
broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java	Adds batch subscription-group deletion with single persist/update.
broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java	Routes new request codes and implements batch delete handlers; refactors pop retry topic collection.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[oss-sentinel-ai]

Review by github-manager-bot

Summary

This PR addresses two related performance issues in AdminBrokerProcessor:

1. Removes synchronized from deleteTopic since topicConfigTable is already a ConcurrentHashMap
2. Adds batch deletion APIs (DELETE_TOPIC_IN_BROKER_LIST and DELETE_SUBSCRIPTIONGROUP_LIST) to reduce network round-trips

Findings

• [Good] AdminBrokerProcessor.java:769 — Removed synchronized from deleteTopic. The underlying topicConfigTable is a ConcurrentHashMap, so per-key remove is atomic. This eliminates unnecessary serialization of concurrent admin delete requests.
• [Good] AdminBrokerProcessor.java — Added deleteTopicList() and deleteSubscriptionGroupList() handlers following the existing *_LIST convention (consistent with UPDATE_AND_CREATE_TOPIC_LIST).
• [Good] DeleteTopicListRequestBody.java / DeleteSubscriptionGroupListRequestBody.java — Clean request body classes extending RemotingSerializable, following existing patterns.
• [Good] MQClientAPIImpl.java:2202-2220 / 2281-2301 — Client-side batch deletion methods properly implemented with timeout handling.
• [Good] RequestCode.java — Added DELETE_TOPIC_IN_BROKER_LIST and DELETE_SUBSCRIPTIONGROUP_LIST request codes.
• [Good] TopicConfigManager.java / SubscriptionGroupManager.java — Added deleteTopicList() and deleteSubscriptionGroupList() batch methods.
• [Good] AdminBrokerProcessorTest.java — Comprehensive test coverage including:
  • Empty list validation (returns INVALID_PARAMETER)
  • System topic rejection in batch
  • Happy path with multiple topics
  • Duplicate handling in batch
  • Batch persist verification

Analysis

The implementation is well-structured:

1. Backward compatibility: Existing single-item deletion APIs remain unchanged
2. Consistent patterns: Follows the *_LIST convention established by CreateTopicListRequestBody
3. Proper validation: Empty lists and system topics are rejected
4. Test coverage: Edge cases are well covered

Suggestions

1. Consider adding a maximum batch size limit to prevent excessive memory usage or long-running operations.
2. The batch persist test verifies deduplication — good attention to edge cases.

Verdict

✅ Approved — Well-designed performance optimization with proper validation and comprehensive test coverage.

---

Automated review by github-manager-bot

[codecov-commenter]

Codecov Report

❌ Patch coverage is 64.24242% with 59 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.03%. Comparing base (82a6a78) to head (9afd46f).
⚠️ Report is 9 commits behind head on develop.

Files with missing lines	Patch %	Lines
...ocketmq/broker/processor/AdminBrokerProcessor.java	75.26%	12 Missing and 11 partials ⚠️
...g/apache/rocketmq/client/impl/MQClientAPIImpl.java	0.00%	18 Missing ⚠️
...l/body/DeleteSubscriptionGroupListRequestBody.java	56.25%	7 Missing ⚠️
...on/builder/DefaultAuthorizationContextBuilder.java	77.77%	1 Missing and 3 partials ⚠️
...ting/protocol/body/DeleteTopicListRequestBody.java	62.50%	3 Missing ⚠️
.../broker/subscription/SubscriptionGroupManager.java	66.66%	1 Missing and 1 partial ⚠️
...ache/rocketmq/broker/topic/TopicConfigManager.java	66.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10448      +/-   ##
=============================================
- Coverage      48.06%   48.03%   -0.03%     
- Complexity     13313    13327      +14     
=============================================
  Files           1377     1379       +2     
  Lines         100632   100787     +155     
  Branches       12995    13023      +28     
=============================================
+ Hits           48369    48417      +48     
- Misses         46344    46410      +66     
- Partials        5919     5960      +41     

☔ 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 batch deletion APIs for topics (DELETE_TOPIC_IN_BROKER_LIST) and subscription groups (DELETE_SUBSCRIPTIONGROUP_LIST), with proper per-resource authorization, input validation, and atomic-all-or-nothing semantics.

Findings

• [Info] Authorization — Correctly emits one DELETE context per resource in the batch, preventing bypass when the body carries the target list instead of the header. This is a security-critical detail handled well.
• [Info] Error handling — Returns a structured error response with the list of failed items and the first exception. Good for debugging partial failures.
• [Info] Dedup — Uses LinkedHashSet to preserve insertion order while deduplicating. Reasonable choice.
• [Info] Refactoring — Extracts collectPopRetryTopics() from inline code in deleteTopic(). Good cleanup that avoids duplication.
• [Info] Cleanup order — Deletes client config and subscription first, then topic config last. Allows retry after partial failure.
• [Warning] AdminBrokerProcessor.deleteTopicInBrokerList() — The RemotingCommand response is created once at the top and reused. If deleteTopicInBroker internally sets response fields, ensure there's no state leakage between the success and error paths.
• [Warning] MQClientAPIImpl.deleteTopicInBrokerList() — The method creates the request body and sends it. Consider adding a timeout or batch size limit to prevent excessively large requests.

Suggestions

• Consider adding a max batch size constant (e.g., 100) to prevent abuse or accidental large requests.
• The DeleteTopicListRequestBody and DeleteSubscriptionGroupListRequestBody classes are clean. Consider adding @AllArgsConstructor for convenience if not already present.
• Cross-repo: If batch deletion is exposed in the dashboard or CLI, ensure those clients are updated to use the new APIs.

Overall: Well-structured feature with proper authorization and error handling. A few minor suggestions but nothing blocking.

---

Automated review by github-manager-bot