unbridled-41 opened a new pull request, #11044: URL: https://github.com/apache/rocketmq/pull/11044
### Problem / Evidence Since `BatchUnregistrationService`, the decision to unregister a broker (heartbeat expiry in `scanNotActiveBroker`, channel-close events) and its execution are separated by queue latency and blocking `closeChannel` I/O. The decision (`setupUnRegisterRequest`) matches only by `clusterName + brokerAddr`, and `unRegisterBroker` removes unconditionally: - `brokerLiveTable.remove(brokerAddrInfo)` — no freshness re-check; - the addr mapping via `removeIf(item -> item.getValue().equals(brokerAddr))` — by address only, ignoring the `brokerId` in the request (while `registerBroker` distinguishes ids for the same address); - and all topic QueueDatas for the broker. If the broker is alive and re-registers between the expiry decision and the queued execution (namesrv GC pause / scan backlog that transiently expires live brokers), the brand-new registration is deleted and the broker vanishes from all routes until its next periodic re-registration (`registerNameServerPeriod` ≈ 30s) — cluster-wide TOPIC_NOT_EXIST / no-route windows. The address-only removal also lets a queued slave unregister wipe a master re-registered at the same address. Deterministic regression test `RouteInfoManagerNewTest#testStaleChannelDestroyDoesNotWipeFreshReRegistration`: register (channel1) → re-register (channel2) → execute the destroy request derived from channel1 → the fresh registration must survive. Before the fix it is removed (`pickupTopicRouteData` returns null; verified against ff8f6f74c with the equivalent pre-fix execution path — see Tests). ### Root cause / Fix Pair each queued unregister request with the channel whose destruction produced it (`BrokerUnregistration`), and inside the write-locked removal skip requests whose current live entry belongs to a newer channel. This mirrors the channel-identity guard that `onChannelDestroy(Channel)` already applies at decision time, but closes the decision-to-execution gap. Explicitly initiated unregisters (the broker's own UNREGISTER_BROKER request, and `RouteInfoManager#unregisterBroker`) keep their unconditional behavior (`expectedChannel == null`). ### Priority PRIORITY = 72：影响 30（活 broker 从路由中消失最长 30s，全集群对该 broker 的路由/写入失败——GC 停顿或扫描积压后可触发）+ 波及范围 10（namesrv 注销路径）+ 可复现性 18（确定性单元测试复现完整交错）+ 维护价值 14（与既有 channel 身份校验模式一致）。FIX_CONFIDENCE = 82（判据明确：channel 身份不变性；broker 主动注销路径保持无条件）。 ### Tests - `mvn -pl namesrv test -Dtest=RouteInfoManagerNewTest`: `Tests run: 32, Failures: 0, Errors: 0`（含 3 个新测试：过期销毁跳过、匹配销毁仍删除、broker 主动注销无条件） - 修复前实测（ff8f6f74c + 等价公共 API 执行路径）：stale destroy 场景 `AssertionError: fresh registration must survive the stale destroy request` - `mvn -pl namesrv test -Dtest=RouteInfoManagerTest,RouteInfoManagerBrokerRegisterTest,RouteInfoManagerStaticRegisterTest`: 23/23 ### Risk Low-to-moderate. The skip only triggers when a queued destroy request refers to a live entry whose channel has been replaced — i.e. exactly the stale-event case; a same-channel destroy (the normal case, including the existing await-drain tests) behaves as before. The batch queue element type changes from `UnRegisterBrokerRequestHeader` to the internal `BrokerUnregistration` wrapper; the public `submitUnRegisterBrokerRequest(header)` / `unRegisterBroker(Set<header>)` signatures used by `DefaultRequestProcessor` and existing tests are unchanged. If a broker genuinely left while its replacement channel exists (re-registration with the same addr), the next heartbeat expiry scan will re-emit a destroy event for the actual channel, so no permanent leak. -- 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]
