Skip to content

Delete the ateerrors failure-reason taxonomy - #1817

Merged
Luiz Oliveira (laoj2) merged 1 commit into
agent-substrate:mainfrom
zoez7:delete-ateerrors
Sep 23, 2026
Merged

Luiz Oliveira (laoj2) merged 1 commit into
agent-substrate:mainfrom
zoez7:delete-ateerrors

Conversation

@zoez7

Copy link
Copy Markdown
Collaborator

Stacked on #1220 — review only the last commit until that merges

@zoez7
Zoe Zhao (zoez7) force-pushed the delete-ateerrors branch 2 times, most recently from 079e58f to 2774f0e Compare September 23, 2026 17:22
@zoez7
Zoe Zhao (zoez7) marked this pull request as ready for review September 23, 2026 17:24
With every atelet error crashing the actor, the Reason taxonomy no
longer drives any behaviour: the tags only labeled crash telemetry, and
for atelet-originated errors the label already read UNKNOWN. Remove the
classification machinery end to end rather than keep maintaining a
bounded enum nothing consumes:

- Delete internal/ateerrors and the %w Reason tags at their source
  sites; the one in-process decision that matched on a Reason
  (uploadLocalCheckpointDir probing for an already-uploaded snapshot)
  now matches on the new ategcs.ErrObjectNotFound sentinel instead.
- Drop ate.failure.reason and ate.failure.domain from the actor crash
  counter, the snapshot duration histograms, the Crashed log event, and
  the crash log records.
- Retire crashActorOnError, and with it the synthetic DataLoss status it
  returned after every crash. A workflow step that crashes the actor
  now returns the atelet error itself, so the caller sees the real
  failure with its own status code rather than one fixed code for
  every cause.
- Update the Weaver registry (metrics, events, cardinality rules) and
  the observability docs to match.
@laoj2
Luiz Oliveira (laoj2) added this pull request to the merge queue Sep 23, 2026
Merged via the queue into agent-substrate:main with commit 74bbfc5 Sep 23, 2026
14 of 15 checks passed
@bowei Bowei Du (bowei) added area/api-machinery area/node kind/cleanup Small fixes that are not bugs, for example a typo in a code comment labels Sep 29, 2026
pull Bot pushed a commit to RolfLobo/substrate that referenced this pull request Sep 30, 2026
agent-substrate#1941)

## Benchamark(atelet, ateom-microvm): log per-actor checkpoint and
restore phase breakdowns

### Why

`SuspendActor` and `ResumeActor` latency is only attributable down to
the `ate.actor.{checkpoint,restore}.duration` histogram phases, and the
biggest buckets — `ateom_checkpoint`, `ateom_restore` — are opaque. When
a large memory benchmark suspend takes 6 s we want to able to tell where
the time is been spent during the suspend.

### What

One joinable, developer-facing log record per operation per layer, with
the full actor identity (allowed in logs, barred from metric labels),
the snapshot scope, and one float-seconds field per phase that ran.

