Skip to content

atelet: cache pool resolutions so a list flap cannot split the CPU counter - #1479

Open
Tim Bai (baizhenyu) wants to merge 2 commits into
agent-substrate:mainfrom
baizhenyu:statspoller-pool-cache
Open

atelet: cache pool resolutions so a list flap cannot split the CPU counter#1479
Tim Bai (baizhenyu) wants to merge 2 commits into
agent-substrate:mainfrom
baizhenyu:statspoller-pool-cache

Conversation

@baizhenyu

@baizhenyu Tim Bai (baizhenyu) commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

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-filtered rate() 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-pool label is stamped at creation; pods never move between pools), so caching resolutions per pod UID carries zero correctness risk. resolveWorkerPools:

  • lets fresh resolutions win (harmless either way, given immutability),
  • falls back to cachedPools when the list fails outright or misses one pod,
  • rebuilds the cache restricted to the pods whose ateom directories exist this sweep — pruning departed pods and bounding memory with the same prune-to-the-living pattern lastCPU uses.

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

  • List flap: seed the cache, fail the next list, assert the samples stay on the pooled label set with no pool-less split.
  • Prune: an entry whose ateom directory is gone leaves the cache; the live pod's entry stays.
  • Residual case: a pod first seen during an outage groups without pool labels (one group, not an error).
  • hack/verify-all.sh green.

Part of #896, toward #550.

Note: cache lifetime is tied to ateom directories, which nothing cleans up

cachedPools evicts an entry on the first sweep after its ateom directory disappears — but today nothing removes ateoms/<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:

  • Correctness is unaffected. Pod UIDs are never reused, and a retained entry is only consulted when a sample arrives from that UID — a dead socket produces none.
  • Memory is a rounding error on a pre-existing cost. The cache is bounded by the directory count (~150 bytes per departed pod, ~1.5 MB per ten thousand); each stale directory already costs more than its cache entry does — a dial attempt every sweep plus debug-log noise, on main today, independent of this PR.
  • Eviction is deliberately not tied to the pod list — "the list stopped returning this pod" is exactly the flap signal this fix exists to distrust. The filesystem stays the retention authority, consistent with being the discovery authority.

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.

@baizhenyu Tim Bai (baizhenyu) added kind/bug Something isn't working / bugfixes area/observability bug-bash and removed kind/bug Something isn't working / bugfixes labels Sep 4, 2026
…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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/observability bug-bash kind/bug Something isn't working / bugfixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: one failed pod list re-homes ate.actor.stats.cpu.time increments onto a permanent pool-less series

1 participant