Conversation
The supervisor uses _terminal_state inconsistently: failed and skipped outcomes are recorded when the worker reports them, while other outcomes are recorded only after the API call succeeds. This couples the known task outcome to API availability and makes deferred reporting harder to reason about. This changes things so we record the worker鈥檚 outcome immediately on an instance field and use the existing pending message to track delivery. Failed API calls retain that message and keep heartbeats running. Upon task process exit pending reports take precedence over exit-code inference. Server-directed termination (zombie detection, external clearing, etc.) overrides pending reporting and any worker outcomes received during shutdown. The previous tests required _terminal_state to remain unset after an API failure because setting it stopped heartbeats. That assertion reflected the old coupling between outcome and delivery. The updated tests check the underlying guarantees: retain the report, continue heartbeats, and recover through the correct endpoint. Post-task finalization timeouts now depend on the recorded completion timestamp, independently of whether reporting succeeds. This also bounds finalization when the initial API call fails.
Reporting a retry to the API can replace the task instance UUID before on_retry_callback has run. The callback and other finalizers still use the old UUID, so their API calls can fail even though they are part of finishing that attempt. This changes things so we hold the RetryTask message until the worker exits. Heartbeats continue while finalization runs, and the existing overtime limit still applies. Server-directed termination discards the pending report. The report keeps the original end date, retry delay and reason. We don't add the retry delay again after finalization, but if that delay expires while callbacks are running, the next attempt now waits for them. Other outcomes are reported at the same point as before. In particular, success still reaches the server before callbacks run, so this doesn't delay downstream scheduling on success. | Outcome | Before | After | | --- | --- | --- | | Success | Before finalization | Unchanged | | Skipped | After worker exit | Unchanged | | Failed | After worker exit | Unchanged | | Retry | Before finalization | After worker exit | | Deferred / rescheduled / awaiting input | Immediately | Unchanged | The tests check that retry callbacks can still use the original attempt, including through dag.test(), and that callback errors don't prevent the retry report. They also cover heartbeats, the finalization timeout, and external state changes while the report is pending.
ashb
marked this pull request as ready for review
September 16, 2026 19:03
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.
Delay retry reporting until task finalization completes
Reporting a retry to the API can replace the task instance UUID before
on_retry_callback has run. The callback and other finalizers still use
the old UUID, so their API calls can fail even though they are part offinishing that attempt.
This changes things so we hold the RetryTask message until the worker
exits. Heartbeats continue while finalization runs, and the existing
overtime limit still applies. Server-directed termination discards the
pending report.
The report keeps the original end date, retry delay and reason. We don't
add the retry delay again after finalization, but if that delay expires
while callbacks are running, the next attempt now waits for them.
Other outcomes are reported at the same point as before. In particular,
success still reaches the server before callbacks run, so this doesn't
delay downstream scheduling on success.
The tests check that retry callbacks can still use the original attempt,
including through dag.test(), and that callback errors don't prevent the
retry report. They also cover heartbeats, the finalization timeout, and
external state changes while the report is pending.
Built on top of #73249