Repository navigation
Delete the ateerrors failure-reason taxonomy - #1817
Merged
Luiz Oliveira (laoj2) merged 1 commit intoSep 23, 2026
Merged
Conversation
Zoe Zhao (zoez7)
force-pushed
the
delete-ateerrors
branch
2 times, most recently
from
September 23, 2026 17:22
079e58f to
2774f0e
Compare
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.
Zoe Zhao (zoez7)
force-pushed
the
delete-ateerrors
branch
from
September 23, 2026 18:33
2774f0e to
c354a97
Compare
Luiz Oliveira (laoj2)
approved these changes
Sep 23, 2026
Merged
via the queue into
agent-substrate:main
with commit Sep 23, 2026
74bbfc5
14 of 15 checks passed
2 tasks done
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)
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.
Stacked on #1220 — review only the last commit until that merges