Skip to content

[v3-4-test] Starve dag runs that fail to deserialize during scheduling - #75841

Closed
vatsrahul1001 wants to merge 1 commit into
v3-4-testfrom
fix-scheduler-deser-starved-dagruns-v3-4-test
Closed

vatsrahul1001 wants to merge 1 commit into
v3-4-testfrom
fix-scheduler-deser-starved-dagruns-v3-4-test

Conversation

@vatsrahul1001

Copy link
Copy Markdown
Contributor

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_runs dimension, because the full #75263 was entangled with #70307 ("Fill scheduler task batches after concurrency rejection"), which is not on v3-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 by max_tis:

  1. _build_schedulable_tis_query fetches only the highest-priority rows (the bad run's TI).
  2. _task_concurrency_allows_execution skips it (deserialization fails → return False), but nothing is starved.
  3. found_new_filters is False → the selection loop breaks.
  4. Lower-priority runnable TIs behind the bad run are never queued that tick — and the same thing repeats every tick.

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 the CI passed gate on every open PR into v3-4-test.

Fix

Port only the self-contained starved_dag_runs slice from main's #75263 onto the selection machinery already present on v3-4-test:

  • mark a dag run as starved when its Dag fails to deserialize (and short-circuit an already-starved run);
  • exclude starved runs from the re-query in _build_schedulable_tis_query;
  • count a newly starved run in found_new_filters so the loop re-queries and makes forward progress.

This makes the backport behave identically to main without pulling in #70307. The found_new_filters refill loop and the query-exclusion builder it relies on are already on v3-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:

test_find_executable_task_instances_skips_dag_that_cannot_be_deserialized PASSED
test_find_executable_task_instances_reraises_db_errors_from_dag_load PASSED

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.8)

Generated-by: Claude Code (Opus 4.8) following the guidelines

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.
@vatsrahul1001

Copy link
Copy Markdown
Contributor Author

Closing in favour of 75802

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant