perf: stream the task manager's pending queue in chunks instead of loading every job - #922
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The orphan reaper still fully instantiates ordinary pending rows before lazy iteration, undermining the performance optimization.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Optimizes task-manager scheduling by streaming pending jobs in projected, paginated chunks and hydrating tasks only when needed.
Changes:
- Adds lazy pending-task pagination with start-limit-aware processing.
- Adds projected loading and deferred-field hydration.
- Adds regression tests for pagination, query counts, and hydration.
File summaries
| File | Summary |
|---|---|
awx/main/tests/functional/task_management/test_scheduler.py |
Tests pagination, query efficiency, and partial-task hydration. |
awx/main/scheduler/task_manager.py |
Implements chunked scheduling, projections, hydration, and scheduling-order changes. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Fix _prior_values_store handling after hydration and avoid per-task lazy queries for InventoryUpdate.inventory_source.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
awx/main/scheduler/task_manager.py:462
- The projection only loads the
InventoryUpdate.inventory_sourceFK, butDependencyGraph.mark_inventory_update()later readsjob.inventory_source.inventory_idfor every waiting/running inventory update. Since the related object is not selected, this path still performs one lazy query per runningInventoryUpdate, defeating the no-lazy-load/query-scaling goal for that task type. Select the needed related row (or otherwise make its inventory ID available without a per-task fetch) and cover running inventory updates in the regression test.
'InventoryUpdate___inventory_source',
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The moderate inventory-update related-object loading issue remains unresolved.
Review details
Suppressed comments (1)
awx/main/scheduler/task_manager.py:462
only('InventoryUpdate___inventory_source')loads the foreign-key id, but it does not populate the relatedInventorySourcerow.DependencyGraph.mark_inventory_update()dereferencesjob.inventory_source.inventory_id, so every active or newly startedInventoryUpdatestill incurs a related-object query; the new no-lazy-load test does not exercise this because its pending updates are blocked before graph insertion. Please either select the needed relatedinventory_idin this queryset or change the graph to use an already-loaded ID, and cover multiple active inventory updates.
'InventoryUpdate___inventory_source',
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
The task manager loaded every pending, waiting and running job as a full polymorphic model object on every 20-second cycle, even though it can start at most
START_TASK_LIMIT(default 100) jobs per cycle. This PR makes the pending queue stream lazily in chunks, loads only the columns the scheduling loop actually reads, and fully hydrates a task right beforepre_start()so the start path never operates on a partial object.This started as a roadmap suggestion to "window the task manager's query to
START_TASK_LIMIT". That literal change isnot what this PR does, and the reason is documented in code: the limit counts jobs that started, not jobs that were examined. Blocked and capacity-starved jobs are skipped without consuming it, so a plainLIMITwould let a run of old blocked jobs hide every newer startable job behind them indefinitely. Reproduced before the change: with 900 older jobs from a non-concurrent template followed by 100 newer startable ones, the current code starts 91 in a cycle and aLIMIT 100variant starts 1.Upstream AWX
develhas the identical unwindowed query, so this is an Ascender-original change.What changed
awx/main/scheduler/task_manager.pyTaskBase.get_tasks_queryset()is split out ofget_tasks()so subclasses can shape the query without changing how it is materialized.DependencyManagerandWorkflowManagerbehaviour is unchanged.TaskManager.iter_pending_tasks()yields pending jobs oldest first using keyset pagination on(created, id)in chunks ofmax(START_TASK_LIMIT, 500)(pending_task_chunk_size, overridable per instance for tests).process_pending_tasks()now checks the start limit and the timeout before pulling the next task, so once the limit is reached no further chunk is loaded. It accepts any iterable and returns the number of tasks examined.TaskManager.TASK_FIELDSis anonly()projection applied to every task the manager loads: theUnifiedJobcolumns the loop reads plus the per-subclass fields the dependency graph needs (Job___project,InventoryUpdate___inventory_source,WorkflowJob___allow_simultaneous, and so on).polymorphic_ctypemust be listed explicitly; without it django-polymorphic lazy-loads that column once per subclass instance.TaskManager.hydrate_task()callsrefresh_from_db(fields=<deferred fields>)on the same instance at the top ofstart_task()and before the dependency-failure save.pre_start()decryptsstart_args, walks credentials and writesjob_explanation, andUnifiedJob.save()readsstarted,finished,elapsed,cancel_flagand more; on a partial instance each of those would be fetched by its own query. Restricting the refresh to deferred fields means thestatus,controller_nodeandexecution_nodealready decided on the instance survive. Cost: one query per started job, none per skipped job.HasEditsMixin.sync_edit_snapshot(attnames)(awx/main/models/base.py) is a new helper thathydrate_task()calls right after the refresh.PrimordialModeltakes its edit-tracking snapshot at instantiation, so an instance loaded withonly()lacks the deferred fields; after hydration every one of them compared as "changed" andPrimordialModel.save()rewrotemodified_bywith the current user, which isNonein the dispatcher. The helper records the current values of exactly the refreshed fields as the baseline, so a change already made to another loaded field is still detected,and it assigns a new dict rather than mutating the old one becauseProject.save()keeps a reference to the previous snapshot to compare against after saving. A globalrefresh_from_dboverride was tried and rejected:ImplicitRoleFielddoes a fullrefresh_from_db()insidepost_saveon every role-bearing model, and Project update triggering, instance-group policy scheduling and organization-transfer role updates all compare the snapshot after that point. Note that starting a job assignsinstance_group, an editable field, so a started job'smodified_bywas already reset before this PR (the dev database has none set across 158 user-launched jobs); the fix matters for the dependency-failure save and for correctness of the snapshot in general.DependencyGraph.mark_inventory_update()(awx/main/scheduler/dependency_graph.py) now keys on the InventoryUpdate's owninventory_id, which is copied from the source at creation, instead of dereferencingjob.inventory_source.inventory_id. That dereference cost one related-object query per active inventory update on every cycle, before this PR as well as after it, because neither the old full load nor the projection selected the related row. The field is nullable so the old dereference remains as a fallback, andInventoryUpdate___inventorywas added toTASK_FIELDS.reap_jobs_from_orphaned_instances()now excludes rows with an emptyexecution_nodein SQL. Ordinary pending jobs have no execution node, and the old query selected every one of them (an empty string is never a registered hostname) only to skip them in Python, materializing the whole pending queue as full polymorphic objects a second time each cycle. Genuinely orphaned rows (a non-empty, unregisteredexecution_node) are still reaped; the Python guard is unchanged._schedule()order is nowafter_lock_init→reap_jobs_from_orphaned_instances→ load waiting/running →process_tasks. Reaping resets orphaned waiting jobs to pending; with a lazy pending load they must be reaped before the load, otherwise the same job would appear both as a stale waiting entry in the graph and as a pending candidate in the samecycle.Behavioural notes
task_manager_pending_processedmetric now reports the number of pending jobs examined in the cycle rather than the total pending count.task_manager_get_tasks_secondsshrinks andtask_manager_process_pending_tasks_secondsgrows correspondingly, because the pending load now happens inside the loop.Tests
Four tests added to
awx/main/tests/functional/task_management/test_scheduler.py:test_pending_queue_is_paginated_not_capped– three older non-concurrent jobs and one newer free job with a chunk size of 2; the free job in the second chunk still starts and pagination is observed to cross the boundary.test_pending_queue_loading_stops_at_start_task_limit– withstart_task_limit = 1and chunk size 1, exactly one waiting/running query and one pending chunk are issued; the rest of the queue is never loaded.test_task_manager_loop_does_not_lazy_load– Job, ProjectUpdate, InventoryUpdate and WorkflowJob tasks are all kept blocked by a running sibling; the cycle's query count is identical with 4 and with 16 pending tasks. This guardsTASK_FIELDS: a new attribute read in the loop that is not in the projection shows up as a per-task query.test_start_task_hydrates_partially_loaded_task– asserts the loop really works on partial objects, that the task reachingstart_taskends up with no deferred fields, that node assignment is preserved, and that untouched columns (extra_vars,job_args) survive the save.Two tests added to
TestJobReaperinawx/main/tests/functional/test_dispatch.py:test_reaper_does_not_load_ordinary_pending_jobs– with 3 pending jobs present, the reaper issues no query againstmain_job(django-polymorphic only issues that per-type follow-up when a pending Job row was actually loaded; the old query triggers it).test_reaper_still_reaps_job_on_unregistered_execution_node– a running job on an unknown execution node is still failed.test_active_inventory_updates_do_not_lazy_load_inventory_sourceintest_scheduler.pyruns a cycle with one and thenfour running inventory updates from distinct sources and asserts the query count does not change (it was 15 versus 18 before the fix).TestSyncEditSnapshotinawx/main/tests/functional/models/test_base.pycovers the helper: refreshed fields are not edits after the sync, they would be without it, a change made before the sync to a loaded field is still an edit, and achange after the sync is an edit. Intest_scheduler.py,test_start_task_hydrates_partially_loaded_taskasserts a no-change save of a hydrated task keepsmodified_by, andtest_dependency_failure_keeps_modified_bycovers the dependency-failure save end to end.Full suite: 4014 passed, 6 skipped (
awx/main/tests/unit,awx/main/tests/functional,awx/conf/tests,awx/sso/tests).Live check: a sliced job template was launched through the autoreloaded dispatcher running this code; the parent workflow job and both slice jobs completed successfully with controller and execution nodes assigned.
Measurements
Old and new implementations were loaded side by side in one process against identical data and run as a full task manager cycle (orphan reaper included), on
tools_awx_1with one hybrid node of capacity 458, job launch stubbed to bookkeeping only, jobs created inside a rolled-back transaction. Timings on this box vary by roughly 30% between runs; treat ratios as approximate.Where the cost actually is: Postgres returns 1,000 of these rows in about 3 ms via the status index. The rest is django-polymorphic instantiation (about 1.4 ms per row for full objects, about 0.5 ms with the projection) plus 0.3–0.9 ms per row of loop work. Before this PR the old reaper query loaded the entire pending queue a second time each cycle, which is why the old column is roughly double a bare load-plus-loop.
Two caveats on the table:
dependent_jobsprefetch) and 50 chunks add up. 500 keeps a free-draining queue at one query while avoiding that overhead.Reviewer notes
TASK_FIELDScomment says where to add a field if the loop grows a new attribute read;test_task_manager_loop_does_not_lazy_loadwill fail if it is forgotten.process_pending_taskswas changed from aforloop to check-then-next(); with aforloop the generator would load one extra chunk after the limit was hit.