[ISSUE #11043] Skip stale channel-destroy unregister when the broker re-registered

[unbridled-41]

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 ff8f6f7 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.

Closes #11043

[codecov-commenter]

Codecov Report

❌ Patch coverage is 93.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.74%. Comparing base (ff8f6f7) to head (6a2e768).

Files with missing lines	Patch %	Lines
...e/rocketmq/namesrv/routeinfo/RouteInfoManager.java	92.59%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #11044      +/-   ##
=============================================
- Coverage      48.86%   48.74%   -0.13%     
+ Complexity     13797    13762      -35     
=============================================
  Files           1381     1381              
  Lines         101574   101597      +23     
  Branches       13213    13216       +3     
=============================================
- Hits           49638    49519     -119     
- Misses         45924    46042     +118     
- Partials        6012     6036      +24     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[unbridled-41]

CI status: 9/10 checks pass. The single failure, maven-compile (windows-latest, JDK-8), is a timing flake in ConsumerOrderInfoManagerLockFreeNotifyTest#testConsumeTheChangeInvisibleLonger — ConditionTimeoutException: Condition ... was not fulfilled within 6 seconds (awaitility timeout on an AtomicBoolean, broker module). This PR only touches the namesrv module (RouteInfoManager/BatchUnregistrationService), which is not on the broker test classpath; the same full broker suite passed on windows-latest in the #11042 run ~25 minutes earlier. Recording as an unrelated baseline flake.

[unbridled-41]

Evidence chain (audit 2026-09-05):

Before the fix — ff8f6f7; the queued channel-destroy unregister was executed through the exact public path the service thread uses (unRegisterBroker(Set<UnRegisterBrokerRequestHeader>)), after register(channel1) → register(channel2) with the same address:

java.lang.AssertionError: fresh registration must survive the stale destroy request
    at ...ScratchPreFixCheckTest.preFix_staleDestroyWipesFreshRegistration

(harness was a scratch test not included in the commit; the committed regression tests exercise the same semantics deterministically via the post-fix package-private seam)

After the fix — branch tip 6a2e768, re-measured 2026-09-05:

mvn -pl namesrv test -Dtest=RouteInfoManagerNewTest
Tests run: 32, Failures: 0, Errors: 0, Skipped: 0
mvn -pl namesrv test -Dtest=RouteInfoManagerTest,RouteInfoManagerBrokerRegisterTest,RouteInfoManagerStaticRegisterTest
Tests run: 23, Failures: 0, Errors: 0, Skipped: 0

Three new tests pin the semantics: stale channel-destroy is skipped; same-channel destroy still removes; explicitly initiated unregister (null expected channel) stays unconditional.

Baseline noise note: running RouteInfoManagerNewTest on the unfixed baseline already logs 29 occurrences of "unregisterBroker Exception" / "Handle unregister broker request failed" (teardown/async noise measured via a stashed baseline run); the fixed run logs fewer of them with 0 test failures — the log noise is pre-existing, not introduced here.

CI: 9/10 checks pass. The single windows-latest failure (ConsumerOrderInfoManagerLockFreeNotifyTest awaitility 6s timeout, broker module) is unrelated to this namesrv-only change — see the earlier comment.

[RockteMQ-AI]

Summary

Introduces BrokerUnregistration to pair each unregister request with the channel that produced it, preventing a stale channel-destroy from wiping a fresh re-registration. The design is clean: the queue now carries BrokerUnregistration instead of the bare header, and doUnRegisterBroker skips entries whose channel no longer maps to the same broker. Good test coverage of the race.

LGTM.

---

Automated review by github-manager-bot