diff --git a/cmd/atenet/internal/router/cmd.go b/cmd/atenet/internal/router/cmd.go index 233f717ee..936aca59f 100644 --- a/cmd/atenet/internal/router/cmd.go +++ b/cmd/atenet/internal/router/cmd.go @@ -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") 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") diff --git a/cmd/atenet/internal/router/config.go b/cmd/atenet/internal/router/config.go index 7b784f8e4..936099428 100644 --- a/cmd/atenet/internal/router/config.go +++ b/cmd/atenet/internal/router/config.go @@ -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. diff --git a/cmd/atenet/internal/router/dataplane.go b/cmd/atenet/internal/router/dataplane.go index e77a0b274..af305e5e0 100644 --- a/cmd/atenet/internal/router/dataplane.go +++ b/cmd/atenet/internal/router/dataplane.go @@ -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 diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index 315d7adf9..1ba6812c3 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -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 @@ -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 { @@ -154,6 +178,7 @@ func NewXdsServer(xdsPort int) *XdsServer { ingressPort: 8080, extProcMessageTimeout: defaultExtProcMessageTimeout, extProcMaxRequests: defaultExtProcMaxRequests, + routeTimeout: defaultRouteTimeout, } } @@ -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() @@ -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()), }, }, }, diff --git a/cmd/atenet/internal/router/xds_test.go b/cmd/atenet/internal/router/xds_test.go index 07d9e1cc3..3b4e6aef0 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -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 diff --git a/manifests/ate-install/atenet-router.yaml b/manifests/ate-install/atenet-router.yaml index 769d1c2e5..15f0b2beb 100644 --- a/manifests/ate-install/atenet-router.yaml +++ b/manifests/ate-install/atenet-router.yaml @@ -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: