Skip to content

Commit 823f6c9

Browse files
h4x3rotabclaude
andcommitted
refactor(consul-postgres-ha): mesh-conn post-QUIC cleanup pass
Drops the MESH_CONN_TCP_ONLY env knob entirely (from dialICE, both compose templates, and reportLinkStats's tick cadence). The flag was investigated as a yamux-era escape hatch and proven non-helpful — pion still selects relay-UDP candidates regardless because the relay candidate's transport comes from the TURN allocation's relayed leg (always UDP unless RFC 6062 TCP-allocation requested), not from the client→TURN leg. With the QUIC switch, the underlying loss is handled by the transport layer, so the knob has no remaining purpose and was becoming misleading. Also quiets reportLinkStats: tick 10s → 60s and skip the log line entirely when bytes haven't moved since the last tick. Idle peer pairs no longer spam every 10 seconds. Final-stats line on stop is unchanged so postmortems still get a summary regardless of activity. Drops the unused *quic.Conn parameter from reportLinkStats, refreshes the stale "log every 10s" banner, and tightens the MESH_CONN_RELAY_ONLY comment in worker.yaml so the rationale ("flip on if worker-to-worker direct pairs fail") doesn't contradict itself. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ed3310a commit 823f6c9

3 files changed

Lines changed: 19 additions & 36 deletions

File tree

consul-postgres-ha/stage4/compose/coordinator.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ services:
4949
- TURN_SHARED_SECRET=${TURN_SHARED_SECRET}
5050
# See worker.yaml for the rationale on MESH_CONN_RELAY_ONLY.
5151
- MESH_CONN_RELAY_ONLY=${MESH_CONN_RELAY_ONLY:-}
52-
- MESH_CONN_TCP_ONLY=${MESH_CONN_TCP_ONLY:-}
5352
volumes:
5453
- /tmp/dstack-runtime/secrets:/run/secrets:ro
5554
- /tmp/dstack-runtime/instance:/run/instance:ro

consul-postgres-ha/stage4/compose/worker.yaml

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,12 @@ services:
5050
- TURN_HOST=${TURN_HOST}
5151
- TURN_SHARED_SECRET=${TURN_SHARED_SECRET}
5252
# MESH_CONN_RELAY_ONLY=1 forces ICE to gather only Relay candidates,
53-
# routing all peer traffic through the coturn server. Trade-off:
54-
# adds the Vultr round-trip to every peer hop, but avoids the
55-
# direct-pair connectivity-check failures we hit between dstack
56-
# workers (host/srflx/prflx pairs never establish, while relay
57-
# always works). Set when DIRECT_FAILS_FOR_THIS_DEPLOYMENT.
53+
# routing all peer traffic through the coturn server. Default off
54+
# because coordinator-to-worker direct candidates do work; flip on
55+
# if you hit a deployment where worker-to-worker direct pairs fail
56+
# (host/srflx/prflx never establish, only relay does), trading some
57+
# latency through coturn for guaranteed reachability.
5858
- MESH_CONN_RELAY_ONLY=${MESH_CONN_RELAY_ONLY:-}
59-
# MESH_CONN_TCP_ONLY=1 restricts ICE to TCP candidates only. The
60-
# underlying conn becomes a TCP byte stream that yamux can survive
61-
# on (UDP loss kills yamux keepalives — proven by trace at
62-
# 268KB pg_basebackup transfers).
63-
- MESH_CONN_TCP_ONLY=${MESH_CONN_TCP_ONLY:-}
6459
volumes:
6560
- /tmp/dstack-runtime/secrets:/run/secrets:ro
6661
- /tmp/dstack-runtime/instance:/run/instance:ro

consul-postgres-ha/stage4/mesh-conn/main.go

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ func dialAndPump(cfg *Config, self, peer Peer) error {
435435
// the underlying ice.Conn (i.e. wire bytes including QUIC overhead).
436436
// QUIC's StreamCount isn't directly exposed, so we report just bytes.
437437
stopStats := make(chan struct{})
438-
go reportLinkStats(peer.ID, counted, qconn, stopStats)
438+
go reportLinkStats(peer.ID, counted, stopStats)
439439
defer close(stopStats)
440440

441441
// 3. Bind localhost UDP+TCP listeners for every one of peer's ports.
@@ -640,8 +640,8 @@ func pumpUDPStreamToSock(s *quic.Stream, sock *net.UDPConn, dst *net.UDPAddr) er
640640
// =============================================================================
641641
// Per-link instrumentation: count bytes through the ICE conn (i.e. the
642642
// raw wire bytes including QUIC framing/encryption overhead) and log
643-
// every 10s. Useful for diagnosing whether a link drop happens after
644-
// 0 bytes, 1KB, or 100MB.
643+
// a periodic summary. Useful for diagnosing whether a link drop happens
644+
// after 0 bytes, 1KB, or 100MB.
645645
// =============================================================================
646646

647647
type countingConn struct {
@@ -703,8 +703,12 @@ func (p *iceConnPacketConn) SetDeadline(_ time.Time) error { return nil }
703703
func (p *iceConnPacketConn) SetReadDeadline(_ time.Time) error { return nil }
704704
func (p *iceConnPacketConn) SetWriteDeadline(_ time.Time) error { return nil }
705705

706-
func reportLinkStats(peerID string, conn *countingConn, qconn *quic.Conn, stop <-chan struct{}) {
707-
t := time.NewTicker(10 * time.Second)
706+
// reportLinkStats logs a periodic summary per peer link. Once a minute,
707+
// and only when bytes actually moved since the last tick, so an idle
708+
// mesh stays quiet. Always logs the final summary on stop, regardless
709+
// of activity, since that's what postmortems read.
710+
func reportLinkStats(peerID string, conn *countingConn, stop <-chan struct{}) {
711+
t := time.NewTicker(60 * time.Second)
708712
defer t.Stop()
709713
var lastIn, lastOut uint64
710714
for {
@@ -716,7 +720,10 @@ func reportLinkStats(peerID string, conn *countingConn, qconn *quic.Conn, stop <
716720
return
717721
case <-t.C:
718722
in, out := conn.bytesIn.Load(), conn.bytesOut.Load()
719-
log.Printf("[%s] link: in=%d (+%d B/10s) out=%d (+%d B/10s) reads=%d writes=%d",
723+
if in == lastIn && out == lastOut {
724+
continue
725+
}
726+
log.Printf("[%s] link: in=%d (+%d B/min) out=%d (+%d B/min) reads=%d writes=%d",
720727
peerID, in, in-lastIn, out, out-lastOut,
721728
conn.reads.Load(), conn.writes.Load())
722729
lastIn, lastOut = in, out
@@ -831,27 +838,9 @@ func dialICE(cfg *Config, remoteID string) (*ice.Conn, error) {
831838
if os.Getenv("MESH_CONN_RELAY_ONLY") == "1" {
832839
candidateTypes = []ice.CandidateType{ice.CandidateTypeRelay}
833840
}
834-
// MESH_CONN_TCP_ONLY=1 restricts ICE to TCP NetworkTypes AND drops
835-
// UDP-transport TURN URLs (pion's NetworkTypes only filters HOST
836-
// candidates; Relay candidates inherit transport from their TURN
837-
// URL's Proto, regardless of NetworkTypes). Without dropping UDP
838-
// URLs, pion still picks `relay ... (proto=udp)` and we get the
839-
// same UDP-loss behavior that kills yamux keepalives.
840-
netTypes := []ice.NetworkType{ice.NetworkTypeUDP4, ice.NetworkTypeTCP4}
841-
if os.Getenv("MESH_CONN_TCP_ONLY") == "1" {
842-
netTypes = []ice.NetworkType{ice.NetworkTypeTCP4}
843-
var tcpURLs []*stun.URI
844-
for _, u := range urls {
845-
if u.Proto == stun.ProtoTypeTCP {
846-
tcpURLs = append(tcpURLs, u)
847-
}
848-
}
849-
urls = tcpURLs
850-
log.Printf("[%s] TCP-only mode: %d URLs after UDP filter", remoteID, len(urls))
851-
}
852841
agent, err := ice.NewAgent(&ice.AgentConfig{
853842
Urls: urls,
854-
NetworkTypes: netTypes,
843+
NetworkTypes: []ice.NetworkType{ice.NetworkTypeUDP4, ice.NetworkTypeTCP4},
855844
CandidateTypes: candidateTypes,
856845
})
857846
if err != nil {

0 commit comments

Comments
 (0)