Conversation
The Minecraft plugin force-reconnects when it sees no inbound frame for
75s (MinecraftRealtimeClient#heartbeatTick). The backend never emitted an
unsolicited frame: RealtimeCodec only built serverHello/error/
deployDrainAdvice, and client heartbeats were consumed without a reply
(`case HEARTBEAT -> state.recordHeartbeat()`).
So on a server with no punishments or notifications flowing, the plugin
received exactly one frame ever -- the ServerHello -- and then silence. Its
watchdog tripped on the first 25s tick past 75s, tore down a perfectly
healthy connection, and reconnected, logging:
[Realtime] No frames received within 75s; forcing reconnect
roughly every 100 seconds, forever. Each cycle also dropped the plugin to
fallback polling and re-ran a full baseline fetch.
RealtimeCodec.heartbeat(long) previously existed but was never wired to a
caller and was removed as dead code in 19217af, two minutes after the
plugin-side watchdog landed in minecraft@326275e.
Add RealtimeServerHeartbeatEmitter, a scheduled counterpart to
RealtimeHeartbeatSweeper: the sweeper closes connections whose client
heartbeats stopped, the emitter keeps client watchdogs fed. Default
interval is 25s, giving ~3 heartbeats per 75s client window.
The heartbeat envelope deliberately carries no event_id -- both the plugin
and the panel transport-ACK any frame that has one, which would turn every
keepalive into a request/response pair.
Fixing this backend-side repairs every already-deployed plugin without
requiring server operators to update their jar.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sweep wrote to every connection in the process on the scheduler thread. A WebSocket write to a peer that has stopped reading blocks: ConcurrentWebSocketSessionDecorator only buffers for *concurrent* senders (tryFlushMessageBuffer uses a non-blocking tryLock), so the sole sender always takes the blocking path through StandardWebSocketSession -> getBasicRemote().sendBinary(), bounded only by Tomcat's ~20s BLOCKING_SEND_TIMEOUT. One wedged client therefore stalled every connection behind it in the sweep -- across all tenants -- starving healthy clients of the very heartbeat that keeps their 75s watchdog quiet. That reintroduces the reconnect storm this component exists to prevent. Hand delivery to RealtimeDispatchExecutor keyed by server, matching how InProcessRealtimeEventDispatcher already fans out domain events: the stall stays inside the offending tenant's shard and off the scheduler thread. Eligibility is re-checked inside the task since a connection can close between the sweep and the task reaching its worker. Tests: doesNotWriteOnTheSweepThread and partitionsDeliveryPerServerSoOneTenantCannotStallAnother pin the contract structurally; wedgedTenantDoesNotDelayHeartbeatForAnotherTenant exercises the real sharded executor with a genuinely blocked write. All three were confirmed to fail against the previous inline implementation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
@greptile re-review? |
Sharding heartbeat delivery by server narrowed the stall but did not remove it. dispatchWorkers caps at min(cores, 8), so with more than eight tenants unrelated servers collide on a worker by pigeonhole: one wedged peer's blocking write (~20s, Tomcat BLOCKING_SEND_TIMEOUT) stalls every other server batched behind it on that single-threaded worker. Enough collisions and a healthy client crosses its 75s inbound-liveness window and force-reconnects -- the exact failure this component prevents. Sharing the dispatch executor also coupled keepalives to domain-event delivery: a wedged heartbeat could hold up real punishment and notification fan-out for every tenant on the shard, and heartbeats have no ordering requirement that justified using an order-preserving per-tenant queue. Any delivery granularity coarser than one connection leaves head-of-line blocking, so dispatch each connection independently onto its own virtual thread. That matches the granularity of the per-connection send lock in RealtimeSessionOperations, so a wedged peer can only ever stall itself. Java 21 virtual threads make it cheap: the blocking write parks and unmounts its carrier instead of occupying a pooled thread. The sweep is also simpler -- no grouping, no shard keying. The package-private constructor that injects the executor for tests makes Spring's constructor set ambiguous, which fails at context startup with "No default constructor found" rather than at compile time. The injection point is annotated explicitly and pinned by RealtimeServerHeartbeatEmitterWiringTest, since no other test boots a context. submitsOneTaskPerConnection and wedgedConnectionDoesNotDelayHeartbeatFor- Another were both confirmed to fail against the previous per-server batching, and the wiring test against an unannotated constructor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Minecraft plugin force-reconnects when it sees no inbound frame for 75s (MinecraftRealtimeClient#heartbeatTick). The backend never emitted an unsolicited frame: RealtimeCodec only built serverHello/error/ deployDrainAdvice, and client heartbeats were consumed without a reply (
case HEARTBEAT -> state.recordHeartbeat()).So on a server with no punishments or notifications flowing, the plugin received exactly one frame ever -- the ServerHello -- and then silence. Its watchdog tripped on the first 25s tick past 75s, tore down a perfectly healthy connection, and reconnected, logging:
roughly every 100 seconds, forever. Each cycle also dropped the plugin to fallback polling and re-ran a full baseline fetch.
RealtimeCodec.heartbeat(long) previously existed but was never wired to a caller and was removed as dead code in 19217af, two minutes after the plugin-side watchdog landed in minecraft@326275e.
Add RealtimeServerHeartbeatEmitter, a scheduled counterpart to RealtimeHeartbeatSweeper: the sweeper closes connections whose client heartbeats stopped, the emitter keeps client watchdogs fed. Default interval is 25s, giving ~3 heartbeats per 75s client window.
The heartbeat envelope deliberately carries no event_id -- both the plugin and the panel transport-ACK any frame that has one, which would turn every keepalive into a request/response pair.
Fixing this backend-side repairs every already-deployed plugin without requiring server operators to update their jar.
Confidence Score: 5/5
What T-Rex did
Reviews (4): Last reviewed commit: "Deliver realtime heartbeats per connecti..." | Re-trigger Greptile