Repository navigation
atenet/router: fail fast on FailedPrecondition instead of parking - #3472
Merged
Tim Hockin (thockin) merged 1 commit intoOct 30, 2026
Merged
Conversation
Before this commit, the router parked every FailedPrecondition from ResumeActor, so a request to a CRASHED or DELETING actor held a parking slot for the whole 5 s budget before it answered 503. Now the router no longer parks on FailedPrecondition: every such answer now fails on the first attempt. Signed-off-by: Yuan Gao <[email protected]>
Yuan Gao (ygao-g)
requested review from
Bowei Du (bowei) and
Julian Gutierrez Oschmann (juli4n)
October 29, 2026 23:50
Julian Gutierrez Oschmann (juli4n)
previously approved these changes
Oct 30, 2026
Yuan Gao (ygao-g)
requested review from
Benjamin Elder (BenTheElder) and
Dmitry Berkovich (dberkov)
October 30, 2026 15:45
Collaborator
|
SUSPENDING is something we will need to fix (post 1.0), I think. If a request comes in to resume an actor which is mid-suspend, we should probably abort the snapshot and resume. |
Bowei Du (bowei)
previously approved these changes
Oct 30, 2026
Tim Hockin (thockin)
previously approved these changes
Oct 30, 2026
Collaborator
Zoe Zhao (@zoez7) - could you please add it to the list of tasks we might need consider for lifecycle post GA |
Yuan Gao (ygao-g)
dismissed stale reviews from Tim Hockin (thockin), Bowei Du (bowei), and Julian Gutierrez Oschmann (juli4n)
via
October 30, 2026 16:20
cc975da
Yuan Gao (ygao-g)
force-pushed
the
explore-3269-minimal
branch
from
October 30, 2026 16:29
cc975da to
430cbf9
Compare
Tim Hockin (thockin)
enabled auto-merge
October 30, 2026 16:37
Tim Hockin (thockin)
approved these changes
Oct 30, 2026
Dmitry Berkovich (dberkov)
approved these changes
Oct 30, 2026
2 tasks done
1 of 2 tasks
Dmitry Berkovich (dberkov)
pushed a commit
that referenced
this pull request
Nov 2, 2026
) A routed request to an actor in ACTOR_STATE_CRASHED parks in atenet-router for the whole 5 s budget, makes about 19 `ResumeActor` calls, holds one parking slot throughout, and then answers 503. The state never clears on its own, so the wait buys nothing. Fixes #3269 Part of #1562 `retryable` in `cmd/atenet/internal/router/ingress/resumer.go` drops `FailedPrecondition` from the park arm, so the request fails on the first attempt with the same 503 and the same body. `ResourceExhausted` and `Unavailable` still park, `Aborted` is still always retried, and nothing changes with parking disabled. No ateapi change. Behavior change: every `FailedPrecondition` from `ResumeActor` now fails fast, not only CRASHED. That covers DELETING, SUSPENDING and REVERTING, a missing template, a volume without a storage ID, a worker without a node name, and a failed `DetachActor`. Two situations behind SUSPENDING and REVERTING: - An operation in progress. `SuspendActor`, `RevertActor` and `DeleteActor` hold the actor lease for their whole run, so a resume during one gets `Aborted`, which still parks. Nothing changes here. - A failed operation. A failed suspend/revert that fails at any step, leaves the actor SUSPENDING/REVERTING with no lease held. The resume then gets `FailedPrecondition` and now fails fast. A retried suspend or revert is what moves the actor on, never the resume retry. The status code and the body are unchanged. The e2e `TestRoutedRequestToCrashedActorFailsFast` crashes an actor by deleting its worker pod and routes one request: 503 naming the state within 2 s, where a parked request would first wait out the 5 s budget. It skips on the AgentGateway lane, whose resume path is outside this repo. `docs/concepts/request-parking.md` now lists `FailedPrecondition` under fail fast. Verified with `go test -race -count=3 -shuffle=on` on the router package, `hack/verify-all.sh`, and the demo e2e suite on kind. AI use: AI-assisted under my direction; I reviewed the diff and ran the tests. - [x] Tests pass - [x] Docs updated Signed-off-by: Yuan Gao <[email protected]> (cherry picked from commit 542eca1) Signed-off-by: Eitan Yarmush <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A routed request to an actor in ACTOR_STATE_CRASHED parks in atenet-router for the whole 5 s budget, makes about 19
ResumeActorcalls, holds one parking slot throughout, and then answers 503. The state never clears on its own, so the wait buys nothing.Fixes #3269
Part of #1562
retryableincmd/atenet/internal/router/ingress/resumer.godropsFailedPreconditionfrom the park arm, so the request fails on the first attempt with the same 503 and the same body.ResourceExhaustedandUnavailablestill park,Abortedis still always retried, and nothing changes with parking disabled. No ateapi change.Behavior change: every
FailedPreconditionfromResumeActornow fails fast, not only CRASHED. That covers DELETING, SUSPENDING and REVERTING, a missing template, a volume without a storage ID, a worker without a node name, and a failedDetachActor. Two situations behind SUSPENDING and REVERTING:SuspendActor,RevertActorandDeleteActorhold the actor lease for their whole run, so a resume during one getsAborted, which still parks. Nothing changes here.FailedPreconditionand now fails fast. A retried suspend or revert is what moves the actor on, never the resume retry.The status code and the body are unchanged.
The e2e
TestRoutedRequestToCrashedActorFailsFastcrashes an actor by deleting its worker pod and routes one request: 503 naming the state within 2 s, where a parked request would first wait out the 5 s budget. It skips on the AgentGateway lane, whose resume path is outside this repo.docs/concepts/request-parking.mdnow listsFailedPreconditionunder fail fast. Verified withgo test -race -count=3 -shuffle=onon the router package,hack/verify-all.sh, and the demo e2e suite on kind.AI use: AI-assisted under my direction; I reviewed the diff and ran the tests.