Skip to content

Keep task attempt UUIDs and try numbers aligned - #73554

Merged
ashb merged 10 commits into
apache:mainfrom
astronomer:ti-attempt-number
Sep 26, 2026
Merged

ashb merged 10 commits into
apache:mainfrom
astronomer:ti-attempt-number

Conversation

@ashb

@ashb ashb commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

An attempt has a UUID A and try number N: (A,N). Previously, we assigned
a new UUID B when retrying or clearing, but left the scheduler to
increment the number. That left (B,N) in the database: the replacement's
UUID paired with the previous attempt's number.

We want to always allocate both fields together, archiving (A,N)
before creating (B,N+1). The scheduler then uses that allocation:

Retry lifecycle Previously Now
Running (A,N) (A,N)
UP_FOR_RETRY (B,N) (B,N+1)
Scheduled (B,N+1) (B,N+1)

This has UI impacts, as previously try N+1 was only created just as it starts.

In both of these screenshots below, try 1 has failed and the task is eligible for retry.

UI Before: (There is no try selector visible, as there is only a single try)

main-before

UI after (try 2 is now visible, in state of "up for retry", logs explicitly never fetched, but a "see here" message shown by the UI instead)

pr-after

When clearing a running task, we must also wait for its process to stop
before allowing the replacement to run. We reject heartbeats with 409
when the TI is RESTARTING, which tells the worker to stop. But we cannot
tell from that response when the worker has actually stopped. Its child
may still be handling SIGTERM, running cleanup, or changing external
state. Starting the replacement then could let both attempts act on
the same external system.

We therefore leave both fields unchanged throughout shutdown:

Running clear Identity State
Before clear (A,N) RUNNING
Clear requested (A,N) RESTARTING
Cleared again (A,N) RESTARTING
Termination reported (B,N+1) NULL

By keeping the UUID unchanged as A, we let the worker report against
the attempt it owns. We also reject start requests during RESTARTING
so another workload cannot start that attempt while shutdown continues.

The supervisor knows the child has stopped once it has reaped it.
We need the supervisor to report that fact to the API before we can
release the replacement without relying on an executor event. It sends
SERVER_TERMINATED through the existing finish request to acknowledge
the shutdown. We do not store SERVER_TERMINATED as a TI state: the
worker is confirming that it stopped as instructed, rather than
reporting a task failure.

Once the supervisor confirms termination, we make the replacement
eligible immediately and restore its retry budget. A clear must allow
an immediate rerun even when the task has exhausted its retries,
without introducing an ordinary retry delay or failure callback.

We still need to support workers back to Task SDK 1.0.0, which cannot
send this acknowledgement. The scheduler therefore accepts terminal
executor events as a fallback while the matching attempt is RESTARTING.
Executor SUCCESS only means the workload finished, so either SUCCESS
or FAILED can complete the clear. The API and scheduler both lock and
recheck the row before allocating a replacement. If the API completes
the clear first, the old executor event no longer matches the live
attempt. We do not need to track the worker version.

Moving allocation earlier must not change retry timing or which logs
users see. During UP_FOR_RETRY, we still calculate backoff and select
logs using the failed attempt N, although the live row now identifies
N+1. We preserve both fields across deferral and rescheduling because
they continue the same attempt. Try zero still means we have not
allocated an attempt.

We also need to bring existing pending rows into the new convention.
Migration 0134 advances UP_FOR_RETRY rows and NULL-state rows with
positive try numbers: the old code had already replaced their UUIDs
but left scheduling to advance their numbers. Rows at try zero with
NULL state remain unallocated.

We use Cadwyn to preserve older API contracts. For current clients,
the API returns 410 for an archived attempt and 404 for an unknown UUID;
older clients retain 404 for both. If a worker loses a response, it can
use 410 to establish that its attempt has ended, but cannot infer that
the server accepted the particular outcome it reported.

We retain the existing heartbeat-timeout and orphan recovery behavior.
Those recovery paths cannot prove that the old worker has stopped.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

  • 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.

@boring-cyborg boring-cyborg Bot added area:API Airflow's REST/HTTP API area:db-migrations PRs with DB migration area:scheduler area:task-sdk area:Triggerer area:UI Related to UI/UX. For Frontend Developers. kind:documentation labels Sep 22, 2026
@ashb

ashb commented Sep 22, 2026

Copy link
Copy Markdown
Member Author

This inconsistency was (re)discovered here #65932 (comment)

@ashb
ashb force-pushed the ti-attempt-number branch 2 times, most recently from 0f4050b to a500381 Compare September 25, 2026 09:50
@ashb
ashb marked this pull request as ready for review September 25, 2026 09:52
@ashb ashb removed kind:documentation area:UI Related to UI/UX. For Frontend Developers. area:Triggerer labels Sep 25, 2026
@ashb

ashb commented Sep 25, 2026

Copy link
Copy Markdown
Member Author

Pulled out of review for final test run.

@ashb
ashb requested a review from kaxil September 25, 2026 12:14

@kaxil kaxil left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, a few minor comments inline. The migration one is the only one I am not 100% sure on.

An attempt has a UUID A and try number N: (A,N). Previously, we assigned
a new UUID B when retrying or clearing, but left the scheduler to
increment the number. That left (B,N) in the database: the replacement's
UUID paired with the previous attempt's number.

We want to always allocate both fields together, archiving (A,N)
before creating (B,N+1). The scheduler then uses that allocation:

| Retry lifecycle | Previously | Now     |
|-----------------|------------|---------|
| Running         | (A,N)      | (A,N)   |
| UP_FOR_RETRY    | (B,N)      | (B,N+1) |
| Scheduled      | (B,N+1)    | (B,N+1) |

When clearing a running task, we must also wait for its process to stop
before allowing the replacement to run. We reject heartbeats with 409
when the TI is RESTARTING, which tells the worker to stop. But we cannot
tell from that response when the worker has actually stopped. Its child
may still be handling SIGTERM, running cleanup, or changing external
state. Starting the replacement then could let both attempts act on
the same external system.

We therefore leave both fields unchanged throughout shutdown:

| Running clear        | Identity | State      |
|----------------------|----------|------------|
| Before clear         | (A,N)    | RUNNING    |
| Clear requested      | (A,N)    | RESTARTING |
| Cleared again        | (A,N)    | RESTARTING |
| Termination reported | (B,N+1)  | NULL       |

By keeping the UUID unchanged as A, we let the worker report against
the attempt it owns. We also reject start requests during RESTARTING
so another workload cannot start that attempt while shutdown continues.

The supervisor knows the child has stopped once it has reaped it.
We need the supervisor to report that fact to the API before we can
release the replacement without relying on an executor event. It sends
SERVER_TERMINATED through the existing finish request to acknowledge
the shutdown. We do not store SERVER_TERMINATED as a TI state: the
worker is confirming that it stopped as instructed, rather than
reporting a task failure.

Once the supervisor confirms termination, we make the replacement
eligible immediately and restore its retry budget. A clear must allow
an immediate rerun even when the task has exhausted its retries,
without introducing an ordinary retry delay or failure callback.

We still need to support workers back to Task SDK 1.0.0, which cannot
send this acknowledgement. The scheduler therefore accepts terminal
executor events as a fallback while the matching attempt is RESTARTING.
Executor SUCCESS only means the workload finished, so either SUCCESS
or FAILED can complete the clear. The API and scheduler both lock and
recheck the row before allocating a replacement. If the API completes
the clear first, the old executor event no longer matches the live
attempt. We do not need to track the worker version.

Moving allocation earlier must not change retry timing or which logs
users see. During UP_FOR_RETRY, we still calculate backoff and select
logs using the failed attempt N, although the live row now identifies
N+1. We preserve both fields across deferral and rescheduling because
they continue the same attempt. Try zero still means we have not
allocated an attempt.

We also need to bring existing pending rows into the new convention.
Migration 0134 advances UP_FOR_RETRY rows and NULL-state rows with
positive try numbers: the old code had already replaced their UUIDs
but left scheduling to advance their numbers. Rows at try zero with
NULL state remain unallocated.

We use Cadwyn to preserve older API contracts. For current clients,
the API returns 410 for an archived attempt and 404 for an unknown UUID;
older clients retain 404 for both. If a worker loses a response, it can
use 410 to establish that its attempt has ended, but cannot infer that
the server accepted the particular outcome it reported.

We retain the existing heartbeat-timeout and orphan recovery behavior.
Those recovery paths cannot prove that the old worker has stopped.
Retries now allocate the next try before it starts, so selecting the
last executed try only in Logs makes tab changes misleading. Keep the
current try as the default and preserve explicit selections across tabs.

Explain why a pending try has no logs, don't try to fetch any logs from
the API server, and offer the previous try instead.
…x tests

Some tests were manually creating TIs in "incorrect" state, this fixes them.
New clients will usually ack the 409 to by reporting `server_terminated`
state, but old clients (and crashed tasks) won't, so we need to make
sure that executor events still correctly progress a RESTARTING TI into
a new try in UP_FOR_RETRY/None.
@ashb
ashb force-pushed the ti-attempt-number branch from f486099 to 8f692d6 Compare September 26, 2026 15:08
@ashb
ashb merged commit 05519cd into apache:main Sep 26, 2026
156 checks passed
@ashb
ashb deleted the ti-attempt-number branch September 26, 2026 21:16
namanjain24-sudo added a commit to namanjain24-sudo/airflow that referenced this pull request Sep 28, 2026
…ents

process_executor_events calls ti.set_state() without a session when the
Dag of a finished task instance cannot be loaded. set_state is
@provide_session and settings.Session is scoped, so create_session()
returned the scheduler's own session and committed and closed it on
exit, in the middle of the batch.

A second call site with the same bug (a cleared task instance reported
terminated) was superseded by apache#73554, which replaced that whole code
path with TaskInstance.complete_restart(), already passing the
scheduler's session correctly.
namanjain24-sudo added a commit to namanjain24-sudo/airflow that referenced this pull request Oct 5, 2026
…ents

process_executor_events calls ti.set_state() without a session when the
Dag of a finished task instance cannot be loaded. set_state is
@provide_session and settings.Session is scoped, so create_session()
returned the scheduler's own session and committed and closed it on
exit, in the middle of the batch.

A second call site with the same bug (a cleared task instance reported
terminated) was superseded by apache#73554, which replaced that whole code
path with TaskInstance.complete_restart(), already passing the
scheduler's session correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API area:db-migrations PRs with DB migration area:scheduler area:task-sdk full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants