wang-jiahua opened a new issue, #10446:
URL: https://github.com/apache/rocketmq/issues/10446
### Before Creating the Enhancement Request
- [x] I have confirmed that this should be classified as an enhancement
rather than a bug/feature.
### Summary
Today `AdminBrokerProcessor` only exposes single-target deletion APIs and
decorates `deleteTopic` with `synchronized`, which forces concurrent deletes to
run one after another. When a large number of topic / consumer-group metadata
entries need to be cleaned up, this causes admin-request backlog and N
persistence I/O calls.
This issue proposes:
1. Removing the `synchronized` keyword from
`AdminBrokerProcessor#deleteTopic`, since the underlying `topicConfigTable` is
already a `ConcurrentHashMap`.
2. Adding two new batch APIs 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`
### Motivation
- The current `deleteTopic` method is `synchronized` (see
[AdminBrokerProcessor.java#L761](https://github.com/apache/rocketmq/blob/82a6a784ee35b36d94b3ab644bd5394834e2c540/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java#L761)).
Concurrent admin requests targeting *different* topics are forced to run one
after another on the admin executor, although the underlying `topicConfigTable`
(`ConcurrentHashMap`) already provides thread-safe single-key removal.
- Single-item `DELETE_TOPIC_IN_BROKER` (see
[AdminBrokerProcessor.java#L272](https://github.com/apache/rocketmq/blob/82a6a784ee35b36d94b3ab644bd5394834e2c540/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java#L272))
and `DELETE_SUBSCRIPTIONGROUP` require N RPCs to delete N items and trigger N
`persist()` calls on the on-disk config files.
In metadata cleanup scenarios (e.g. tenant decommissioning, batch retention
purge, cluster migration) this leads to user-visible slow admin operations and
unnecessary disk I/O.
### Describe the Solution You'd Like
1. **Remove `synchronized` from `AdminBrokerProcessor#deleteTopic`.**
Mutations happen on `topicConfigTable` (`ConcurrentHashMap`), so atomicity per
key is already guaranteed. Pop-retry collection is read-only on concurrent-safe
structures.
2. **Add two new request codes** in `RequestCode`:
- `DELETE_TOPIC_IN_BROKER_LIST`
- `DELETE_SUBSCRIPTIONGROUP_LIST`
3. **Add corresponding request bodies**:
- `DeleteTopicListRequestBody { List<String> topicList }`
- `DeleteSubscriptionGroupListRequestBody { List<String> groupNameList;
boolean cleanOffset }`
4. **Add batch entry points** in `AdminBrokerProcessor`:
- `deleteTopicList(...)` — validates each topic, dedupes (preserving
input order), optionally collects pop-retry topics, then calls
`TopicConfigManager#deleteTopicConfigList(...)` with **one** `persist()` for
the entire batch.
- `deleteSubscriptionGroupList(...)` — same pattern, calling
`SubscriptionGroupManager#deleteSubscriptionGroupConfigList(...)`.
5. **Aggregate `persist()` in manager-level helpers**:
- `TopicConfigManager#deleteTopicConfigList(List<String>)`
-
`SubscriptionGroupManager#deleteSubscriptionGroupConfigList(List<String>)`
6. **Add client-side helpers** in `MQClientAPIImpl`:
- `deleteTopicInBrokerList(addr, topicList, timeout)`
- `deleteSubscriptionGroupList(addr, groupNameList, removeOffset,
timeout)`
### Describe Alternatives You've Considered
- **`ReadWriteLock` instead of `synchronized`**: not necessary since
`topicConfigTable`/`subscriptionGroupTable` are already `ConcurrentHashMap`,
and `deleteTopicConfig` is a single-key remove plus a `persist()`.
- **Async dispatch**: changing the contract to async would break callers
that rely on synchronous success/failure semantics. Synchronous batch keeps the
existing behavior.
- **Keep N RPCs but parallelize on the client side**
(`DefaultMQAdminExtImpl#deleteTopicInBrokerConcurrent` already exists): still
triggers N `persist()` calls per broker, doesn't address the persistence
amplification on the broker side.
### Additional Context
A PR will follow once this issue is created. Unit tests are added in
`AdminBrokerProcessorTest`:
- `testDeleteTopicListInBroker`
- `testDeleteTopicListBatchPersist` (verifies single `persist()` and dedup)
- `testDeleteSubscriptionGroupList`
--
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]
