[Bug] Controller: a stale channel-close event evicts a re-registered broker and triggers a spurious master election
- 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 a question.
- [x] I searched open GitHub Issues and pull requests and found no duplicate.
- [x] I confirmed that this bug belongs to Apache RocketMQ.
### Runtime platform environment
All platforms; reproduced with a deterministic unit test on the current `develop` branch.
### RocketMQ version
`develop` at `ff8f6f74c`
### JDK Version
All
### Describe the Bug
`DefaultBrokerHeartbeatManager#onBrokerHeartbeat` never rebinds the channel of an existing `BrokerLiveInfo` (the `prev != null` branch only refreshes the timestamp/timeout/priority/epoch), so the live entry stays pinned to the broker's **first-ever** channel. When that old channel eventually fires `channelInactive` / `onChannelException` / `onChannelIdle`, `onBrokerChannelClose` matches it against the stale stored channel, removes the live entry of a broker that is healthy on a new channel, and fires the broker-inactive lifecycle event, which makes `ControllerManager#onBrokerInactive` trigger a master election for a perfectly healthy master.
This is realistic in at least two ordinary situations:
1. The broker-to-controller connection dies half-open on the controller side; the broker client detects it first, reconnects, and keeps heartbeating on the new channel. Minutes later the controller's TCP stack learns the old channel is dead and fires the close event.
2. The controller-side idle/close event for the old channel is only delivered after the broker has already re-registered on a new channel.
### Steps to Reproduce
1. Broker B (master of its broker-set) sends a heartbeat to the controller on channel A → `BrokerLiveInfo` holds channel A.
2. The broker reconnects and keeps heartbeating on channel B (every heartbeat hits the `prev != null` branch, channel stays A).
3. Channel A fires `onBrokerChannelClose(A)` on the controller.
4. Observe that B's live entry is removed and `notifyBrokerInActive` fires although B is actively heartbeating on channel B; `ControllerManager#onBrokerInactive` then calls `triggerElectMaster` because the evicted broker is the current master.
### What Did You Expect to See?
A close event for a channel that the broker no longer uses must not evict the live entry: the heartbeat path should rebind `BrokerLiveInfo` to the current channel, so `onBrokerChannelClose` only removes entries whose stored channel is the one that actually closed.
### What Did You See Instead?
The live entry of the re-registered master is removed, a broker-inactive notification is fired, and the controller bumps the master epoch / runs a failover for a healthy broker-set.
### Additional Context
The sibling NameServer implementation (`RouteInfoManager`) updates the broker's channel on every heartbeat, so the controller behavior is inconsistent with the nameserver. The fix is to rebind the channel in the `prev != null` branch of `onBrokerHeartbeat` when a non-null channel arrives; a regression test in `DefaultBrokerHeartbeatManagerTest` reproduces the eviction deterministically.
### Corresponding PR
- PR apache/rocketmq#11046
Contributor guide
Research direction
Start with DefaultBrokerHeartbeatManager#onBrokerHeartbeat and onBrokerChannelClose, then read DefaultBrokerHeartbeatManagerTest. Reproduce the stale-channel eviction described in the issue and verify that the heartbeat path tracks the current channel, while a close event for the old channel no longer removes the live broker entry or triggers inactivity handling.
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
- 35/100