[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