atelet: cache pool resolutions so a list flap cannot split the CPU counter - #1479
Open
Tim Bai (baizhenyu) wants to merge 2 commits into
Open
atelet: cache pool resolutions so a list flap cannot split the CPU counter#1479Tim Bai (baizhenyu) wants to merge 2 commits into
Tim Bai (baizhenyu) wants to merge 2 commits into
Conversation
…unter The sweep resolved worker pools with one fresh pod list per tick, and a failed list grouped that tick's samples without the pool labels. For the gauges that is a one-tick blink; for ate.actor.stats.cpu.time it is permanent -- a counter binds every increment to its attribute set, so one flap re-homes that sweep's CPU increase onto a pool-less series that then coexists with the pooled one forever: pool-filtered rates dip on every flap, a phantom unlabeled series bursts exactly during apiserver trouble, and the split never heals. A pod's pool is immutable for the pod's lifetime, so caching by pod UID carries no correctness risk: resolveWorkerPools lets fresh resolutions win, falls back to cachedPools when the list fails or misses a pod, and rebuilds the cache restricted to the pods whose ateom directories exist -- the same prune-to-the-living bound lastCPU keeps. The residual unlabeled case is a pod first seen while the apiserver is unreachable, which heals on the next good list. Fixes agent-substrate#1478. Part of agent-substrate#896, toward agent-substrate#550.
Tim Bai (baizhenyu)
force-pushed
the
statspoller-pool-cache
branch
from
September 4, 2026 20:16
8dbe905 to
ba4cd68
Compare
fetchWorkerPools (was workerPools) is a func field that reads like map access at its call sites; the verb also carries the property the cache design revolves around -- expensive, remote, can fail -- while resolveWorkerPools keeps the answer-by-whatever-means role. The constructor follows as newWorkerPoolFetcher.
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.
Fixes #1478.
One failed pod list per sweep grouped that tick's samples without
ate.workerpool.*labels. Gauges blink and heal; the CPU counter does not: an OTel counter binds increments to their attribute set, so a single flap re-homes that sweep's CPU increase onto a newly-born pool-less series that coexists with the pooled one forever — pool-filteredrate()dips on every flap, a phantom unlabeled series bursts exactly during apiserver trouble, and cardinality doubles per flapped group for the process lifetime.Fix
A pod's pool is immutable for the pod's lifetime (the
ate.dev/worker-poollabel is stamped at creation; pods never move between pools), so caching resolutions per pod UID carries zero correctness risk.resolveWorkerPools:cachedPoolswhen the list fails outright or misses one pod,lastCPUuses.The residual unlabeled case shrinks to "pod first seen while the apiserver is unreachable", which heals on the next good list — and is pinned by a test as the documented behavior.
Testing
hack/verify-all.shgreen.Part of #896, toward #550.
Note: cache lifetime is tied to ateom directories, which nothing cleans up
cachedPoolsevicts an entry on the first sweep after its ateom directory disappears — but today nothing removesateoms/<podUID>/when a worker pod is deleted (the only cleanup anywhere is an ateom clearing its own stale socket at boot; directories accumulate until node replacement). Repo archaeology found no decision behind this: the directory set had no reader until the stats sweep (#961) promoted it into a discovery registry, so GC was never needed before.Consequences for this PR, sized honestly:
The durable fix is directory GC (atelet holds both halves of the evidence each sweep: the dir set and the node's pod list; "absent from the pod list AND socket dead for N consecutive sweeps → remove" is safe against half-born ateoms and transient list failures). That is a pre-existing issue being filed separately; when it lands, cache eviction follows it automatically with zero changes here.