Skip to content

Commit af593e8

Browse files
committed
feat(coderd): probe execute tokens in interrupt reconciliation and the execution sweep
1 parent 2d6b3c1 commit af593e8

7 files changed

Lines changed: 862 additions & 67 deletions

File tree

coderd/database/querier.go

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/queries.sql.go

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/queries/chattoolcallexecutions.sql

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,12 @@ RETURNING *;
199199
-- name: ClaimStaleChatToolCallExecutionCancels :many
200200
-- Claims a batch of cancel_requested rows whose reconciliation
201201
-- stalled (the post-interrupt pass lost its agent dial or died).
202-
-- Rows without recorded process identity are claimed too, so a
203-
-- server crash between the interrupt commit and reconciliation
204-
-- cannot strand them. Bumping updated_at inside the claim acts as
205-
-- a cross-replica lease so concurrent sweepers do not hammer the
206-
-- same unreachable agent; FOR UPDATE SKIP LOCKED keeps sweepers
207-
-- from serializing on each other.
202+
-- Rows without recorded process identity are claimed too: the
203+
-- sweeper resolves them through the dispatch target's token index.
204+
-- Bumping updated_at inside the claim acts as a cross-replica lease
205+
-- so concurrent sweepers do not hammer the same unreachable agent;
206+
-- FOR UPDATE SKIP LOCKED keeps sweepers from serializing on each
207+
-- other.
208208
WITH candidates AS (
209209
SELECT id
210210
FROM chat_tool_call_executions

coderd/x/chatd/ARCHITECTURE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -926,9 +926,9 @@ The execute timeout is clamped to 4 hours at claim time and the clamped value is
926926

927927
### Interrupt reconciliation
928928

929-
After the interrupt commit, the worker best-effort reconciles the `cancel_requested` rows: it dials the recorded agent with a short timeout and sends a kill signal. A confirmed outcome (the agent definitively has no such process, or a post-kill snapshot shows exit) resolves the row to `canceled`. A delivered but unconfirmed kill records `cancel_signal_sent_at` and leaves the row `cancel_requested` with its full process identity. Rows without a recorded process identity are polled through the record grace window (an interrupted in-flight `StartProcess` can still land its handle via the uncanceled record write); a handle that arrives gets the kill flow, and only after the window closes does the row resolve to `unknown`. The `unknown` write itself requires the row to still have no process identity: a handle landing concurrently wins the race, and the reconciler then runs the kill flow on the freshly identified process instead. The interrupt commit maps background rows to `detached` only when their handle is recorded, and the synthetic result committed for such a call carries the process handle: the process is deliberately left alive, and a generic cancellation would strand it without an addressable ID. A background row whose start was still in flight becomes `cancel_requested`: its committed synthetic result carries no handle, so once the late handle lands the reconciler kills the process like foreground work instead of leaving it running unaddressable. This runs after the commit, so a slow or unreachable agent never delays the interrupt.
929+
After the interrupt commit, the worker best-effort reconciles the `cancel_requested` rows: it dials the recorded agent with a short timeout and sends a kill signal. A confirmed outcome (the agent definitively has no such process, reports it already exited, or a post-kill snapshot shows exit) resolves the row to `canceled`. A delivered but unconfirmed kill records `cancel_signal_sent_at` and leaves the row `cancel_requested` with its full process identity, so a later reconciler can still act on it. Rows without a recorded process identity are resolved by probing the token index of the dispatch target recorded at claim time, never the chat's current agent: a found process is first adopted onto the row (so a transient kill failure leaves durable process identity for the sweep, not a token-only row) and then goes through the normal kill-and-confirm flow, a pending reservation or an absent token is re-probed until the record grace window since the claim closes (an interrupted in-flight `StartProcess` may not have reached the agent yet, so no reservation exists and a process can still appear moments after the commit), a trustworthy absent token resolves to `canceled` only after that window, and transport failures that persist leave the row for a later reconciler. When the token index cannot help (untrustworthy absence, agents without the probe route, no recorded dispatch target), the reconciler waits out the same grace window for a late handle write, since `RecordStart` runs on an uncanceled bound and may land after the commit; a handle that arrives gets the kill flow, and only then does the row resolve to `unknown`. The `unknown` write itself requires the row to still have no process identity: a handle landing concurrently wins the race, and the reconciler then runs the kill flow on the freshly identified process instead. The interrupt commit maps background rows to `detached` only when their handle is recorded, and the synthetic result committed for such a call carries the process handle: the process is deliberately left alive, and a generic cancellation would strand it without an addressable ID. A background row whose start was still in flight becomes `cancel_requested`: its committed synthetic result carries no handle, so a process identified late (by the record write or the token probe) is killed like foreground work instead of being left running unaddressable. This runs after the commit, so a slow or unreachable agent never delays the interrupt.
930930

931-
The immediate pass is best-effort, so a periodic per-replica sweep guarantees convergence: every minute it claims `cancel_requested` rows whose `updated_at` is older than the retry age (the claim bumps `updated_at` inside `FOR UPDATE SKIP LOCKED`, acting as a cross-replica lease), with each pass bounded by a deadline so a batch of unreachable agents cannot collapse the retry cadence. Rows with process identity re-run the kill flow until a terminal outcome is recorded; rows without it (a crash between the interrupt commit and reconciliation) get the same late-handle wait as the immediate pass and then resolve to `unknown`. An interrupted process is therefore eventually killed even when its agent was unreachable at interrupt time or the server died mid-reconciliation.
931+
The immediate pass is best-effort, so a periodic per-replica sweep guarantees convergence: every minute it claims `cancel_requested` rows whose `updated_at` is older than the retry age (the claim bumps `updated_at` inside `FOR UPDATE SKIP LOCKED`, acting as a cross-replica lease), with each pass bounded by a deadline so a batch of unreachable agents cannot collapse the retry cadence. The sweep re-runs the immediate pass's routing until a terminal outcome is recorded: rows with process identity get the kill flow, and rows without it go back through the token probe. A background process the probe finds is adopted onto the row and killed like foreground work: its committed synthetic result carries no handle, so sparing it would strand a running process. An interrupted process is therefore eventually killed even when its agent was unreachable at interrupt time or the server died mid-reconciliation.
932932

933933
Rows whose kill can never be confirmed still terminalize instead of sweeping forever. A row whose agent row is gone (deleted or soft-deleted, including the FK nulling `workspace_agent_id` on hard delete) resolves to `canceled`: the workspace pod is gone and the process died with it. This check runs before the give-up bound, so an old row with a provably dead process records `canceled` rather than `unknown`. A row whose agent merely stays unreachable resolves to `unknown` once a full day has passed since its cancellation (anchored on `result_committed_at`, which the cancelling transaction stamps; only the history-delete mapping deliberately re-stamps it forward to the edit time, which extends the budget, so a long-lived call interrupted a day after its dispatch still gets its full kill-retry budget); the kill was never confirmed, and the row records that honestly.
934934

0 commit comments

Comments
 (0)