wang-jiahua opened a new pull request, #10448:
URL: https://github.com/apache/rocketmq/pull/10448
<!-- Please make sure the target branch is right. In most case, the target
branch should be `develop`. -->
### Which Issue(s) This PR Fixes
Fixes #10446
### Brief Description
This PR addresses two related problems in `AdminBrokerProcessor`:
1. **Concurrent admin requests are serialized.**
`AdminBrokerProcessor#deleteTopic` is decorated with `synchronized`, but the
underlying `topicConfigTable` is already a `ConcurrentHashMap` and the actual
mutation (`remove`) is atomic per key. The keyword forces concurrent deletes to
run one after another on the admin executor, causing metadata-deletion backlog
under load. This PR removes the redundant `synchronized`.
2. **Single-target deletion APIs cause N RPCs and N persists.** Bulk
metadata cleanup (e.g. tenant decommissioning, retention purge) currently
requires N round-trips and triggers N `persist()` calls on the broker config
files. This PR adds two batch APIs whose names follow the existing `*_LIST`
convention (`UPDATE_AND_CREATE_TOPIC_LIST`,
`UPDATE_AND_CREATE_SUBSCRIPTIONGROUP_LIST`):
- `DELETE_TOPIC_IN_BROKER_LIST`
- `DELETE_SUBSCRIPTIONGROUP_LIST`
#### 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`:
- Remove `synchronized` from `deleteTopic`.
- Extract `collectPopRetryTopics(...)` shared helper.
- Add `deleteTopicList(...)` and `deleteSubscriptionGroupList(...)` with
input dedup (preserving order), system-topic / blank validation, and one-shot
persist + one-shot `messageStore.deleteTopics(...)` call.
- `MQClientAPIImpl`: add `deleteTopicInBrokerList(...)` and
`deleteSubscriptionGroupList(...)` client helpers.
#### 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.
- The behavior of `deleteTopic` after removing `synchronized` is preserved
per key, since `topicConfigTable.remove` is atomic and `persist()` is
internally serialized.
### 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
```
--
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]
