[server] Coordinator standby tracks dynamic config changes to avoid s…

[fhan688]

Purpose

Linked issue: close #3625

On a CoordinatorServer, DynamicConfigManager fetched dynamic configs from ZooKeeper only once at startup and then ignored every later config-change notification: the notification handler returned early whenever isCoordinator == true. As a result, a long-running standby coordinator's local config drifted away from ZooKeeper.

When such a standby was later promoted to leader, its components were configured from that stale local snapshot even though ZooKeeper already held newer values. The most impactful case is SASL/PLAIN multi-user credentials (security.sasl.plain.credentials, hot-reloaded by FlussProtocolPlugin and registered on both server roles): after failover, the new leader could authenticate clients against a frozen credential set — a dynamically added user would be rejected, and a dynamically removed user would still be accepted.

This PR makes standby coordinators continuously apply dynamic config-change notifications, exactly like tablet servers do, so a promoted leader always reflects the latest config.

Brief change log

• DynamicConfigManager: remove the isCoordinator early-return in the change-notification handler, and drop the now-dead isCoordinator field and constructor parameter. Both server roles now apply notifications.
• Fix a concurrency issue that this change surfaces on the leader, which now both edits configs (via alterConfigs) and listens for notifications. Previously the notification path fetched from ZooKeeper outside the write lock, and alterConfigs applied-then-persisted without holding the lock across both steps, so a stale fetch could be applied on top of a newer, already-committed change. DynamicServerConfig now exposes:
  • refreshDynamicConfig(configSupplier, skipErrorConfig) — fetch and apply atomically under the write lock (used by startup() and the notification handler);
  • updateAndPersistDynamicConfig(configs, persistAction) — apply locally and persist to ZooKeeper atomically under the write lock (used by alterConfigs).
• Update CoordinatorServer and TabletServer to the two-argument DynamicConfigManager constructor.

Tests

• DynamicConfigChangeTest#testTracksConfigChangeNotificationsFromOtherWriter (new): a manager applies a config change written to ZooKeeper by another writer — the core behavior the fix enables. Verified it fails without the fix (expected 3 but was 0).
• CoordinatorHighAvailabilityITCase#testStandbyTracksDynamicConfigAndNewLeaderUsesItAfterFailover (new): alter a dynamic config on the leader, assert the standby tracks it, kill the leader's ZK session to trigger failover, and assert the promoted leader still uses the up-to-date value.
  Verified it fails without the fix ("Standby coordinator did not apply dynamic config change while standby").
• Updated the existing DynamicConfigManager construction call sites in DynamicConfigChangeTest and made each test close its manager after use (previously leaked notification watchers, now material because coordinators react to notifications).
• Full runs pass: DynamicConfigChangeTest (26), CoordinatorHighAvailabilityITCase (6), and the server startup/election suites (CoordinatorServerTest, TabletServerTest, CoordinatorServerElectionTest).

API and Format

No public API or storage-format changes. DynamicConfigManager's constructor signature changed, but it is an internal server class. No RPC message or ZooKeeper znode-format changes.

Documentation

No. This is a bug fix with no new user-facing feature or configuration.

[loserwang1024]

As discussed, please only allow the follower coordinator to subscribe to ZooKeeper notifications. The leader already maintains the latest configuration changes, so having it subscribe here may cause version rollback issues (e.g., A → B → A). When a follower transitions to become the leader, please disable the subscription at that point.

[fhan688]

As discussed, please only allow the follower coordinator to subscribe to ZooKeeper notifications. The leader already maintains the latest configuration changes, so having it subscribe here may cause version rollback issues (e.g., A → B → A). When a follower transitions to become the leader, please disable the subscription at that point.

Thanks @loserwang1024, you're right — done. I've reworked it so the leader no longer consumes notifications:

• Added a listeningEnabled gate to DynamicConfigManager. The notification handler no-ops when it's off, so a paused instance won't react to ZooKeeper changes.
• pauseListening() is called in CoordinatorServer#initCoordinatorLeader (on promotion) and resumeListening() in cleanupCoordinatorLeader (on demotion). A standby/tablet server keeps listening as before.
• On demotion, resumeListening() also re-fetches once from ZooKeeper: while the server was leader the watcher kept advancing its last-processed sequence id but skipped the bodies, so past notifications won't replay — the explicit re-sync picks up whatever the new leader changed meanwhile.
• Reverted the write-lock refactor from the previous revision (refreshDynamicConfig/updateAndPersistDynamicConfig) back to the original updateDynamicConfig. With the leader no longer listening, a single instance only ever has one writer, so that locking is no longer needed.

I could reproduce the A → B → A rollback you described in a unit test (SET null → DELETE restoring the default was rolled back to null by the manager's own notification), which is now covered by testPausedManagerIgnoresNotificationsAndResumeReSyncs. The failover IT is extended to check the promoted leader keeps the value it tracked as standby. PTAL.

[loserwang1024]

LGTM