From 74298e1d9e283e27a98baf068fe35f55e48421ac Mon Sep 17 00:00:00 2001 From: Maya Wang Date: Mon, 3 Aug 2026 06:44:25 -0700 Subject: [PATCH 1/2] feat: configurable Envoy route timeout for long-running actor requests Envoy's end-to-end timeout on the workload route is hardcoded at 10s. An actor that legitimately holds a request open longer than that gets cut off: a harness relaying an LLM completion keeps the request open for the whole generation, and the client sees a 504 mid-turn. Add --route-timeout on atenet-router. The default is 10s, so behavior is unchanged, and a non-positive value leaves the default in place. The knob bounds the actor's own handling time only. The resume that may precede a request is covered by request parking and the ext_proc message timeout, both of which already derive from --parked-request-budget. Wired as a flag on the existing router config struct rather than an env read, matching how the parked-request and ext_proc knobs are done, and documented as a commented-out entry in the atenet-router manifest. The test reads the timeout back out of buildRoutes, where Envoy actually picks it up, and pins that route to OriginalDstClusterName: a change that moved actor traffic onto some other route would otherwise leave the test passing while the timeout governed a route nothing uses. --- cmd/atenet/internal/router/cmd.go | 1 + cmd/atenet/internal/router/config.go | 7 ++++ cmd/atenet/internal/router/dataplane.go | 1 + cmd/atenet/internal/router/xds.go | 27 ++++++++++++- cmd/atenet/internal/router/xds_test.go | 50 ++++++++++++++++++++++++ manifests/ate-install/atenet-router.yaml | 5 +++ 6 files changed, 90 insertions(+), 1 deletion(-) 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..1f2d280de 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -95,6 +95,12 @@ 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 + // XdsServer implements an aggregated discovery service server for dynamic Envoy router nodes. type XdsServer struct { xdsPort int @@ -139,6 +145,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 +165,7 @@ func NewXdsServer(xdsPort int) *XdsServer { ingressPort: 8080, extProcMessageTimeout: defaultExtProcMessageTimeout, extProcMaxRequests: defaultExtProcMaxRequests, + routeTimeout: defaultRouteTimeout, } } @@ -188,6 +200,19 @@ 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 + } +} + func (x *XdsServer) SetTlsConfig(httpsPort int, certPath string) { x.mu.Lock() defer x.mu.Unlock() @@ -612,7 +637,7 @@ func (x *XdsServer) buildRoutes() *routev3.RouteConfiguration { ClusterSpecifier: &routev3.RouteAction_Cluster{ Cluster: OriginalDstClusterName, }, - Timeout: durationpb.New(10 * time.Second), + Timeout: durationpb.New(x.routeTimeout), }, }, }, diff --git a/cmd/atenet/internal/router/xds_test.go b/cmd/atenet/internal/router/xds_test.go index 07d9e1cc3..36315e09c 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -564,6 +564,56 @@ func TestXdsServer_ExtProcCircuitBreaker(t *testing.T) { }) } +func TestXdsServer_RouteTimeout(t *testing.T) { + // routeTimeout digs the timeout out of the one workload route buildRoutes + // emits, which is where Envoy actually reads it. + // + // 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 timeout governs a route nothing uses. + routeTimeout := func(t *testing.T, x *XdsServer) time.Duration { + 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.GetTimeout().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) + } + }) + + // A zero value is what an operator who never passes --route-timeout would + // produce if the flag default were dropped. Envoy reads a zero route + // timeout as "no timeout at all", so the setter must ignore it rather than + // silently turning every stuck actor into a held-open request. + 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) + } + } + }) +} + 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: From ba0753546c7e785649b4f7201dfd6b633678c42d Mon Sep 17 00:00:00 2001 From: Maya Wang Date: Mon, 3 Aug 2026 21:27:10 -0700 Subject: [PATCH 2/2] atenet: pair the route timeout with a matching idle timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raising --route-timeout on its own does not lengthen a turn. The HCM sets no stream_idle_timeout, so Envoy applies its 5m default, and a stream carrying no bytes while the actor works is idle by that measure even though the turn is progressing — a non-streaming completion sends nothing until it is done. Envoy resets the stream at 5m whatever the route timeout says, so the knob silently stops working past that point. Set the route-level idle_timeout to the larger of the route timeout and the 5m Envoy already applies. Below 5m the route timeout fires first regardless, so today's behavior is unchanged; above it the operator's ceiling becomes the real one. Route-level rather than HCM-level keeps it scoped to workload traffic. Also corrects the NonPositiveKeepsDefault comment, which implied the flag could produce a zero. It cannot — --route-timeout carries a default. The guard is there because the setter is reachable from any caller and because zero is the one value Envoy reads as "no timeout at all". --- cmd/atenet/internal/router/xds.go | 35 ++++++++++++++++- cmd/atenet/internal/router/xds_test.go | 52 +++++++++++++++++++++----- 2 files changed, 77 insertions(+), 10 deletions(-) diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index 1f2d280de..1ba6812c3 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -101,6 +101,19 @@ const defaultExtProcMaxRequests = 2048 // 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 @@ -213,6 +226,25 @@ func (x *XdsServer) SetRouteTimeout(d time.Duration) { } } +// 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() @@ -637,7 +669,8 @@ func (x *XdsServer) buildRoutes() *routev3.RouteConfiguration { ClusterSpecifier: &routev3.RouteAction_Cluster{ Cluster: OriginalDstClusterName, }, - Timeout: durationpb.New(x.routeTimeout), + 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 36315e09c..3b4e6aef0 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -565,14 +565,14 @@ func TestXdsServer_ExtProcCircuitBreaker(t *testing.T) { } func TestXdsServer_RouteTimeout(t *testing.T) { - // routeTimeout digs the timeout out of the one workload route buildRoutes - // emits, which is where Envoy actually reads it. + // 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 timeout governs a route nothing uses. - routeTimeout := func(t *testing.T, x *XdsServer) time.Duration { + // 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 { @@ -582,7 +582,15 @@ func TestXdsServer_RouteTimeout(t *testing.T) { if got := action.GetCluster(); got != OriginalDstClusterName { t.Fatalf("workload route targets cluster %q, want %q", got, OriginalDstClusterName) } - return action.GetTimeout().AsDuration() + 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) { @@ -599,10 +607,13 @@ func TestXdsServer_RouteTimeout(t *testing.T) { } }) - // A zero value is what an operator who never passes --route-timeout would - // produce if the flag default were dropped. Envoy reads a zero route - // timeout as "no timeout at all", so the setter must ignore it rather than - // silently turning every stuck actor into a held-open request. + // 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) @@ -612,6 +623,29 @@ func TestXdsServer_RouteTimeout(t *testing.T) { } } }) + + // 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) {