[type:fix] Immediate websocket master rediscovery and configurable ch…

[harshkha3417]

Fixes #7808.

Motivation

When ShenYu Admin runs in cluster mode and the current master fails over, the gateway's websocket connection to the old master drops or gets demoted, but the gateway remains stale until WebsocketSyncDataService's background timer triggers masterCheck().

This issue was caused by four cooperating bottlenecks:

1. Hard-coded 60s Check Interval: WebsocketSyncDataService scheduled masterCheck() with a fixed 60-second delay on AbstractRoundTask, which was not configurable via WebsocketConfig / application properties (shenyu.sync.websocket.master-check-interval).
2. Delayed Re-seeding on Client Pruning: In masterCheck(), CollectionUtils.isEmpty(clients) was only evaluated at the start of the method. When the dead/demoted master client was pruned by the while loop, clients became empty and remained empty for another entire round (up to 60s) before re-seeding clients from websocketConfig.getUrls().
3. Passive Socket Closure Without Master Re-election Trigger: When the websocket connection to the master admin dropped unexpectedly (onClose), ShenyuWebsocketClient only attempted to reconnect to its own dead URL via internal exponential backoff without notifying WebsocketSyncDataService that the cluster master connection had been lost.
4. Lack of Running Mode Probing on Open Connections: If an Admin instance lost master status without severing the TCP/websocket connection, the client remained permanently marked with isConnectedToMaster == true because RUNNING_MODE was only queried once on onOpen.

Modifications

1. Configurable Master Check Interval:
   • Added masterCheckInterval (defaults to 60 seconds) with getter, setter, equals, hashCode, and toString to WebsocketConfig.
   • In WebsocketSyncDataService, used websocketConfig.getMasterCheckInterval() when initializing the periodic round task.
2. Immediate Re-seeding in the Same Pass:
   • In WebsocketSyncDataService.masterCheck(), re-evaluated CollectionUtils.isEmpty(clients) after the client pruning loop. If all clients were removed, clients is immediately re-seeded from websocketConfig.getUrls().
3. Master Lost Reactive Notification:
   • Added constructor overload to ShenyuWebsocketClient accepting Consumer<ShenyuWebsocketClient> onMasterLostCallback.
   • In onClose(), if wasMaster && !this.manuallyClosed.get(), onMasterLostCallback is invoked.
   • In onMessage() for RUNNING_MODE, if cluster mode reports wasMaster && !isMaster && !this.manuallyClosed.get(), onMasterLostCallback is invoked.
   • In WebsocketSyncDataService, onMasterLost schedules a debounced one-shot masterCheck() (after a 200ms delay) so the gateway immediately converges to the newly elected master without waiting for the full periodic interval.
4. Periodic Running Mode Verification:
   • In ShenyuWebsocketClient.healthCheck(), if the connection is open and the running mode is not standalone, DataEventTypeEnum.RUNNING_MODE.name() is sent alongside heartbeats to promptly catch leadership demotions.
5. Testing:
   • Updated WebsocketConfigTest for getter/setter, equals, hashCode, and toString.
   • Added tests in WebsocketSyncDataServiceTest verifying configurable check interval, same-pass re-seeding when all clients are removed, and master loss notification triggering rediscovery.
   • Added tests in ShenyuWebsocketClientTest covering onClose notifications, manuallyClosed suppression, RUNNING_MODE demotion notification, and cluster healthCheck probing.

Verification

• shenyu-sync-data-websocket unit tests passed (127/127)
• shenyu-spring-boot-starter-sync-data-websocket unit tests passed (4/4)

[Aias00]

Approved. The debounce design is correct, and the trigger condition is scoped more tightly than the description suggests — which is the part I was most worried about.

What I verified:

The callback is gated on wasMaster.

if (wasMaster && !this.isConnectedToMaster && !this.manuallyClosed.get() && Objects.nonNull(onMasterLostCallback)) {
    onMasterLostCallback.accept(this);
}

Two things this gets right that a naive version would not: a follower client losing its connection does not trigger a rediscovery, so only the client that actually owned the master role can start the cycle; and manuallyClosed prevents the service’s own close() from kicking off a rediscovery during shutdown — otherwise every graceful shutdown would write a burst of "master lost" into the timer.

One rediscovery per burst. rediscoveryScheduled.compareAndSet(false, true) collapses N simultaneous onMasterLost callbacks into a single 200 ms task, and the flag is cleared both in the task body and in the scheduling-failure catch. Combined with masterCheck() being synchronized, a failover where every client notices at once produces exactly one check rather than one per client.

Config handling is sound. websocketConfig.getMasterCheckInterval() <= 0 ? 60 : ... keeps the previous behaviour for anyone who does not set the new property, so this is additive. The timerTask null-guards in the standalone-mode path and the re-seeding of clients when the list empties are both defensive fixes to real gaps — masterCheck could previously leave the client list permanently empty.

One concern worth thinking about before merge:

An extended master outage becomes a ~5 Hz reconnect loop. The flag is reset inside the scheduled task before masterCheck() runs:

public void run(final TaskEntity taskEntity) {
    rediscoveryScheduled.set(false);
    masterCheck();
}

If the master stays unreachable, masterCheck closes/creates clients, the newly created clients immediately report master-lost, and each report re-schedules another 200 ms check. That is a dramatic change from the previous fixed 60 s cadence, and it is now the normal path during an outage rather than an exceptional one. A backoff — growing the delay when two rediscoveries fire within a short window, or a simple attempt counter reset on a successful connection — would keep the fast failover while preventing an admin-cluster reconnect storm. Worth at least a comment stating the expected steady-state rate during a sustained outage.

Two minor notes:

• The 200 ms delay is a magic number in three places (task delay, and implicitly the debounce window). A named constant would document that it is "long enough for the demoted client to settle, short enough to feel immediate".
• masterCheck() holds the service monitor for the whole reconnect sequence, so an onMasterLost arriving during a slow check blocks on the lock rather than being dropped — correct, but it means the CAS does not bound concurrency, only scheduling. Fine as is; just noting the semantics.

The test coverage (WebsocketSyncDataServiceTest, ShenyuWebsocketClientTest, WebsocketConfigTest) matching each of the four bottlenecks listed in the description is the right structure for a change that touches config, timer and connection lifecycle at once.