Skip to content

Improve LocalExecutor bookkeeping: correctly add tasks to the running list - #74142

Merged
eladkal merged 3 commits into
mainfrom
local-executor-bookkeeping
Oct 5, 2026
Merged

eladkal merged 3 commits into
mainfrom
local-executor-bookkeeping

Conversation

@ashb

@ashb ashb commented Oct 3, 2026

Copy link
Copy Markdown
Member

Since #73916 landed, BaseExecutor now keeps a UUID-to-coordinates map for each
attempt and drops an entry once the key is no longer queued, running or in the
event buffer. The issue was that LocalExecutor never added dispatched work to
running, so the entry was dropped while the task was still executing. The
scheduler then received the final event with no coordinates ("Received executor
event with state success for task instance (coordinates=None)").

This PR fixes that, and addresses a few gotchas that could, in edge cases
around callbacks or post-task finalization, lead to a forever-dead/locked
worker slot.

Holding the key in running until the worker finishes needs LocalExecutor
to know when that is, and workers share one activity queue so the parent
cannot tell which worker took which workload. Results now carry the
worker's pid and START tells the parent which pid owns the key. That lets
us:

  • release the slot only on a terminal result from the owning worker,
    ignoring results from unknown pids or pids that don't own the key
  • fail the workload when its worker dies, instead of leaving it in
    running until the scheduler's heartbeat timeout

A resumed task reuses its key, so a per-key dispatch count stops the old
run's terminal result from releasing the new run's slot.


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

Stack created with GitHub Stacks CLI • Give Feedback 💬

@ashb
ashb requested review from XD-DENG and o-nikolas as code owners October 3, 2026 09:12
@ashb
ashb added this pull request to stack #73919 October 3, 2026 09:12
@boring-cyborg boring-cyborg Bot added the area:Executors-core LocalExecutor & SequentialExecutor label Oct 3, 2026
Base automatically changed from edge-exeuctor-keyed-by-uuid to main October 3, 2026 09:13
@ashb
ashb force-pushed the local-executor-bookkeeping branch from 59eb578 to 67a2a7c Compare October 3, 2026 09:13
@ashb

ashb commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Addressing the issue raised by Shahar in #73916 (comment)

@ashb ashb added this to the Airflow 3.4.0 milestone Oct 3, 2026
@ashb
ashb force-pushed the local-executor-bookkeeping branch from de6cbb5 to 2b2b21b Compare October 3, 2026 11:40
ashb added 3 commits October 3, 2026 18:22
…g` list

Since #73916 landed, BaseExecutor now keeps a UUID-to-coordinates map for each
attempt and drops an entry once the key is no longer queued, running or in the
event buffer. The issue was that LocalExecutor never added dispatched work to
`running`, so the entry was dropped while the task was still executing. The
scheduler then received the final event with no coordinates ("Received executor
event with state success for task instance <uuid> (coordinates=None)").

This PR fixes that, and addresses a few gotchas that could, in edge cases
around callbacks or post-task finalization, lead to a forever-dead/locked
worker slot.

Holding the key in `running` until the worker finishes needs LocalExecutor
to know when that is, and workers share one activity queue so the parent
cannot tell which worker took which workload. Results now carry the
worker's pid and START tells the parent which pid owns the key. That lets
it:

- release the slot only on a terminal result from the owning worker,
ignoring results from unknown pids or pids that don't own the key
- fail the workload when its worker dies, instead of leaving it in
`running` until the scheduler's heartbeat timeout

A resumed task reuses its key, so a per-key dispatch count stops the old
run's terminal result from releasing the new run's slot.
@ashb
ashb force-pushed the local-executor-bookkeeping branch from 1c1b063 to 302c914 Compare October 3, 2026 18:33

@fat-catTW fat-catTW left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@eladkal
eladkal merged commit 4083290 into main Oct 5, 2026
79 checks passed
@eladkal
eladkal deleted the local-executor-bookkeeping branch October 5, 2026 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Executors-core LocalExecutor & SequentialExecutor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants