Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/atenet/internal/router/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func NewRouterCmd() *cobra.Command {
cmd.Flags().StringVar(&cfg.Auth.AteapiServerName, "ateapi-server-name", "", "SNI / hostname expected on the ateapi server cert. Optional.")
cmd.Flags().BoolVar(&cfg.Auth.AteapiUseTokenAuth, "ateapi-use-token-auth", false, "Authenticate to ateapi with the Bearer token from --ateapi-token-file instead of the client certificate from --ateapi-client-cert.")
cmd.Flags().StringVar(&cfg.Auth.AteapiTokenFile, "ateapi-token-file", "", "Projected SA token file used as Bearer credential. Required with --ateapi-use-token-auth, ignored otherwise.")
cmd.Flags().DurationVar(&cfg.RouteTimeout, "route-timeout", defaultRouteTimeout, "Envoy's end-to-end timeout on the workload route, bounding one request from the ingress listener to the actor's response. Raise it for actors whose turns legitimately run long — a harness relaying an LLM completion holds the request open for the whole generation. This does not cover the resume that may precede the request; see --parked-request-budget")
Comment thread
mayawang marked this conversation as resolved.
cmd.Flags().DurationVar(&cfg.ParkedRequest.Budget, "parked-request-budget", defaultParkedRequestBudget, "Maximum time a resume flight keeps a request parked (held and retried) waiting for its actor to become routable; concurrent requests for the same actor share one flight and its budget")
cmd.Flags().IntVar(&cfg.ParkedRequest.Max, "parked-request-max", defaultParkedRequestMax, "Maximum number of requests that may be parked simultaneously; excess requests are shed with 503. 0 disables parking (requests fail fast on worker-pool saturation)")
cmd.Flags().DurationVar(&cfg.ParkedRequest.RetryInterval, "parked-request-retry-interval", defaultParkedRequestRetryInterval, "Delay before a parked request's first resume retry")
Expand Down
7 changes: 7 additions & 0 deletions cmd/atenet/internal/router/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ type routerConfig struct {

Auth authConfig

// RouteTimeout is Envoy's end-to-end timeout on the workload route: the
// ceiling on one request from the ingress listener to the actor's response.
// It bounds the actor's own handling time, not the resume that precedes it
// — parking and the ext_proc timeout cover that. A non-positive value
// leaves Envoy on defaultRouteTimeout.
RouteTimeout time.Duration

// ParkedRequest configures request parking: hold and retry requests whose
// actor cannot be served immediately due to transient worker-pool
// saturation, instead of failing fast. A non-positive Max disables parking.
Expand Down
1 change: 1 addition & 0 deletions cmd/atenet/internal/router/dataplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func (s *RouterServer) startEnvoyDataplane(ctx context.Context, g *errgroup.Grou
setOtlpCollector(ctx, xdsSrv, s.cfg.OtlpCollectorAddress)
xdsSrv.SetTraceRootSamplingPercent(traceRootSamplingPercent)

xdsSrv.SetRouteTimeout(s.cfg.RouteTimeout)
xdsSrv.SetExtProcMaxRequests(s.cfg.extProcMaxRequests())
if parkCfg.enabled() {
// Envoy must keep a parked request open at least as long as the router
Expand Down
60 changes: 59 additions & 1 deletion cmd/atenet/internal/router/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,25 @@ const defaultExtProcMessageTimeout = 5 * time.Second
// requests to already-running actors. See buildCluster.
const defaultExtProcMaxRequests = 2048

// defaultRouteTimeout is Envoy's end-to-end route timeout for workload traffic:
// the ceiling on a single request from the ingress listener to the actor's
// response. It bounds the actor's own handling time, not the resume that
// precedes it — parking and the ext_proc timeout cover that part.
const defaultRouteTimeout = 10 * time.Second

// envoyDefaultStreamIdleTimeout is the stream idle timeout Envoy applies when
// the HTTP connection manager does not set one. We never set it, so this is
// what governs today.
//
// It is a distinct limit from the route timeout: the route timeout bounds the
// upstream response time, while this bounds how long the stream may go with no
// encode/decode event at all. A turn that produces no bytes while the actor
// thinks — a non-streaming completion, or a request parked across a resume —
// is idle by this measure even though it is progressing, so without an
// override a route timeout above five minutes would never be reached. See
// routeIdleTimeout.
const envoyDefaultStreamIdleTimeout = 5 * time.Minute

// XdsServer implements an aggregated discovery service server for dynamic Envoy router nodes.
type XdsServer struct {
xdsPort int
Expand Down Expand Up @@ -139,6 +158,11 @@ type XdsServer struct {
// router's processing server, parked requests included. Must be >= the
// parking lot size (enforced at startup in Run).
extProcMaxRequests uint32

// routeTimeout is Envoy's end-to-end timeout on the workload route. Actors
// that hold a request open for a long turn — an LLM streaming a response,
// say — need this above the default or Envoy cuts the turn off with a 504.
routeTimeout time.Duration
}

func NewXdsServer(xdsPort int) *XdsServer {
Expand All @@ -154,6 +178,7 @@ func NewXdsServer(xdsPort int) *XdsServer {
ingressPort: 8080,
extProcMessageTimeout: defaultExtProcMessageTimeout,
extProcMaxRequests: defaultExtProcMaxRequests,
routeTimeout: defaultRouteTimeout,
}
}

Expand Down Expand Up @@ -188,6 +213,38 @@ func (x *XdsServer) SetExtProcMaxRequests(n int) {
}
}

// SetRouteTimeout sets Envoy's end-to-end timeout on the workload route. Raise
// it for actors whose turns legitimately run long — a harness relaying an LLM
// completion holds the request open for the whole generation, and at the
// default the client sees a 504 mid-turn. A non-positive value leaves the
// default unchanged.
func (x *XdsServer) SetRouteTimeout(d time.Duration) {
x.mu.Lock()
defer x.mu.Unlock()
if d > 0 {
x.routeTimeout = d
}
}

// routeIdleTimeout resolves the route-level idle timeout that accompanies the
// route timeout. Caller must hold x.mu.
//
// Raising --route-timeout on its own would not work: the stream a long turn
// runs on is idle for the whole turn whenever the actor sends nothing until it
// is done, and Envoy would reset it at the five-minute stream idle default
// before the requested timeout was ever reached. The idle timer must therefore
// never be the limit that bites first.
//
// Taking the larger of the two keeps the operator's ceiling honest without
// making the idle timer stricter than it already is: below five minutes the
// route timeout fires first anyway, so this leaves today's behavior alone.
func (x *XdsServer) routeIdleTimeout() time.Duration {
if x.routeTimeout > envoyDefaultStreamIdleTimeout {
return x.routeTimeout
}
return envoyDefaultStreamIdleTimeout
}

func (x *XdsServer) SetTlsConfig(httpsPort int, certPath string) {
x.mu.Lock()
defer x.mu.Unlock()
Expand Down Expand Up @@ -612,7 +669,8 @@ func (x *XdsServer) buildRoutes() *routev3.RouteConfiguration {
ClusterSpecifier: &routev3.RouteAction_Cluster{
Cluster: OriginalDstClusterName,
},
Timeout: durationpb.New(10 * time.Second),
Timeout: durationpb.New(x.routeTimeout),
IdleTimeout: durationpb.New(x.routeIdleTimeout()),
},
},
},
Expand Down
84 changes: 84 additions & 0 deletions cmd/atenet/internal/router/xds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,90 @@ func TestXdsServer_ExtProcCircuitBreaker(t *testing.T) {
})
}

func TestXdsServer_RouteTimeout(t *testing.T) {
// routeAction digs out the one workload route buildRoutes emits, which is
// where Envoy actually reads its timeouts.
//
// It pins the route to OriginalDstClusterName rather than trusting position:
// that is the cluster carrying actor traffic to the worker's atunnel ingress.
// A change that moves actor traffic onto some other route would otherwise
// leave this passing while the timeouts govern a route nothing uses.
routeAction := func(t *testing.T, x *XdsServer) *routev3.RouteAction {
t.Helper()
hosts := x.buildRoutes().GetVirtualHosts()
if len(hosts) != 1 || len(hosts[0].GetRoutes()) != 1 {
t.Fatalf("buildRoutes() = %d virtual hosts, want exactly 1 with 1 route", len(hosts))
}
action := hosts[0].GetRoutes()[0].GetRoute()
if got := action.GetCluster(); got != OriginalDstClusterName {
t.Fatalf("workload route targets cluster %q, want %q", got, OriginalDstClusterName)
}
return action
}
routeTimeout := func(t *testing.T, x *XdsServer) time.Duration {
t.Helper()
return routeAction(t, x).GetTimeout().AsDuration()
}
idleTimeout := func(t *testing.T, x *XdsServer) time.Duration {
t.Helper()
return routeAction(t, x).GetIdleTimeout().AsDuration()
}

t.Run("Default", func(t *testing.T) {
if got := routeTimeout(t, NewXdsServer(0)); got != defaultRouteTimeout {
t.Errorf("default route timeout = %v, want %v", got, defaultRouteTimeout)
}
})

t.Run("SetterOverrides", func(t *testing.T) {
x := NewXdsServer(0)
x.SetRouteTimeout(5 * time.Minute)
if got := routeTimeout(t, x); got != 5*time.Minute {
t.Errorf("route timeout after SetRouteTimeout(5m) = %v, want 5m", got)
}
})

// The flag cannot produce a zero: --route-timeout carries defaultRouteTimeout,
// so an operator who never passes it gets 10s, not 0. The guard is on the
// setter because SetRouteTimeout is part of the type's API and reachable
// from any caller, and because a zero here is the one value Envoy reads as
// "no timeout at all" — a mis-set knob would silently turn every stuck
// actor into a held-open request rather than failing visibly. The sibling
// setters guard the same way.
t.Run("NonPositiveKeepsDefault", func(t *testing.T) {
for _, d := range []time.Duration{0, -time.Second} {
x := NewXdsServer(0)
x.SetRouteTimeout(d)
if got := routeTimeout(t, x); got != defaultRouteTimeout {
t.Errorf("route timeout after SetRouteTimeout(%v) = %v, want default %v", d, got, defaultRouteTimeout)
}
}
})

// The route timeout alone does not bound a long turn. A stream carrying no
// bytes while the actor works is idle by Envoy's reckoning, and Envoy resets
// it at the 5m stream idle default whatever the route timeout says. These
// pin the relationship: the idle timer never bites before the ceiling the
// operator asked for, and it is not tightened below what applies today.
t.Run("IdleTimeoutTracksLongerRouteTimeout", func(t *testing.T) {
x := NewXdsServer(0)
x.SetRouteTimeout(30 * time.Minute)
if got := idleTimeout(t, x); got != 30*time.Minute {
t.Errorf("idle timeout with a 30m route timeout = %v, want 30m: a shorter idle timer would reset the stream first", got)
}
})

t.Run("IdleTimeoutKeepsEnvoyDefaultWhenRouteTimeoutIsShorter", func(t *testing.T) {
for _, d := range []time.Duration{defaultRouteTimeout, envoyDefaultStreamIdleTimeout} {
x := NewXdsServer(0)
x.SetRouteTimeout(d)
if got := idleTimeout(t, x); got != envoyDefaultStreamIdleTimeout {
t.Errorf("idle timeout with a %v route timeout = %v, want %v (unchanged from Envoy's default)", d, got, envoyDefaultStreamIdleTimeout)
}
}
})
}

func TestXdsServer_SetOtlpCollector(t *testing.T) {
// --otlp-collector-address defaults to OTEL_EXPORTER_OTLP_ENDPOINT, so the
// URL forms that variable carries have to reduce to the bare host and port
Expand Down
5 changes: 5 additions & 0 deletions manifests/ate-install/atenet-router.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ spec:
- "--ateapi-address=dns:///api.ate-system.svc:443"
- "--ateapi-ca-file=/run/servicedns-ca/trust-bundle.pem"
- "--ateapi-client-cert=/run/podidentity.podcert.ate.dev/credential-bundle.pem"
# Envoy's end-to-end timeout on the workload route. Raise it for actors
# whose turns legitimately run long — a harness relaying an LLM
# completion holds the request open for the whole generation, and at the
# 10s default the client gets a 504 mid-turn.
# - "--route-timeout=5m"
env:
- name: POD_NAME
valueFrom:
Expand Down
Loading