Repository navigation
grpctunnel: size streams per tunnel server — grow greedily under load, prune firmly at rest - #188
Conversation
8249e71 to
7374603
Compare
…, prune firmly at rest
A tunnel server (one relay-grpc-tunnel instance) can only hand a call to a
stream this agent holds on that same server, and the dispatcher sends a
token's calls to one tunnel server at a time. The agent sized its streams as
a total across servers and left placement to the load balancer, which
favours the nearest zone, so the server receiving calls could hold a single
stream: one call in flight made it "all busy" and callers there queued,
with idle streams on other servers and nothing telling the agent to grow.
Streams carry I/O, not work: an open is a round trip on a pooled connection
(the dispatcher is notified only for a token's first stream) and an idle
stream costs its shared tunnel server a heartbeat and a few goroutines. So
the agent now opens greedily and prunes firmly instead of rationing.
Coverage (coverage.go) runs each 1s sizing tick. The agent's streams are
grouped by the tunnel server they landed on (ServerHello.ServerId) into one
serverCoverage each, which reports its state; coverage plans the tick from
those states and balanceCoverage carries the plan out:
covered streams >= busy + 2 nothing
growing short, calls in flight open max(8, calls in flight, shortfall)
that tick -- doubling under pressure;
unbudgeted, paused while idle streams
elsewhere reach 4x demand, capped
filling short, idle open toward the reserve, budgeted
overdue short, idle, 30s fill, then evacuate: recycle its idle
streams so it drops the token -- never
a server with calls in flight, only
while another is covered, one per 30s
evacuated last 10 minutes recycle idle streams; no grow or fill
While anything is short the target is held. retireExcess prunes idle
streams once 2 minutes old and unused for 10, never taking a server below
its reserve except to obey a cap. The resting reserve stays at 2 per tunnel
server: a larger one was harder to place under zone-skewed placement and
costs every shared tunnel server more heartbeats.
Each server's budget counts calls in flight, not streams used recently: a
tunnel server hands each call to its newest idle stream, so counting
streams made a quiet server look short and got it evacuated.
Caps: agent maxStreams and MaxInflightRequests 256 -> 2048; tunnel server
MaxStreamsPerToken default 64 -> 1024, since one tunnel server carries a
token's whole concurrency.
Visibility: grpc_tunnel_server_streams{server_id,state},
grpc_tunnel_coverage_actions_total{action=grow|fill|evacuate}, and a
servers array in /systemcheck with each server's streams, busy, budget and
coverage state. Metric construction moves to initMetrics, shared with the
test client.
Tests (coverage_test.go) drive the real sizeTick against simulated workers
and tunnel servers under uniform, zone-skewed and unfillable placement; the
incident cases fail with coverage disabled. Growth is bracketed without
depending on placement: one tick asks for a chunk; with every stream
landing on the busy server a 1000-call burst is in flight in 8 ticks; with
none landing the agent stays bounded and keeps the busy server; after a
burst it prunes back to rest. Older sizing tests are restated in terms of
the reserve and cap constants.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7374603 to
1143945
Compare
keithfz
left a comment
There was a problem hiding this comment.
makes sense to me, being able to do this at the dispatcher when snyk broker gets killed would be nice
| func (s *serverCoverage) state(now int64) coverageState { | ||
| switch { | ||
| case s.evacuatedAt != 0: | ||
| return stateEvacuated | ||
| case s.short() == 0: | ||
| return stateCovered | ||
| case s.busy > 0: | ||
| return stateGrowing | ||
| case now-s.shortSince >= int64(coverageGrace): | ||
| return stateOverdue | ||
| default: | ||
| return stateFilling | ||
| } |
There was a problem hiding this comment.
is there any chance short requests get missed by the tick (not even sure how much that would that matter here i guess, but could cause more evacuations potentially)?
There was a problem hiding this comment.
the ticks happen frequencly and the evactuations have a 30s cooldown on them so per-tick should not matter
Yes i agree. The design I'd actually like is more of a queue processor design: all of the BE requests go into a queue, and these agents compete for satisfying the requests. After we get rid of snyk-broker i think i'm going to look at that - we can use redis for a simple queue then have the agents pick from that while the dispatcher just waits. |
Context
Scoping: This change fixes the actual balancing dynamics of how we make sure we have robust connections to the server.
Current design:
relay-dispatcherdecides thats the one it wants to use. However on the agent side we think we have 8 streams available so we don't need to grow. That one stream becomes a choke point. We see that in the logs today.New design:
The agents should be able to handle A LOT of traffic, but we want to keep the number of streams sized to the load b/c we don't want them heartbeating the servers to death when not in use. So the algo is: the agent tracks how many in-flight calls it has and if that is exhausted, it opens new streams to handle more load, then prunes them when things go idle. This is the existing design, we just twak it.
Now we:
evacuateit which is just to prune those streams so we can concentrate them on servers we like.Much better to have loaded servers than customers with piling up calls, so we are optimizing for getting stuff dispatched.
Note: once we get rid of
snyk-brokerwe can make this system smarter eg the dispatcher can do a better job of knowing where the available streams are so we solve this problem there (sort by health + available streams rather than "last healthy")