Repository navigation
[Bug] Cron session-artifact registry only grows; every list() reads all registered partitions
#3864
rain-lee1999
started this conversation in
Bug reports
Replies: 1 comment
|
Confirmed on current main (74591cd): I have a patch underway along the TS lines, since 0.9.8 cached parsed state per file keyed by stat and the re-read looks like a port regression:
Orthogonal to the passivation-gate work in #3840/#3847 — this is about what the jobs probe costs, not when it fires. Branch link to follow on my fork. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Summary
AgentCronJobStorein session-artifact mode keeps a map of session id ->scheduled-jobsfile. Entries are added from several paths (session bind, store bind, passive catalog, deleted-session cancel). No path removes them.list()reads and parses every registered file on each call. In a long-lived worker with many RLM descendants, the map grows with every session the worker has ever touched, so everylist()costs O(sessions ever registered).The same design caused a real stall on the 0.9.8 TypeScript daemon (numbers below). On current Rust main (e375911) I found the same register-only map and read-all
list(), and a unit benchmark showslist()cost grows linearly with registered partitions (see Measured on Rust main). I have not reproduced the stall end to end on Rust.Observed on 0.9.8 (TS daemon, for context)
One long session, about 1,460 RLM descendants over about 21 h on one worker:
attach,cron_listandheartbeats_listtimed out.agent_message.sendtook minutes.In TS the store cached parsed state per file, keyed by stat. So each
list(), job read or scheduler tick cost one syncstatSyncper registered artifact, not a re-read. Every passivated descendant stayed registered. The idle sweep calledlist()once per resident session, which gives O(R x (R + P)) stats per sweep (R = resident, P = passive). Offline, 470 x 1,100 = 517k stats took about 2.05 s per sweep, before swap pressure. Roster flushes and scheduler ticks added O(R + P) each.A local patch on 0.9.8 removed the growth with three changes: do not register a passive descendant that has no jobs file, unregister after passivation when the file is absent, and compute
list()once per sweep.Root cause on current main
register_session_artifact. No unregister exists inpa-core/src/cron/store.list->read_statesrunsread_jobs_state(read + serde parse) for every registered file.read_jobs_staterunsread_to_stringon every call. A missing file costs one failed open; an existing file costs a full read and parse.bind_session,bind_store_artifact, deleted-session cancel.jobs_store.list()to check one session (worker.rs). It runs on every park of a child worker (worker/turn.rs:123->maybe_release_settled_child_kernel), andcan_passivate_workerruns it on each idle-eviction timer fire (worker/turn.rs:256). I found no loop that callslist()once per session. The scheduler tick also reads all files twice viamutate_states(state.rs:51,66).Measured on Rust main (unit benchmark)
An integration test in
crates/pa-core/testsbuilds oneAgentCronJobStore::for_session_artifacts()store, registers N partitions withregister_session_artifact(), and timeslist()(median of 7 calls). Case "missing": no partition file exists. Case "present": every file holds{"jobs":[],"dispatches":[]}. The gate column runs N calls oflist()plus a filter for one session id, which simulates one release-gate check per child on a shared store. Release build, Apple M3 (arm64, macOS), rustc 1.94.0, warm page cache.The jobs lists are empty, so this is a lower bound: real job entries add parse cost.
Reproduction (end to end, not run)
AgentCronJobStore::list()and the settled-child release gate as N grows.Expected behavior
Suggested direction
Any of these would remove the growth:
unregister_session_artifactand call it after passivation, release or delete when the session has no jobs file (this is what fixed the TS case).scheduled-jobsfile exists.list()once per sweep or batch instead of once per session, or add a per-session lookup (jobs_for_session) for the release gate.Question for maintainers
Is the long-lived single-worker store expected to accumulate every descendant session? If yes, would a per-session lookup for the release gate be acceptable?
All reactions