feat: configurable Envoy route timeout for long-running actor requests - #714
feat: configurable Envoy route timeout for long-running actor requests#714Maya Wang (mayawang) wants to merge 1 commit into
Conversation
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.
Ron Lev (ronlv10)
left a comment
Ron Lev (ronlv10)
left a comment
There was a problem hiding this comment.
Thanks for creating this PR! We encountered the same issue when trying to use substrate.
| 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") |
There was a problem hiding this comment.
Have you considered making it configurable per actor-template (or per request)? I think that making it global for the whole router might be too broad, as this timeout could vary between actor types.
There was a problem hiding this comment.
for this we would need an api, right? this can follow as a follow up issue. Any objection Ron Lev (@ronlv10) ?
| } | ||
| }) | ||
|
|
||
| // A zero value is what an operator who never passes --route-timeout would |
There was a problem hiding this comment.
is this true ?
the config is cmd.Flags().DurationVar(&cfg.RouteTimeout, "route-timeout", defaultRouteTimeout, no ?
Maya Wang (@mayawang) can you clarify what do you mean here?> |
Lior Lieberman (LiorLieberman)
left a comment
Lior Lieberman (LiorLieberman)
left a comment
There was a problem hiding this comment.
Have we considered using idle_timeout vs timeout here?
idle_timeout sets the maximum time that a stream can exist without any network activity (which feels more relevant for our usecase?). The timer resets every time a byte is sent or received.
You need both. idle_timeout is how long ateom can remain suspended without breaking network connections. request_timeout is how long request can be in-flight, potentially across multiple suspend/resume cycles. |
4 participants
Summary
Envoy's end-to-end timeout on the workload route is hardcoded at
10sinbuildRoutes. An actor that legitimately holds a request open longer gets cutoff: a harness relaying an LLM completion keeps the request open for the whole
generation, and the client sees a 504 mid-turn.
Adds
--route-timeouton atenet-router. The default is 10s, so behavior isunchanged unless an operator passes the flag.
Changes
cmd/atenet/internal/router/— addsXdsServer.routeTimeoutwith aSetRouteTimeoutsetter and adefaultRouteTimeoutconst, wired fromrouterConfig.RouteTimeout/--route-timeout. Same shape as the adjacentSetExtProcMessageTimeoutandSetExtProcMaxRequests, and a flag on theexisting config struct rather than an env read, matching the convention the
parked-request work established.
A non-positive value leaves the default in place, since Envoy reads a zero route
timeout as no timeout at all.
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.manifests/ate-install/atenet-router.yamldocuments it as a commented-out entry.Verification
go build ./...,go vet ./...,go test ./...— all pass.xds_test.goreads the timeout back out ofbuildRoutes, where Envoyactually picks it up: default, setter override, and
non-positive-keeps-default. The helper pins that route to
OriginalDstClusterName— a change that moved actor traffic onto some otherroute would otherwise leave the test passing while the timeout governed a
route nothing uses.
On a live GKE cluster, read back out of Envoy's own
/config_dump. Withthe new image and no flag, the workload route reports
timeout: 10s, so thedefault is genuinely unchanged. With
--route-timeout=5mit reportstimeout: 300s. Same binary, same manifest, only the flag differs.Caveat on that measurement: it was taken before
ingress: route actor ingress through the atunnel mTLS serverlanded, so the route it read was the olddynamic_forward_proxypath to pod-IP:80. After rebasing, the timeoutattaches to the
actor_original_dstroute that replaced it — which is nowpinned by the test above rather than left to inspection.
Regression, resume with parking on the path: a conversation actor that had
been suspended for 4 days was resumed by an ordinary request through the
router — HTTP 200 in 3.74s, exactly one parked request,
parking_wait_duration_seconds{outcome="served"} = 3.459s, no shed and nobudget_exhausted.Relationship to #465
This is a stopgap for the connected-socket suspend/restore problem tracked in
#465 (suspend-safe actor networking). Once actor network traffic survives
checkpoint/restore natively, much of the need to raise this ceiling should go
away; this just makes the current behavior tunable in the meantime.
Fixes #<issue_number_goes_here>