NetherNet transport improvements from production use#7
Open
SendableMetatype wants to merge 7 commits into
Open
NetherNet transport improvements from production use#7SendableMetatype wants to merge 7 commits into
SendableMetatype wants to merge 7 commits into
Conversation
Collected fixes from running the transport on production servers: unique placeholder remote addresses per connection instead of a shared 0.0.0.0 (keeps per IP limits and throttling functional), a guarded single fire channelActive, signaling stability fixes for long uptimes, a WebSocket frame aggregator on the signaling pipeline, reassembly buffer release on close, an outbound fragmentation bound, and notes on the reliable only channel design.
reconnect(freshToken) replaces only the socket to the signaling service. The signaling instance, its handlers, and everything built on it (server channel, WebRTC factories, live peer connections) survive, so a signaling drop no longer requires tearing the transport down. Liveness is now detectable on idle servers: a WebSocket protocol ping every 15 seconds guarantees inbound pongs on a healthy socket, so isChannelAlive(maxSilence) also catches silently half open TCP. Per channel scheduled tasks are tracked and cancelled on channel inactive, so reconnects no longer leak ping loops. TURN credential pushes are applied for the lifetime of the socket instead of only during connect, and the JSON RPC endpoint refreshes credentials every 30 minutes, so late joining peers no longer receive expired relay credentials. Pending RPC requests fail fast when the socket dies. The frame aggregator limit is raised to 128 KB for batched RPC frames.
isActive() reads an observer maintained flag instead of a blocking JNI call per write. Outbound fragmentation reuses one scratch direct buffer per channel and sends with sendAsync, which no longer stalls the event loop on the native network thread. Inbound packets are copied once instead of twice: complete single segment messages take a fast path and assembled multi segment buffers are handed off instead of re copied.
Connection setup hops from the signaling I/O thread to the server channel event loop: the signaling thread only registers the signal handler (so candidates arriving behind an offer are never dropped) and its keepalives can no longer be starved by blocking native WebRTC calls. Candidates arriving before the peer connection exists are queued in order. The server channel can be backed by a pool of PeerConnectionFactory instances with round robin assignment, spreading DTLS and SCTP load across several native network threads instead of serializing every player on one. Channels now receive their real remote address from the ICE selected candidate pair callback, which fires before DTLS and the data channels open, replacing the random placeholder before activation. Relayed connections record the TURN relay address. The old commented out getStats approach is removed.
Each in flight JSON RPC request now records the WebSocket channel it was written to. When a socket dies, onChannelInactive fails only the requests that were sent on that socket: during a reconnect the old channel's inactive event can no longer fail requests already written to the replacement socket, which previously left those callers with a spurious ClosedChannelException while the reply was still on its way.
Every retry bumps a generation counter that async engine callbacks capture: offer creation, description observers, ICE candidate emission, connection state changes, and the data channel open handler all bail once a retry has moved past them. Without this a delayed callback from an abandoned attempt could send stale SDP or candidates under the new connection id, trigger a spurious extra retry, or mark the handshake complete with data channels belonging to a closed peer connection. The signal id check also runs again inside the event loop task, since a retry can cycle the connection id between the check on the signaling thread and the task running, which previously let an old attempt's answer reach the new attempt's peer connection.
The native addIceCandidate rejects candidates while the peer connection has no remote description, and setRemoteDescription completes asynchronously, so a candidate arriving right behind the CONNECT_RESPONSE could be dropped. Candidates now queue until the remote description observer reports success and are then applied in arrival order. A handshake retry clears the queue along with the rest of the attempt state.
Owner
|
Thank you for the PR. I will review this weekend. |
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
2 participants
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.
A set of improvements from running the NetherNet transport on a production Geyser deployment, where frequent signaling drops at higher player counts motivated most of this.
One deliberate tradeoff to flag for your judgment: child channels previously all shared new InetSocketAddress(0), which is clearly recognizable as not a real address but breaks anything keyed on the remote address (per IP connection limits treat every NetherNet player as one host). This PR gives each connection a unique random 10.x.x.x placeholder instead, which keeps address keyed logic functional but is less obviously fictional in logs. With the candidate pair callback in this PR the placeholder only covers the window before ICE nomination and the rare case where the callback never fires, so if you prefer the recognizable 0.0.0.0 or another marker there, I am happy to adjust.
Note: the last two commits use RTCDataChannel.sendAsync and PeerConnectionObserver.onSelectedCandidatePairChanged from Kas-tle/webrtc-java#3, so this compiles once a webrtc-java release contains that change. Tested in production for signaling drop recovery without player disconnects, throughput at higher player counts, and correct remote addresses.