Repository navigation
Delete ate.scheduler.eligible_workers - #1804
Merged
Julian Gutierrez Oschmann (juli4n) merged 1 commit intoSep 22, 2026
Merged
Julian Gutierrez Oschmann (juli4n) merged 1 commit into
Julian Gutierrez Oschmann (juli4n) merged 1 commit into
Conversation
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.
Jeff Luo (JeffLuoo)
force-pushed
the
delete-eligible-workers-metric
branch
from
September 22, 2026 20:37
1719cdb to
5993bfa
Compare
Jeff Luo (JeffLuoo)
requested review from
Da Huang (git286) and
Julian Gutierrez Oschmann (juli4n)
September 22, 2026 20:44
Julian Gutierrez Oschmann (juli4n)
approved these changes
Sep 22, 2026
Julian Gutierrez Oschmann (juli4n)
left a comment
Collaborator
There was a problem hiding this comment.
Thanks!
Julian Gutierrez Oschmann (juli4n)
enabled auto-merge
September 22, 2026 20:55
1 of 2 tasks
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)
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.
Closes #1802.
What this does
Deletes the
ate.scheduler.eligible_workershistogram. Nothing replaces it in this PR.Why
It is expensive on the resume path.
Schedulebuilt a second slice of the matching workers that scheduling never read, walked it again, and repeatedHasRoomfor each entry.HasRoomparses 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.yamlbars 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.constraintgoes with the instrument. No other signal used the attribute.Scope of the change
cmd/ateapi/internal/scheduling/metrics.godeleted, along with theWithMeteroption and the second filter pass inSchedule.docs/metrics/registry/metrics.yaml. Thepool-keys-pairedexception indocs/metrics/substrate.yamldropped, because the empty pool pair was this instrument's alone.docs/observability.md: the table row and the two label notes removed.TestSchedule_EligibleWorkersMetricremoved. The behaviors it covered (draining workers, a sandbox class mismatch, busy workers) are already in theTestScheduletable.Testing
go test ./cmd/... ./internal/...passes.make verifypasses excepthack/verify/metrics.sh, which needs Weaver or Docker. Neither is available on this machine, so CI is what checks the registry.