Repository navigation
[v3-4-test] Starve dag runs that fail to deserialize during scheduling - #75841
Closed
vatsrahul1001 wants to merge 1 commit into
Closed
vatsrahul1001 wants to merge 1 commit into
vatsrahul1001 wants to merge 1 commit into
Conversation
The deserialization guard backported in #75745 skipped a task instance whose Dag cannot be deserialized but did not remove its dag run from the candidate set. When such a run sorts to the top of a batch limited by max_tis, the selection loop skips it, finds no new starvation filter, and breaks, so lower-priority runnable task instances behind it are never queued that tick. Add the starved_dag_runs dimension (present on main via #75263): mark a dag run as starved when its Dag fails to deserialize, exclude starved runs from the re-query, and count a newly starved run as a new filter so the loop re-queries and makes progress. This matches main's behaviour without pulling in #70307, which is not on v3-4-test.
Contributor
Author
|
Closing in favour of 75802 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the deserialization guard backported in #75745 (manual backport of #75263).
Problem
#75745 backported the crash guard from #75263 but dropped the
starved_dag_runsdimension, because the full #75263 was entangled with #70307 ("Fill scheduler task batches after concurrency rejection"), which is not onv3-4-test.Without
starved_dag_runs, the guard skips a task instance whose Dag fails to deserialize but never removes that dag run from the candidate set. When such a run sorts to the top of a batch limited bymax_tis:_build_schedulable_tis_queryfetches only the highest-priority rows (the bad run's TI)._task_concurrency_allows_executionskips it (deserialization fails →return False), but nothing is starved.found_new_filtersisFalse→ the selection loop breaks.So the backport guarded against the crash but reintroduced the starvation that #75263 exists to prevent. This surfaced as a branch-wide failure of
test_find_executable_task_instances_skips_dag_that_cannot_be_deserialized(shipped by #75745), which fails theCI passedgate on every open PR intov3-4-test.Fix
Port only the self-contained
starved_dag_runsslice from main's #75263 onto the selection machinery already present onv3-4-test:_build_schedulable_tis_query;found_new_filtersso the loop re-queries and makes forward progress.This makes the backport behave identically to
mainwithout pulling in #70307. Thefound_new_filtersrefill loop and the query-exclusion builder it relies on are already onv3-4-test.Testing
The previously-failing test now passes, along with the sibling DB-error-propagation test and the full
test_find_executable_task_instances*/ query-builder / critical-section suite (40 passed) locally on SQLite:Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.8) following the guidelines