[sunnysabor]

Refs #7283.

A gateway can accept a WebSocket connection before its initial configuration callbacks finish. This change adds an opt-in initial synchronization readiness gate: the readiness group remains OUT_OF_SERVICE until a complete, matching synchronization attempt has been applied. Initial connection establishment is asynchronous when the gate is enabled, so an unavailable Admin does not prevent management endpoints from starting.

The flag is shenyu.sync.websocket.initial-sync-readiness (default false). Upgrade Admin first, then enable the gateway flag. Existing clients and gateways with the flag disabled retain MYSELF and the legacy response format. Enabled gateways use a versioned request that older Admin versions ignore; they remain not ready and retry instead of treating connectivity as successful synchronization.

Implementation:

• Admin wraps configuration messages with a connection-scoped request ID and sequence, followed by an end frame carrying the expected count. Empty configuration can complete with zero frames. Followers do not acknowledge the request; leadership is checked again before sending completion.
• Each Admin connection tracks its own attempt. One complete standalone/master attempt can satisfy the shared startup latch; partial attempts from different connections are never combined. Missing frames, callback failures, abandoned attempts and the 60-second attempt timeout cannot open readiness. The existing reconnect loop retries failed or timed-out startup attempts.
• Completion waits for synchronous callbacks and deferred configuration work registered through InitialSyncApplication.register(CompletionStage). Incremental callbacks received before the matching end frame participate in the attempt, and their synchronous or registered asynchronous failures prevent readiness. The end frame closes this participation window: subsequent increments run outside the initial-application scope and cannot add to its pending count. Late completion from an abandoned attempt cannot open the gate. CommonPluginDataSubscriber propagates failures in this scope while preserving its legacy behavior otherwise.
• The synchronization indicator is included in readiness and excluded from aggregate health and liveness. Existing health group members and application readiness remain effective. Kubernetes examples use separate probe groups.
• Once successful, the startup latch is never revoked merely because Admin disconnects.
• Malformed UUID suffixes in versioned requests are ignored with a warning before synchronization starts.
• WebsocketDataHandler now owns an instance EnumMap rather than a static map. This also fixes cross-client handler replacement: constructing another client must not overwrite the subscribers used to apply and acknowledge the first client's configuration.

Application boundary and compatibility:
Subscribers that defer configuration application must register a stage before returning; that stage must cover their nested work and propagate failures. Unregistered background tasks, internally swallowed plugin errors, ongoing upstream health checks and lazy request-time connection establishment cannot be inferred from a void callback. This gate does not assert upstream service health. The implementation uses the current batch refresh/cache behavior; it does not add transactional snapshots or rollback of partially applied configuration. The protocol is opt-in and defaults to disabled. Upgrade Admin before enabling the gateway flag; deferred subscribers must honor the completion-stage contract described above.

Validation:

• Passed Admin protocol, gateway state/client, subscriber and Spring health-group tests, including unsupported peers, empty configuration, failure/retry, stale completions and multiple Admin attempts. Regression coverage follows the current batch refresh callbacks and verifies deferred refresh failures. Additional coverage checks malformed UUID requests followed by a valid request, synchronous/deferred incremental failures before the end frame, retry, and sustained increments after the end frame while initial application is still pending.

• Passed a real loopback WebSocket test with deferred subscriber work, end-frame arrival before application completion and disconnection after initial success.

• Full Java 17 ./mvnw -s /tmp/shenyu-maven-central-settings.xml clean install -Dmaven.javadoc.skip=true: passed, all 235 reactor modules, 16m26s, including tests, Checkstyle and RAT. Validated commit 866d1e504 on 2026-09-29. The temporary settings selects Maven Central.

• git diff --check: passed.

• Dedicated Kubernetes startup-delay/network fault-injection scenarios for this readiness gate were not run; the loopback test is not a substitute for those scenarios.

• Read the contribution guidelines.

• Added regression tests.

• Local full build passed ./mvnw clean install -Dmaven.javadoc.skip=true.

[Aias00]

Thanks for the thorough work here. The opt-in flag (default false), the versioned protocol prefix (MYSELF_V1: / INITIAL_SYNC_V1), and restricting the indicator to the readiness group keep this low risk for existing deployments.

