[ISSUE #9725] Maintain heartbeat between Proxy and Broker

[btlqql]

Which Issue(s) This PR Fixes

• Fixes [Feature] Maintain heartbeat between proxy and broker #9725

Brief Description

Proxy opens remoting channels to brokers on demand, but low-traffic channels can remain completely idle long enough to be closed. The next send or consume request then pays the connection re-establishment cost. The root cause is that Proxy has no lifecycle-managed keepalive for its already-established broker channels.

This change:

• exposes a backward-compatible snapshot API for active remoting channels;
• filters configured and discovered NameServer channels before heartbeat delivery;
• adds a lifecycle-managed Proxy-to-Broker heartbeat service covering messaging, operation, transaction, and lite-subscription clients;
• sends lightweight one-way heartbeats only over existing active broker channels, so it neither discovers nor connects to idle brokers;
• adds enable, interval, and timeout settings with startup validation;
• prevents overlapping rounds, isolates client/channel failures, and records per-round operational statistics.

The default 30-second interval is below the idle close window described in the issue. Existing custom RemotingClient implementations remain source/binary compatible through the default empty snapshot implementation.

Impact

Low-traffic Proxy deployments retain broker connections that are already in use, avoiding avoidable latency spikes on the next request. NameServer connections are excluded, no broker-wide fan-out is introduced, and the behavior can be disabled or tuned through ProxyConfig.

How Did You Test This Change?

Ran on JDK 8:

mvn -pl proxy -am test -Dtest=ProxyBrokerHeartbeatServiceTest,ProxyBrokerHeartbeatConfigTest,MQClientAPIExtTest,NettyRemotingClientTest -Dsurefire.failIfNoSpecifiedTests=false -Dspotbugs.skip=true -Djacoco.skip=true

Result: 11-module reactor build successful; 46 targeted tests passed with 0 failures, 0 errors, and 0 skipped. Checkstyle reported 0 violations. The reviewed diff contains 10 files and 983 added lines.

[RockteMQ-AI]

Review by github-manager-bot

Summary

This PR adds a lifecycle-managed heartbeat service that keeps already-established Proxy-to-Broker remoting channels alive during low-traffic periods, preventing idle-close latency spikes on the next request. The approach is conservative: it only heartbeats over existing active channels, never discovers or connects to new brokers.

Findings

• [Info] RemotingClient.getActiveChannelAddresses() — Adding a default method returning Collections.emptyList() is the right call for backward compatibility. Custom RemotingClient implementations will not break. The NettyRemotingClient override correctly iterates channelTables and checks isOK().

• [Info] MQClientAPIExt.getActiveBrokerAddresses() — The NameServer exclusion logic is sound: removes both configured and discovered NameServer addresses from the active channel set. The null/empty guards are thorough.

• [Info] ProxyBrokerHeartbeatService — The design is well thought out:

  • No overlap: AtomicBoolean running prevents concurrent rounds.
  • Isolation: Each broker heartbeat is independently try/caught, so one failure does not abort the round.
  • Statistics: Per-round success/failure counts are logged, aiding observability.
  • One-way: Uses sendHeartbeatOneway which is appropriate — no response needed for keepalive.

• [Info] ProxyConfig — Defaults (30s interval, 3s timeout, enabled) are sensible. The initData() validation (interval > 0, timeout > 0, timeout < interval) prevents misconfiguration at startup.

• [Warning] ProxyBrokerHeartbeatService.runOnce() — The snapshotAddresses() call goes through ClusterServiceManager which may return addresses from a routing cache. If the cache is stale, the heartbeat might skip a recently-added broker. This is acceptable for keepalive (not discovery), but worth noting that newly established channels may take one round before being heartbeated.

Positive Observations

• Conservative scope: Only maintains existing connections, no fan-out or discovery. This minimizes risk.
• Clean lifecycle: start()/shutdown() with ScheduledExecutorService follows the same pattern as other Proxy services.
• Good test coverage: Tests cover overlap prevention, failure isolation, config validation, null handling, and the NameServer exclusion logic.
• Backward compatible: Default method on RemotingClient interface, no existing behavior changed.

Suggestions

• Consider adding a metric/counter for total heartbeat rounds and per-broker failure counts (beyond logging) for integration with monitoring systems.
• The proxyBrokerHeartbeatTimeoutMillis default of 3s seems reasonable, but if the broker is under heavy load, a one-way heartbeat could be delayed. Consider documenting that the timeout should be less than the interval to avoid accumulation.

Overall this is a clean, low-risk improvement to Proxy connection reliability. 👍

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot

Summary

Adds a ProxyBrokerHeartbeatService that periodically sends heartbeats from Proxy to Broker over existing remoting channels, preventing idle connections from being silently dropped by intermediate network infrastructure (load balancers, firewalls, etc.).

Findings

[Info] Design — The heartbeat is opt-in via enableProxyBrokerHeartbeat (default: true), with configurable interval (30s) and timeout (3s). The getActiveBrokerAddresses() method in MQClientAPIExt correctly excludes NameServer addresses from the heartbeat targets.

[Info] Configuration validation — ProxyConfig.initData() validates that interval and timeout are positive when heartbeat is enabled. Good defensive check.

[Warning] Connection lifecycle — When a Broker goes down, the heartbeat will start failing. Ensure the heartbeat service:

1. Does not log at ERROR level for expected transient failures (use WARN/DEBUG)
2. Does not trigger unnecessary reconnection storms
3. Properly handles the case where a Broker comes back with a different address

[Info] The getActiveBrokerAddresses() implementation correctly handles null returns from getActiveChannelAddresses(), getConfiguredNameServers(), and getAvailableNameSrvList(), and filters out null/empty strings.

[Info] Cross-repo note: This change is self-contained within apache/rocketmq (Proxy module). No changes needed in apache/rocketmq-clients since this is a Proxy-internal mechanism.

Suggestions

• Consider adding a metric/counter for heartbeat successes/failures to aid operational debugging.
• The 3s timeout seems reasonable but should be documented as the threshold after which a Broker is considered unreachable.

---

Automated review by github-manager-bot