[Bug] Namesrv: a stale expiry/channel-close unregister wipes a broker's fresh re-registration
- Dominant language
- Java
- Stars
- 22.6k
- Forks
- 12k
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 27
Description
### Before Creating the Bug Report
- [X] I found a bug, not just asking a question, which should be created in [GitHub Discussions](https://github.com/apache/rocketmq/discussions).
- [X] I have searched the [GitHub Issues](https://github.com/apache/rocketmq/issues) and [GitHub Discussions](https://github.com/apache/rocketmq/discussions) of this repository and believe that this is not a duplicate.
- [X] I have confirmed that this bug belongs to the current repository, not other repositories of RocketMQ.
### Runtime platform environment
Linux, JDK 21, develop (ff8f6f74c)
### RocketMQ version
5.x develop
### Describe the Bug
Since the introduction of `BatchUnregistrationService`, the decision that a broker should be unregistered (heartbeat expiry in `scanNotActiveBroker`, or a channel-close event) and its execution are separated by queue latency and blocking `closeChannel` I/O. `setupUnRegisterRequest` matches only by `clusterName + brokerAddr` in `brokerAddrTable`, and `unRegisterBroker` then removes **unconditionally**:
- `brokerLiveTable.remove(brokerAddrInfo)` (no freshness re-check),
- the addr mapping via `removeIf(item -> item.getValue().equals(brokerAddr))` (matching by address only, ignoring the `brokerId` carried in the request — while `registerBroker` carefully distinguishes ids for the same address),
- and all topic QueueDatas for the broker.
If the broker is actually alive and re-registers between the expiry decision and the queued execution (e.g. after a namesrv GC pause / scan backlog that expired live brokers), the brand-new registration — new `BrokerLiveInfo`, address mapping and topic QueueDatas — is deleted. The live broker then vanishes from all routes until its next periodic re-registration (`registerNameServerPeriod`, ~30s), producing cluster-wide TOPIC_NOT_EXIST / no-route windows. The same defect lets a queued unregister for a slave at addr A remove a master entry re-registered at the same addr A, because removal matches on address only.
Note the `onChannelDestroy(Channel)` overload already guards by channel identity (`entry.getValue().getChannel() == channel`) at decision time — but the `BrokerAddrInfo` overload (used by the expiry scan) has no such check, and even the channel guard cannot cover the decision-to-execution gap.
### Steps to Reproduce
1. Register broker (channel1).
2. Fire the destroy event for it: `onChannelDestroy(new BrokerAddrInfo(cluster, addr))` — the unregister request is queued asynchronously.
3. Re-register the same broker with channel2 (fresh registration).
4. Let the queued request execute: the fresh registration is removed; `pickupTopicRouteData` returns null for its topics.
### Expected Behavior
A destroy-derived unregister must not remove a registration that is newer than the event it was derived from: if the current live entry for the address belongs to a different (newer) channel, the queued request is stale and must be skipped. Explicitly initiated unregisters (the broker's own UNREGISTER_BROKER request) remain unconditional.
### Corresponding PR
- Fix PR: #11044 (linked with `Closes #11043` in the PR description; contains the regression test that fails before the fix and passes after it).
Contributor guide
Research direction
Start with BatchUnregistrationService, scanNotActiveBroker, setupUnRegisterRequest, and unRegisterBroker, then review the linked PR #11044 and its regression test. Done means the stale queued unregister no longer removes a newer registration, while the regression test passes; the issue indicates this work is already under way.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100