I verified the Admin-side ThreadLocal wrapping is sound: syncAllByNamespaceId publishes via eventPublisher.publishEvent, DataChangedEventDispatcher is a plain synchronous ApplicationListener, and there is no ApplicationEventMulticaster customization or @Async in shenyu-admin. Sends therefore happen on the calling thread, so the sequence count and the end-frame total stay consistent.

CI note (not blocking): the three red checks are unrelated to this change — shenyu-integrated-test-http is Maven BUILD SUCCESS and fails at the end on pull access denied for apache/shenyu-integrated-test-http; e2e-mysql fails because Admin cannot obtain a JDBC connection (Failed to determine DatabaseDriver) while MySQL is still starting; e2e is a dependency gate that inherits the upstream failure. k8s-ingress-websocket and the other ingress/e2e-case jobs pass.

Requesting changes on two points:

1. Unguarded UUID.fromString(requestId) in WebsocketCollector.initialSync
A message starting with MYSELF_V1: whose suffix is not a UUID raises IllegalArgumentException, and this branch is not wrapped in a try/catch, so the exception escapes onMessage. Please validate the id and ignore the request with a warning instead of letting client input throw.

2. Incremental-application failures now block readiness
InitialSyncState.applyIncremental brings non-initial incremental callbacks into InitialSyncApplication.run, and CommonPluginDataSubscriber.subscribeDataHandler now throws IllegalStateException whenever InitialSyncApplication.isActive(). Previously such failures were only logged. Consequence: any single incremental config that fails to apply while an attempt is pending sets failed=true, which forces a reconnect and a full retry; if Admin keeps pushing the same bad config, the gateway may never reach readiness. Please confirm this is intended, or scope failure propagation to initial-sync frames only.

Non-blocking comments:

3. pending may never drain
applyIncremental increments pending for every incremental message. If Admin pushes a continuous stream during startup, pending may stay above zero, and together with the 60s attempt timeout the gate may never open. Consider bounding incremental participation or counting it separately.

4. WebsocketDataHandler static -> instance EnumMap
This is a genuine fix (previously all clients shared and overwrote one handler map, so multi-URL setups used whichever client was constructed last), but it is a behavior change bundled here and is not mentioned in the description. Worth calling out.

The tests are solid — good coverage on interleaved increments, multiple Admins not combining partial attempts, deferred failures, abandoned completions, timeouts, and empty snapshots.

[sunnysabor]

@Aias00 Thanks for the review and for checking the CI failures. Pushed 866d1e5 with the following changes and clarification:

1. Invalid request IDs: WebsocketCollector.initialSync now catches IllegalArgumentException, logs a warning, and ignores the request before starting synchronization. The regression test covers empty/malformed UUID suffixes, no synchronization or response for those requests, and a subsequent valid request on the same session.

2. Incremental failures: This is intentional for increments received before the matching end frame, with strict readiness enabled. They modify the same caches as the initial frames, so ignoring an application failure in that window could acknowledge an incomplete application. Such failures still require a fresh attempt; a repeatedly failing configuration in that window can keep the gateway not ready. Added tests for synchronous and deferred incremental failures and successful retry. This remains opt-in and does not affect liveness.

3. Bounded participation: The end frame now closes the incremental participation window, even while previously registered application work remains pending. Later increments run outside InitialSyncApplication and do not increase the attempt's pending count. A regression test sends 100 later increments while initial work is pending, then verifies readiness opens when the initial work completes without waiting for those increments. Work admitted before the end frame still must complete successfully within the existing timeout; this is a fixed completion boundary, not a transactional snapshot or a guarantee of continuously fresh configuration.

4. Handler map ownership: Agreed that this deserves an explicit callout. The instance EnumMap fixes cross-client handler replacement: constructing another client must not overwrite the subscribers used to apply and acknowledge the first client's configuration. This matters for the multi-Admin application/completion boundary.

