Remove coverage badge from README - #9
Merged
Merged
Conversation
TeoSlayer
added a commit
that referenced
this pull request
Apr 30, 2026
TeoSlayer
pushed a commit
that referenced
this pull request
May 3, 2026
…onn callers
Symptom: iter 4 added a 32 KB cap on per-Connection NagleBuf with
ErrSendBufFull returned to the caller of daemon.SendData. The
per-port services use connAdapter to expose Connections as net.Conn
(so http.ServeConn, io.Copy, gRPC etc. can run unchanged over Pilot
tunnels). connAdapter.Write currently propagates ErrSendBufFull
directly:
func (a *connAdapter) Write(p []byte) (int, error) {
if err := a.daemon.SendData(a.conn, p); err != nil {
return 0, err
}
return len(p), nil
}
This violates the implicit net.Conn contract. Standard-library
callers (net/http, io.Copy, bufio.Writer) treat ANY non-nil Write
error as "connection broken" and abort. A slow peer with a
momentarily full NagleBuf — a transient, recoverable condition —
silently breaks every HTTP/gRPC/stream consumer running on top of
the daemon, manifesting as "connection reset" or "EOF" errors at
the application boundary.
Real-world impact: webhook senders, port-forwarded HTTP servers,
gateway-mode TCP proxying, the dataexchange service all break the
moment a peer's cwnd fills (which iter 4's cap is meant to handle
gracefully — bound memory without dropping data).
Replication (sendbuf_caller_bug_test.go):
- Build Established Connection
- Pre-fill NagleBuf to MaxNagleBuf
- Call connAdapter.Write([]byte("ABC")) and time it
- assert: returns (0, ErrSendBufFull) within 100ms (current bug)
GREEN flips assertion: Write must block until NagleBuf drains, then
return (3, nil) — matching net.Conn back-pressure semantics that
HTTP/gRPC/io.Copy actually rely on.
TeoSlayer
pushed a commit
that referenced
this pull request
May 3, 2026
Bug fixed: connAdapter.Write no longer surfaces ErrSendBufFull as a
fatal connection error to net.Conn callers. It now blocks-and-retries
with capped exponential backoff (5ms → 100ms), surfacing the error
only on (a) connection state moving out of Established, or (b) the
30s connAdapterWriteDeadline elapsing. Net.Conn callers (http,
io.Copy, bufio, gRPC) see Write block briefly under back-pressure
and then succeed — matching the implicit contract that's modelled
on a kernel TCP send buffer being full.
Implementation (pkg/daemon/services.go):
func (a *connAdapter) Write(p []byte) (int, error) {
backoff := 5 * time.Millisecond
const maxBackoff = 100 * time.Millisecond
deadline := time.Now().Add(connAdapterWriteDeadline)
for {
err := a.daemon.SendData(a.conn, p)
if err == nil { return len(p), nil }
if !errors.Is(err, ErrSendBufFull) { return 0, err }
a.conn.Mu.Lock(); st := a.conn.State; a.conn.Mu.Unlock()
if st != StateEstablished {
return 0, fmt.Errorf("connection no longer established (state=%v)", st)
}
if time.Now().After(deadline) { return 0, err }
time.Sleep(backoff)
if backoff < maxBackoff { backoff *= 2 }
}
}
Why connAdapterWriteDeadline = 30s: long enough to ride out a
multi-RTT cwnd pause on a slow peer, short enough that a wedged
peer doesn't block the application forever. Future iter could
expose this via SetWriteDeadline, but for now the constant is
sufficient — slow peers eventually drain or the connection tears
down via dead-peer detection (idleSweepLoop, ICMP fast-flip).
Tests:
- TestConnAdapterWriteSurfacesErrSendBufFullToCaller: with
NagleBuf pre-filled to MaxNagleBuf, Write blocks; after a
side goroutine drains NagleBuf, Write returns (3, nil)
within 500ms. Pre-fix this returned (0, ErrSendBufFull)
immediately.
- TestConnAdapterWriteSurfacesNonBufferErrors: a closed conn's
"connection not established" error propagates immediately
(not in retry loop). Within 100ms, non-ErrSendBufFull,
non-nil err.
Race-clean across full pkg/daemon (62s). 14 commits ahead of v1.9.0,
9 bug categories closed.
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
1 participant
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.
No description provided.