Conversation
The workload loop skipped every failure, so lower-priority sessions kept taking the resources a blocked higher-priority session was waiting for. Stop the pass on resource exhaustion only, report the sessions behind as skips rather than failures, and put the whole waiting queue in the pending queue in sequencing order. Skips are recorded and counted on their own history record, and the give-up classification ignores them, so a session queued behind a blocked one is never deprioritized for work it never got to do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates Sokovan scheduling to stop after resource exhaustion while allowing session-specific failures to continue through the queue.
Changes:
- Reports unattempted sessions as skips and preserves pending-queue order.
- Separates skip history from scheduling attempts.
- Consolidates retry history into
LastPhasewith expanded tests.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
changes/13707.fix.md |
Adds the fix changelog. |
src/ai/backend/manager/models/scheduling_history/row.py |
Separates skip and attempt history records. |
src/ai/backend/manager/sokovan/scheduler/coordinator.py |
Uses LastPhase for failure classification. |
src/ai/backend/manager/sokovan/scheduler/handlers/lifecycle/schedule_sessions.py |
Maps provisioner skips to skipped transitions. |
src/ai/backend/manager/sokovan/scheduler/provisioner/provisioner.py |
Stops scheduling after resource exhaustion. |
src/ai/backend/manager/sokovan/scheduler/results.py |
Adds scheduling-skip results. |
src/ai/backend/manager/views/sokovan/lifecycle.py |
Introduces LastPhase. |
tests/unit/manager/models/test_scheduling_history_row.py |
Tests history merge rules. |
tests/unit/manager/repositories/scheduler/test_update_with_history.py |
Tests separate skip history. |
tests/unit/manager/sokovan/scheduler/handlers/conftest.py |
Updates session fixtures. |
tests/unit/manager/sokovan/scheduler/handlers/test_lifecycle_handlers.py |
Tests skip propagation. |
tests/unit/manager/sokovan/scheduler/provisioner/conftest.py |
Expands scheduling fixtures. |
tests/unit/manager/sokovan/scheduler/provisioner/test_provisioner.py |
Tests blocking and non-blocking failures. |
tests/unit/manager/sokovan/scheduler/test_coordinator.py |
Tests phase history and classification. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| self.phase == new_row.phase | ||
| and self.error_code == new_row.error_code | ||
| and self.to_status == new_row.to_status | ||
| and self.records_an_attempt() == new_row.records_an_attempt() |
There was a problem hiding this comment.
Wrong response. Since I get it wrong every time I say it, I kind of just want to leave it out.
fregataa
left a comment
There was a problem hiding this comment.
get_last_session_histories() returns only the latest history record of a session and it seems possible to get only a attempt record, ignoring skip records
| async def test_update_with_history_no_merge_skipped_after_failure( | ||
| self, | ||
| db_with_cleanup: ExtendedAsyncSAEngine, | ||
| test_session_id: SessionId, | ||
| ) -> None: | ||
| """Skips are counted, but on their own record. | ||
|
|
||
| Skips must be visible and countable, while ``attempts`` on the | ||
| attempt record stays the number the give-up (deprioritization) | ||
| classification is allowed to see. | ||
| """ |
There was a problem hiding this comment.
How about adding a test to check a session with "attempt -> skip -> attempt"?
|
The backport to CI on it fails until the markers are resolved; check out its branch, fix, push, and merge it by hand (auto-merge is not enabled for conflicted backports). |
Summary
SessionProvisioner.schedule_resource_groupskipped every failure, so lower-priority sessions kept taking the resources a blocked higher-priority session was waiting for. The pass now stops on resource exhaustion only — an architecture mismatch, an unsatisfied dependency, an exceeded quota or a group with no agents is specific to the session that hit it and must not stall the queue.session_pending_queuenow returns the whole waiting queue in sequencing order rather than only the failed sessions.LastPhasevalue instead of three loose fields.Test plan
session_pending_queuereturns the blocked session and every skipped session in sequencing orderneed_retryrather thangive_upResolves BA-7328
🤖 Generated with Claude Code