Runtime readiness and reconnects: Once the gateway has successfully completed its first synchronization, the shared startup readiness latch remains true for the lifetime of that synchronization service. Network instability, WebSocket disconnects/reconnects, or subsequent incremental-application failures do not revoke this synchronization readiness condition or require a new successful synchronization to restore it. The gateway continues using its existing cached configuration, although failed updates may leave it stale. If a disconnect or application failure occurs before the first successful synchronization, the gateway remains not ready until a later attempt succeeds. A process restart starts a new readiness cycle, and other health indicators can still make overall readiness fail.

Validation on Java 17:

• 47 focused tests passed (WebsocketCollectorTest, InitialSyncStateTest, and the loopback InitialSyncConnectionTest).
• Full ./mvnw -s /tmp/shenyu-maven-central-settings.xml clean install -Dmaven.javadoc.skip=true passed: all 235 reactor modules, 16m26s, including tests, Checkstyle and RAT. The temporary settings selects Maven Central.
• git diff --check passed.

Dedicated Kubernetes fault-injection scenarios were not run locally. Please take another look, especially at the explicit pre-end-frame failure policy in point 2.

[Aias00]

Re-review → approving

Thanks for the response — both points are now settled, one by code and one by confirmed intent.

1. UUID.fromString is guarded. WebsocketCollector.initialSync:328-334 validates before doing anything else:

try {
    UUID.fromString(requestId);
} catch (IllegalArgumentException ex) {
    LOG.warn("Ignoring initial synchronization request with an invalid UUID");
    return;
}

The early return matters — it happens before SpringBeanUtils lookups and before INITIAL_SYNC.set(...), so a malformed MYSELF_V1: frame cannot allocate state or leave the ThreadLocal populated. Combined with the regression test for empty/malformed suffixes and the "subsequent valid request still works" case, this is closed.

2. Incremental failure propagation — accepted as intended. I re-read InitialSyncState after your explanation and the design is coherent:

• applyIncremental:113-117 short-circuits with a plain action.run() when there is no active attempt (requestId == null, failed, or ended), so ordinary steady-state increments are not wrapped and cannot set failed;
• only increments arriving before the matching end frame go through InitialSyncApplication.run(...) (:121), and those do mutate the same caches the initial frames are filling. Acknowledging readiness while one of them failed would be claiming a complete application that is not complete.

Given the feature is opt-in and default false, fail-closed is the right default for a readiness gate: a gateway that could not apply its configuration should not report itself ready. I asked for confirmation rather than a change, and your reasoning ("could acknowledge an incomplete application") is the correct one. Approving on that basis.

One operational gap I would like addressed (not blocking)

That failure path is silent. CommonPluginDataSubscriber:199-203 throws IllegalStateException when isActive(), the exception propagates into InitialSyncApplication.run(...).whenComplete((ignored, error) -> applied(attempt, error)), and applied() (:132-136) only does pending-- / failed = true / completeIfApplied() — no logging anywhere on that path. So an operator sees a gateway that never becomes ready, with nothing in the logs indicating which config type, which id, or which exception caused it. Since needsReconnect() will then force reconnect and re-attempt forever, this is the difference between "5 minutes to diagnose" and "un-diagnosable".

Please log the failure at ERROR with the data type and payload id before (or instead of) rethrowing — e.g. in applied() where error is already in hand, or in CommonPluginDataSubscriber before the throw. Worth a follow-up PR if you would rather keep this one moving.

Non-blocking notes

• pending may not drain: your 60 s cutoff in needsReconnect():75 and the same bound in accept():87 is an adequate backstop — a continuous incremental stream can delay readiness but always ends in a bounded retry rather than hanging. Noting it because completeIfApplied():140 additionally requires pending == 0, so the gate opens only when the stream quiesces inside the window.
• CI: the red checks are shenyu-plugin-mcp-server failing to compile — MessageHandlingResult now requires a fourth McpJsonMapper argument, and the call site passes three. Same failure on #7375 and #7364, which touch unrelated modules, so it is a master-level break, not caused by this change. Please re-run once it is fixed.
• The e2e shenyu-bootstrap-websocket.yml / shenyu-cm.yml updates are a nice touch: they make the new readiness gate actually exercised in CI rather than only unit-tested.