**atelet**
- `Checkpoint` now writes a `Checkpoint timing breakdown` record, the
same way `Restore` has written `Restore timing breakdown` since agent-substrate#1364.
Phases: `sandbox_assets`, `ateom_checkpoint`, `persist`, `total`. One
slice feeds both the histogram and the record, so they cannot disagree.
- Both records carry `error.type` when the operation failed (the gRPC
code; context errors map to `DeadlineExceeded` / `Canceled`). The record
is written on the way out of a failure too, so its completed phases are
kept, and the marker lets a reader exclude a timed-out restore from a
latency distribution. This restores what the record lost when the
`ateerrors` taxonomy was deleted (agent-substrate#1817), using the `error.type`
convention the ateapi instruments already follow.

**ateom-microvm**
- New `phaselog.go`. `CheckpointWorkload` and `RestoreWorkload` emit
records with the same two messages, under
`ateom.actor.checkpoint.duration.<phase>` and
`ateom.actor.restore.duration.<phase>`, decomposing atelet's
`ateom_checkpoint` / `ateom_restore` buckets:
- checkpoint: `pause`, `snapshot`, `durable_dir`, `rootfs_upper`,
`teardown`, `total` — the three captures run concurrently on the paused
guest, so the paused window costs their max, not their sum.
- restore: `prep`, `bundles`, `upper_join`, `lowers`, `tap`,
`vmm_launch`, `vm_restore`, `resume`, `wakeup_probe`, `total` —
sequential; they partition the total. A Data-scope cold boot records
`total` only.
- These timings already existed as ad-hoc `slog.Duration` fields on the
`Actor checkpointed` / `Actor restore phases` lines; those lines are
kept. The record adds stable keys, identity, and seconds (the
histograms' unit).
- The phase names are deliberately private to the binary rather than
added to `internal/ateattr`, so they cannot be mistaken for
`ate.snapshot.phase` metric values. They are micro-VM specific;
ateom-gvisor is unchanged.

**docs/observability.md** is updated: the Restore record is no longer
the only per-actor latency record, and the ateom records are described.

No new instruments, no registry changes, no behavior change.

### Example

```json
{"msg":"Checkpoint timing breakdown","ate.actor.uid":"8f2a…","ate.template.name":"glutton",
 "ate.snapshot.scope":"full",
 "ateom.actor.checkpoint.duration.pause":0.003,
 "ateom.actor.checkpoint.duration.snapshot":0.846,
 "ateom.actor.checkpoint.duration.rootfs_upper":0.022,
 "ateom.actor.checkpoint.duration.teardown":0.232,
 "ateom.actor.checkpoint.duration.total":1.081}
```

Joined with atelet's record for the same actor, a run of the glutton
workload (1 GiB resident, microVM) attributes a 6.0 s p50 suspend as 75%
`persist`, 19% `ateom_checkpoint` (of which the CH `snapshot` is 0.85 s
and `teardown` 0.23 s), and a 5.3 s p50 resume as 79% `download`, 18%
`ateom_restore` (of which `vm_restore` is 0.73 s). The consumer that
produces those tables from pod logs is a separate
`benchmarking/analysis` PR.

### Testing

- `go test ./cmd/atelet/...` and `./cmd/ateom-microvm/...` pass; new
unit tests cover the record shape (seconds, identity keys, zero phases
absent, no duplicate keys), the scope mapping, and `error.type` for
gRPC, context and plain errors.
- `GOOS=linux go vet` clean for both binaries; boilerplate and gofmt
clean.
Timo Derstappen (teemow) added a commit to giantswarm/substrate that referenced this pull request Oct 12, 2026
…at the resume that crashed it

A golden actor whose image the registry refuses is crashed by the
resume at once (the control plane crashes an actor on every failed
atelet Run or Restore since agent-substrate#1220) and the
cause is recorded on the actor (ActorStatus.crash since agent-substrate#1867). The
ActorTemplate reconciler did not read it: a resume error was requeued
with backoff, the next pass found CRASHED and failed the template with
the fixed text "golden actor crashed before its snapshot was taken",
so a consumer watching the template (kagent reports Ready=False
ActorTemplateFailed) could not tell a missing image from anything else.

The reconciler now fails the template with the actor's recorded crash
message wherever it finds the golden actor CRASHED, and re-reads the
actor when its resume fails so a crash that resume caused fails the
template in the same pass, with the registry's answer in the message
(`GoldenActorCrashed: actor ate-golden/<uid> crashed: resume failed:
atelet Run: ... MANIFEST_UNKNOWN: manifest unknown`).

Re-pin note: on v0.2.0-beta5 this patch classified the registry's
final word in the image cache (ateerrors.ReasonFailedGetExternalObject)
and claimed it at atelet's Run and Restore boundaries; v0.3.0-alpha1
crashes on every atelet failure and deleted the ateerrors taxonomy
(agent-substrate#1817), so only the reconciler half remains.

(cherry picked from commit b1d759c)
Timo Derstappen (teemow) added a commit to giantswarm/substrate that referenced this pull request Oct 12, 2026
… restore keeps exceeding its budget

The atelet Restore step of a Resume workflow had one attempt and no
budget of its own: a restore that ran out of time (cancelled, or the
workflow deadline reached during a cold image pull) failed the resume
and the crash record was written under the same exhausted context, so
the write failed too and the actor stayed RESUMING on a claimed worker.
Nothing reclaims that: the next resume fast-forwards to the same step,
and until one succeeds the actor cannot be deleted (FailedPrecondition).

Each restore attempt now runs under --actor-restore-budget (default
90 s, a cold pull and unpack included; 0 keeps the workflow deadline as
the only bound). An attempt that exceeds it, or that could not reach
atelet (Unavailable), is retried: three attempts, a doubling pause,
within --actor-workflow-deadline; the layers an interrupted pull
fetched stay in the node's cache, so a retry makes progress. A restore
that keeps exceeding its budget, or one running when the workflow
deadline arrives, crashes the actor with the budget and the attempt
count in its crash message (ActorStatus.crash), and every crash the
Restore boundary records is written detached from the workflow context
under its own bound, so it lands when the failure was that deadline
itself: CRASHED, worker released, deletable.

A lost lease still ends the workflow untouched (another one owns the
actor), and an error atelet returned keeps its own text in the crash
message, as before.

Re-pin note: on v0.2.0-beta5 the timeout was the AIP-193 reason
RESTORE_TIMED_OUT with the crash directive, read by maybeCrashActor;
v0.3.0-alpha1 crashes an actor on every failed Restore and deleted the
ateerrors taxonomy (agent-substrate#1817), so the outcome is a typed error the Restore
boundary turns into the crash message.

(cherry picked from commit 69fe153)
Timo Derstappen (teemow) added a commit to giantswarm/substrate that referenced this pull request Oct 12, 2026
… and crash it when no durable snapshot remains

A PAUSED actor's local snapshot lives on the node its worker ran on, and
the Resume workflow pins the actor to that node (Constraints.RequiredNodes).
When the node had left the cluster -- a spot interruption, a scale-down, a
node roll -- nothing noticed: the scheduler found no worker on the node and
AssignWorker answered ResourceExhausted "no free workers available", the
message of a full pool. The ingress dataplane parks a request on that
status, so every resume waited out the whole parking budget for an error
no wait could clear, the consumer saw a timeout, and the actor stayed
PAUSED for good: a suspend dialled the atelet on the dead node and stayed
retryable, so the actor could not be suspended and -- a delete suspends
first -- not be deleted either (agent-substrate#398 names the
case; agent-substrate#397 the empty-node variant).

The scheduler now says what its pass over the fleet saw when a node
restriction was in effect: Schedule returns a NoCapacityError -- still
errors.Is ErrNoCapacity -- carrying the restricted nodes, the eligible
workers found on them (all full) and the ACTIVE workers of any pool that
report them. AssignWorker classifies it:

  - eligible workers on the node, all full: ResourceExhausted, "all
    eligible workers on those nodes are busy", the node named, parked as
    before;
  - the node still in the cluster but no eligible worker on it (a pool
    rolling, a pool moved off the node): ResourceExhausted with the node
    named, still parked -- a worker may come back;
  - the node gone from the cluster: the snapshot is lost. Without a
    durable snapshot to fall back to the actor is crashed with the reason
    LOCAL_SNAPSHOT_GONE (CRASHED, ate.actor.crashes labelled, deletable)
    and the caller gets DataLoss with the crash directive; with one, the
    actor is left as it is and the caller gets DataLoss naming the node.

A node counts as gone only when no ACTIVE worker of any pool reports it
and its Node object is absent, read from a Node informer ate-api-server
now runs (RBAC: nodes get/list/watch). A worker's absence alone is no
proof: a pool rolling, an ateom restarting or the atelet DaemonSet
updating leave a node without an ACTIVE worker for a while, with its disk
and the snapshot on it intact. Without a Node lister nothing is declared
lost.

The paused-origin suspend takes the same exit: an atelet missing on a node
that is gone crashes the actor (or fails fast when a durable snapshot
remains) instead of staying retryable forever, so the owner can delete
the actor.

DataLoss is the one status the dataplane never parks on -- it retries
Aborted, and while parking is enabled ResourceExhausted, FailedPrecondition
and Unavailable -- so the caller learns within one round trip. The atenet
router maps it to 410 with the cause in the body, the way it preserves the
capacity message.

The read side of agent-substrate#397: empty entries in
node_vms_with_local_snapshots are ignored when the constraints are built
and when the suspend picks the node to dial, so a record poisoned with
[""] by an older pause finalization is schedulable again (the write side
already refuses an unknown node).

Signed-off-by: Timo Derstappen <[email protected]>
Re-pin note: on v0.2.0-beta5 the terminal answer carried the AIP-193
reason LOCAL_SNAPSHOT_GONE and the crash directive; v0.3.0-alpha1
deleted the ateerrors taxonomy (agent-substrate#1817) and records a crash on the actor
(ActorStatus.crash, agent-substrate#1867), so the refusal is a plain DataLoss status
and the crash message names the lost snapshot and its nodes. The
scheduler's eligible-workers metric upstream deleted (agent-substrate#1804) is not
re-added; NoCapacityError keeps its counts.

(cherry picked from commit 68ec85b)
Timo Derstappen (teemow) added a commit to giantswarm/substrate that referenced this pull request Oct 12, 2026
…kground and resume it on any worker

A pause checkpoints the workload onto its worker's node and nowhere else:
the node holds the only copy of the actor's state, and the record pins the
actor to that node (LocalSnapshotInfo.node_vms_with_local_snapshots →
Constraints.RequiredNodes). A node that goes away — preempted, scaled down,
rolled — takes the state along while the record still says PAUSED there. The
next resume then finds no worker that applies, the scheduler reports the
locality miss as ResourceExhausted "no free workers available" (the message of
a full pool), the router parks the request for its whole budget and every
retry repeats this; a suspend dials the atelet on the dead node and stays
retryable forever, so the actor cannot be suspended or deleted either.
Actors parked on a question for a person sit like this for hours.

Pauses now become durable shortly after they complete. After FinalizePaused
the control plane queues the actor with a background uploader (two workers
per replica, rate-limited requeues, a resync every minute that also finds
pauses from before an upgrade or a restart). The upload goes through the
atelet on the snapshot's node, the path a suspend of a paused actor already
uses, with two differences: the destination is named after the local
snapshot, so a retry re-sends the same request and overwrites the same
objects, and the atelet keeps the local copy (UploadPausedCheckpointRequest
.keep_local) — the actor stays PAUSED and its node remains the fast path. On
success the copy is recorded as the actor's external snapshot with the local
snapshot it came from (ExternalSnapshot.source_local_snapshot_name), under a
version precondition: no lease is held while the checkpoint streams, a
resume that arrives meanwhile is not delayed, and an upload the actor has
outrun is deleted again. The external snapshot the copy replaces is released
once the record points at the copy.

That marker is what the rest keys on. A paused actor's external snapshot is
its durable copy only while it names the local snapshot the actor holds;
the snapshot of an earlier suspend, or the upload of an earlier pause, is
older state and never stands in for the pause. While no copy exists the
node stays a hard constraint, exactly as before. Once one exists:

  - Placement prefers the snapshot's nodes (Constraints.PreferredNodes,
    ranked ahead of every other eligible worker, excluding none) instead of
    requiring them. Restore takes the local snapshot when the chosen worker
    is on such a node and the durable copy — with the scope the pause
    captured, memory included — anywhere else. A paused actor placed off its
    node with no copy is refused before any atelet is dialed rather than
    restored from an older snapshot.
  - A suspend commits the copy as captured: nothing is uploaded, no
    destination minted, the node not dialed except to drop the local copy
    best-effort (the new PruneLocalCheckpoints RPC, bounded to ten seconds).
    Narrowing the copy to the template's commit scope would need the node
    the actor may be leaving.
  - Deleting a paused actor collects the copy with the actor's prefix, as
    for a suspended actor, and asks the node to drop the local copy where an
    atelet answers.

The upload uses the scope the pause captured, not the commit scope: the
copy must restore exactly as the local snapshot would. Tests cover the
marker, the resync selection and its grace, the constraint switch, the
record commit with the replaced snapshot released, the stale upload
discarded and the concurrent replica's record left alone, the suspend
shortcut at mark and upload, the placement guard, the scheduler's preference
and its fallbacks, and — end to end against the fake atelet — the upload
after a pause, the resume onto another node restoring the copy after the
snapshot's node lost its worker, the resume that still prefers the node, the
suspend that uploads nothing, and the delete that releases both copies.

Signed-off-by: Timo Derstappen <[email protected]>
Re-pin note (v0.3.0-alpha1): ExternalSnapshot.source_local_snapshot_name
is field 4 (upstream took 3 for actor_template_uid); LocalSnapshotInfo is
LocalSnapshot (agent-substrate#1856) and SnapshotsConfig is SnapshotConfig (agent-substrate#1791); the
background upload no longer reads a crash directive from atelet (the
ateerrors taxonomy is gone, agent-substrate#1817): an upload atelet refuses is retried
by the resync and the actor's next resume answers with the cause; the
node-gone refusal is a plain DataLoss status. Upstream deleted the
python stubs, so none are regenerated.

(cherry picked from commit 31ae728)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api-machinery area/node kind/cleanup Small fixes that are not bugs, for example a typo in a code comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants