fix: detect a peer that vanishes without FIN or RST - #6
Merged
Conversation
The legacy SQL Server behind this proxy sits at the far end of a link that is cut nightly rather than closed. Nothing arrives, no error is raised, and a blocking read waits forever — so the proxy keeps the client leg open too, waiting for a response that will never come. On the Dremio side that strands the source plugin's state lock permanently: every subsequent health check times out against the lock, and only restarting the coordinator clears it. The teardown path is already correct — any exception in ProcessConnection runs Close(), which closes both legs. What is missing is the exception. Enable TCP keep-alive on both sockets so an unreachable peer faults the socket after ~90 seconds (60s idle, 3 probes 10s apart) instead of never. A live but idle peer answers the probes and is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ld69q37Jq9eMvLLy6zgWwc
ralfbecher
added a commit
that referenced
this pull request
Sep 3, 2026
AcceptConnection constructed the TDSConnection inline and only re-armed BeginAcceptTcpClient afterwards. Constructing a connection dials the far server with a blocking Connect, so a server that swallows SYNs held the single in-flight accept for the OS connect timeout - over two minutes on Linux - and every other client sat in the backlog for exactly as long. Observed against a legacy SQL Server that is blackholed nightly: one connection served per 135 seconds, for every client, all night. Three changes: - Re-arm the listener before setting up the connection, so a slow or dead far server no longer serializes accepts. - Bound the dial with a 10 second timeout rather than the OS SYN retry sequence, so a thread is not parked for minutes per attempt. - Roll back construction when the dial fails. The Stopping subscription is what keeps a TDSConnection alive, and it is taken before the dial, so a failed attempt lingered - still holding the accepted client socket - until the service stopped. That is the origin of the hundreds of NullReferenceExceptions from Dispose at shutdown, one per stranded attempt; _insideStream is null-guarded there as well. Keep-alive (#6) does not cover this case: it needs a connection that was established, and here none ever is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019FMY262wiaqabAsPuQ9b8G
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.
Problem
The legacy SQL Server behind this proxy sits at the far end of a link that is cut nightly rather than closed. Nothing arrives, no error is raised, and the blocking read on the inside leg waits forever. The proxy therefore also keeps the client leg open, waiting for a response that will never come.
Measured downstream effect (Dremio 26, source reached through this proxy): the source plugin's state lock is held by that stranded thread permanently. Every subsequent health check fails with
Timed out while trying to obtain plugin's state in 10s, 24x a day, and neitherapi/v3/catalog/<id>norapiv2/source/<name>can be read any more. Only restarting the Dremio coordinator clears it. In one observed case the source stayed unavailable for two weeks.Fix
The teardown path is already correct: any exception in
ProcessConnectionrunsClose(), which closes both legs. What is missing is the exception.Enable TCP keep-alive on both sockets, so an unreachable peer faults the socket after roughly 90 seconds (60s idle, then 3 probes 10s apart) instead of never.
Close()then propagates the failure to the client, which learns about it via FIN instead of waiting indefinitely.A live but idle peer answers the probes and is unaffected — only a genuinely unreachable one is torn down. Traffic cost is one packet per minute per idle connection.
Notes
TcpKeepAliveRetryCountrequires .NET Core 3.0+ on Linux; the project targetsnet6.0.🤖 Generated with Claude Code