Skip to content

Stop overlapping deferred TI writers on MySQL - #72134

Open
Vamsi-klu wants to merge 4 commits into
apache:mainfrom
Vamsi-klu:fix/65818-deferred-ti-lock-ownership
Open

Stop overlapping deferred TI writers on MySQL#72134
Vamsi-klu wants to merge 4 commits into
apache:mainfrom
Vamsi-klu:fix/65818-deferred-ti-lock-ownership

Conversation

@Vamsi-klu

@Vamsi-klu Vamsi-klu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What is the change?

While a task instance is DEFERRED, only the owning triggerer writes its trigger_id and next_* columns. The scheduler's check_trigger_timeouts becomes a bounded fallback, PK-ordered with LIMIT 100 and FOR UPDATE SKIP LOCKED, that only fires for unassigned triggers or dead TriggererJob heartbeats. Trigger.clean_unused no longer bulk-updates task_instance at all.

Why did I do it?

related: #65818
related: #71391
related: #72062
related: #70961

The scheduler timeout sweep and the triggerer cleanup both issued unbounded UPDATE task_instance statements. Their predicates are disjoint, but InnoDB scans them on different indexes (ti_state vs ti_trigger_id) and deadlocks. #71391 added retries on submit_event; this PR removes the lock overlap itself.

How did I do it?

Every DEFERRED-exit path NULLs trigger_id in the same transaction: clear_task_instances, TaskInstance.set_state, DagRun timeout, mapped REMOVED and skipped extras, and the Execution API skip-downstream route. submit_event and submit_failure select with SKIP LOCKED and no-op if another writer holds the row. The MySQL two-step DELETE in clean_unused re-checks references so a deferral between SELECT and DELETE cannot cascade-delete the TI (#72062). Timeout SET values are unchanged.

What's the impact?

MySQL deadlocks between the two sweeps no longer take down the triggerer, and a deferred task whose triggerer is alive is never timed out by the scheduler.

What's the test plan?

New unit tests in test_scheduler_job.py (healthy triggerer not flipped, dead triggerer fallback, bounded PK order, MySQL-only two-session SKIP LOCKED), test_trigger.py (no UPDATE task_instance, DELETE re-check), test_cleartasks.py, test_mappedoperator.py, and the Execution API skip-downstream test. Each fails on revert.


Was generative AI tooling used to co-author this PR?
  • Yes

Generated-by: Cursor Grok 4.6 following the guidelines

Scheduler timeout and triggerer clean_unused scanned task_instance
on different indexes and deadlocked. While deferred, only the owning
triggerer writes trigger_id. clean_unused no longer bulk-NULLs TIs.
Timeout is a PK-ordered SKIP LOCKED fallback for dead or unassigned
triggerers. DEFERRED-exit paths including clear_task_instances NULL
trigger_id in the same transaction.

related: apache#65818
@Vamsi-klu
Vamsi-klu requested review from XD-DENG and ashb as code owners August 27, 2026 07:17
@boring-cyborg boring-cyborg Bot added area:Scheduler including HA (high availability) scheduler area:Triggerer labels Aug 27, 2026
skip-downstream is a DEFERRED-exit that does not go through set_state.
Leaving trigger_id set leaks Triggers now that clean_unused no longer
bulk-unlinks. Name the timeout fallback batch size so ORDER BY id
LIMIT N fails on revert.
@Vamsi-klu

Copy link
Copy Markdown
Contributor Author

Follow-up commit 2c33a94f8e lands the remaining DEFERRED-exit that the first commit dropped: Execution API ti_skip_downstream now NULLs trigger_id (this is a server-side UPDATE, not a Task SDK payload). Also names _TRIGGER_TIMEOUT_BATCH_SIZE so the PK-ordered LIMIT fails on revert.

generate-tasksdk-datamodels was a false fail here: uv run dirties uv.lock; the generator itself produced no datamodel diff.


Drafted-by: Cursor Grok 4.6

TaskMap.expand_mapped_task writes REMOVED, SKIPPED, and UPSTREAM_FAILED
without going through set_state. A deferred extra that keeps trigger_id
pins the triggerer after clean_unused stopped bulk-unlinking.
@Vamsi-klu

Copy link
Copy Markdown
Contributor Author

Follow-up d206baa9ea: TaskMap.expand_mapped_task also NULLs trigger_id when extras go REMOVED (and on zero-length SKIPPED / unresolved UPSTREAM_FAILED). That path does not go through set_state or DagRun's already-fixed bulk REMOVED write. A deferred extra that kept the FK would pin the triggerer after clean_unused stopped bulk-unlinking.


Drafted-by: Cursor Grok 4.6

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

Labels

area:Scheduler including HA (high availability) scheduler area:Triggerer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant