Skip to content

Delete ate.scheduler.eligible_workers - #1804

Merged
Julian Gutierrez Oschmann (juli4n) merged 1 commit into
agent-substrate:mainfrom
JeffLuoo:delete-eligible-workers-metric
Sep 22, 2026
Merged

Julian Gutierrez Oschmann (juli4n) merged 1 commit into
agent-substrate:mainfrom
JeffLuoo:delete-eligible-workers-metric

Conversation

@JeffLuoo

@JeffLuoo Jeff Luo (JeffLuoo) commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1802.

What this does

Deletes the ate.scheduler.eligible_workers histogram. Nothing replaces it in this PR.

Why

It is expensive on the resume path. Schedule built a second slice of the matching workers that scheduling never read, walked it again, and repeated HasRoom for each entry. HasRoom parses resource quantity strings up to three times for each worker. The metric roughly doubled the per-worker arithmetic of a placement. The filter loop is now one pass over the fleet and builds one slice.

A metric cannot answer the question it was built for. #564 wanted to tell a full fleet apart from an empty intersection of the constraints, such as a cordoned node behind a node requirement. That needs the selectors, the node requirement and the actor. docs/metrics/substrate.yaml bars all three from a metric label and sends them to logs and spans, so no shape of this instrument reaches the answer. A log record at the rejection is the replacement the issue names, and it is not in this PR.

Also removed

ate.scheduling.constraint goes with the instrument. No other signal used the attribute.

Scope of the change

  • cmd/ateapi/internal/scheduling/metrics.go deleted, along with the WithMeter option and the second filter pass in Schedule.
  • The instrument and the attribute removed from docs/metrics/registry/metrics.yaml. The pool-keys-paired exception in docs/metrics/substrate.yaml dropped, because the empty pool pair was this instrument's alone.
  • docs/observability.md: the table row and the two label notes removed.
  • The e2e collector check and the label assertions for the instrument removed.
  • TestSchedule_EligibleWorkersMetric removed. The behaviors it covered (draining workers, a sandbox class mismatch, busy workers) are already in the TestSchedule table.

Testing

go test ./cmd/... ./internal/... passes. make verify passes except hack/verify/metrics.sh, which needs Weaver or Docker. Neither is available on this machine, so CI is what checks the registry.

The metric is expensive on the resume path. It built a second slice of
the matching workers that scheduling never read, walked it again, and
repeated the room check for each entry. That check parses resource
quantity strings up to three times for each worker. The metric roughly
doubled the per-worker arithmetic of a placement. The filter loop is now
one pass over the fleet and builds one slice.

The metric also cannot answer the question it was built for. Telling a
full fleet apart from an empty intersection of the constraints needs the
selectors, the node requirement and the actor. The cardinality rules bar
all three from a metric label and send them to logs and spans, so no
shape of this instrument reaches the answer.

ate.scheduling.constraint goes with it. No other signal used the
attribute.
@JeffLuoo
Jeff Luo (JeffLuoo) force-pushed the delete-eligible-workers-metric branch from 1719cdb to 5993bfa Compare September 22, 2026 20:37
@JeffLuoo Jeff Luo (JeffLuoo) changed the title Delete ate.scheduler.eligible_workers from the resume path Delete ate.scheduler.eligible_workers Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Merged via the queue into agent-substrate:main with commit 9e503c4 Sep 22, 2026
11 checks passed
@bowei Bowei Du (bowei) added area/observability area/scheduling kind/cleanup Small fixes that are not bugs, for example a typo in a code comment labels Sep 29, 2026
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/observability area/scheduling 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.

Delete ate.scheduler.eligible_workers from resume path

3 participants