Repository navigation
runtime/session: suspend actors after abandoned delivery - #3635
shashankram wants to merge 1 commit into
Conversation
\# Description Canceled delivery can leave a restored actor running without a turn, holding worker capacity indefinitely. - Retain expired dispatch attempts for the existing lifecycle worker to process. - Claim suspension only when no active turn or newer delivery owns the Session. - Preserve snapshots and logical readiness so subsequent messages resume normally. - Cover cancellation, competing lifecycle work, retries, and subsequent delivery. \# Change Type ``` /kind fix ``` \# Changelog ```release-note Actors left running after canceled message delivery now suspend and release worker capacity automatically. ``` \# Additional Notes Signed-off-by: Shashank Ram <shashank.ram@solo.io>
| -- +goose Up | ||
| ALTER TABLE session | ||
| ADD COLUMN dispatch_cleanup_executor_id UUID, | ||
| ADD CONSTRAINT session_dispatch_cleanup_claim CHECK ( |
There was a problem hiding this comment.
This CHECK makes a rollback within 1.0 unsafe. A rollback keeps this schema, because the runner accepts a newer database. The older code clears dispatch_id on the first task save, on revoke, on deletion and on crash recovery. If the row has a claim, each of these writes fails the CHECK. Then the session cannot start a turn, and each delete request fails.
A controller that stops during cleanup leaves a claim. So the rollback itself can cause this problem. The Down migration also refuses to run while these rows exist. I reproduced the problem at d30cb0b (SQLSTATE 23514).
Two options:
- Drop the CHECK. Also make each query that checks for a claim require
dispatch_id IS NOT NULL. I prefer this option. - Keep the CHECK. Document that a rollback needs zero claimed rows, and document how to clear them.
| } | ||
| if err != nil { | ||
| logging.FromContext(ctx).ErrorContext(ctx, "abandoned dispatch suspension outcome unknown", "session_id", work.Session.Id, "dispatch_id", work.DispatchID, "error", err) | ||
| return nil // A possibly issued call cannot safely release admission. |
There was a problem hiding this comment.
What happens to this claim after a restart? The claim stays after a shutdown or after any error other than Aborted. GetPendingSessionDispatchCleanup skips claimed rows, so no worker gets the claim again. Each new message then gets ErrDispatchBusy until crash recovery or force deletion.
Task quiescence has the same limit today. But this path runs on every canceled delivery. So each rollout can strand a claim.
Could a new owner adopt the claim and only send the suspend again? If not, please document how to clear a stuck session.
Description
Canceling a message while its actor finishes restoring can leave the actor RUNNING without a turn, holding worker capacity indefinitely. The existing Session lifecycle worker now suspends that actor and keeps the Session READY for the next message.
Uncertain suspension outcomes retain the claim, consistent with existing automatic task suspension.
Cancellation cleanup flow
sequenceDiagram participant G as Gateway participant D as Session store participant W as Lifecycle worker participant S as Substrate G->>D: Expire delivery and retain dispatch ID G->>W: Wake existing worker W->>D: Find expired dispatch W->>S: Read actor identity and state alt Actor is RUNNING W->>D: Claim if dispatch matches and no active turn alt Claim succeeds W->>S: Suspend actor and save snapshot S-->>W: Confirm snapshot and suspension W->>D: Record snapshot and clear dispatch and claim end endTesting
Change Type
Changelog