fix(backend/executor): stop unpickling paused/fired schedules on every get_execution_schedules read - #14439
Conversation
…y read get_execution_schedules() (and get_graph_execution_schedules(), the RPC Sentry flagged as a slow, unbounded query) backed every non-lifecycle read with Scheduler._get_jobs_cached(), which calls APScheduler's stock SQLAlchemyJobStore.get_all_jobs() -- a SELECT with no WHERE clause that scans and unpickles every row in apscheduler_jobs, including paused schedules and already-fired one-shot jobs that nothing ever deletes and that the caller immediately throws away in Python. Add a second cache, _get_active_jobs_cached(), that pushes a next_run_time IS NOT NULL filter down to SQL via the jobstore's existing next_run_time btree index (already present on the stock APScheduler table -- no migration needed). Every caller except the two include_paused=True pause/resume lifecycle lookups now reads through it.
Follow-up to the previous commit: the comment above the original _get_jobs_cached still described it as the primary cache; note that _get_active_jobs_cached is now what everything but the two include_paused=True lifecycle lookups actually calls.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (9)
🧰 Additional context used📓 Path-based instructions (1)Format Python code with `poetry run format`📄 CodeRabbit inference engine (AGENTS.md) Files:
🧠 Learnings (2)📚 Learning: 2026-03-05T15:42:08.207ZApplied to files:
📚 Learning: 2026-08-13T22:09:30.099ZApplied to files:
WalkthroughThe scheduler now uses a dedicated active-job cache with a SQL-level ChangesSchedule cache filtering
Priority: ⬇️ Low — Impact reflects low issue severity. Estimated code review effort: 3 (Moderate) | ~20 minutes Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to Routine schedule reads now exclude paused schedules at the database layer while explicit paused-schedule lifecycle operations remain supported. Filtering, resume behavior, cache invalidation, and gauges are covered, with no remaining merge-readiness risk identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #14439 +/- ##
==========================================
- Coverage 81.34% 81.34% -0.01%
==========================================
Files 3515 3515
Lines 263403 263445 +42
Branches 24413 24416 +3
==========================================
+ Hits 214278 214305 +27
+ Misses 43780 43721 -59
- Partials 5345 5419 +74
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Why / What / How
Why: Sentry flagged
/get_graph_execution_schedulesfor a slow, unbounded DB query (AUTOGPT-SERVER-9NB).Scheduler.get_execution_schedules()— which backs both that RPC and the/schedulesREST routes — reads through a 5s process-wide cache. On a cache miss it called APScheduler's stockSQLAlchemyJobStore.get_all_jobs(), which runsSELECT id, job_state FROM apscheduler_jobs ORDER BY next_run_timewith no WHERE clause — scanning and unpickling every row in the table. That includes paused schedules and already-fired one-shot jobs (APScheduler marks both withnext_run_time = NULL), neither of which anything ever deletes, and both of which the caller immediately discards in Python for every caller except the two pause/resume lifecycle lookups. As that dead-row backlog grows, the query only gets slower.What: Added a second cache,
_get_active_jobs_cached, that pushes anext_run_time IS NOT NULLfilter down to SQL via the jobstore's existingnext_run_timebtree index (already present on APScheduler's stock table definition — no schema change or migration needed).get_execution_schedulesnow reads through it by default; the twoinclude_paused=Truelifecycle callers (pause/resume) keep reading the original unfiltered cache, since they need to find paused rows too.How: Kept the
SQLAlchemyJobStoreinstance backing theEXECUTIONjobstore as a namedself._execution_jobstorereference (instead of only living inline in thejobstores=dict) so the new cache method can query its table directly with a server-side filter, reusing the same engine/connection pool the scheduler already uses (no extra DB connections). TheSCHEDULER_JOBSgauge (labeledstatus="scheduled") now updates from the active-only count, which is the more accurate reading for that label.Changes 🏗️
backend/executor/scheduler.py: new_get_active_jobs_cached()method with SQL-levelnext_run_time IS NOT NULLfiltering;get_execution_schedules()now dispatches to it unlessinclude_paused=True.backend/executor/scheduler_test.py: new integration testtest_paused_schedule_excluded_unless_include_paused(add → list → pause → list excludes it → list withinclude_paused=Truefinds it → resume → list finds it again), run against a real Postgres-backedapscheduler_jobstable.backend/executor/scheduler_unit_test.py,backend/api/features/orgs/regression_test.py: updated two existing mocks that only stubbed the old unfiltered_get_jobs_cachedpath to also stub the new active-jobs path.Agents and large language models used
Claude Code worker on tester VM, Sonnet/Opus 5.
Checklist 📋
For code changes:
poetry run test backend/executor/scheduler_test.py— 56 passed (real Postgres + real APScheduler jobstore, including the new pause/resume regression test)poetry run teston the 8 other files callingget_execution_schedules/get_graph_execution_schedules(scheduler_unit_test.py,v1_test.py,experts/scheduling_test.py,home/service_test.py,experts/experts_db_test.py,copilot/tools/session_context_test.py,copilot/tools/manage_schedules_test.py,orgs/regression_test.py) — 498 passed, 12 xfailedpoetry run ruff check/isort --check/black --check/pyrighton all touched files — cleanFor configuration changes:
.env.defaultis updated or already compatible with my changesdocker-compose.ymlis updated or already compatible with my changes