Add durable execution to RedshiftDataOperator#69530
Merged
Merged
Conversation
amoghrajesh
requested review from
Lee-W,
eladkal,
jason810496,
kaxil,
vikramkoka and
vincbeck
July 7, 2026 07:14
Contributor
Author
|
Added a |
Contributor
Author
|
@o-nikolas I was able to test it, removed |
amoghrajesh
marked this pull request as ready for review
July 8, 2026 12:18
vincbeck
reviewed
Jul 8, 2026
Contributor
Author
|
@vincbeck / @o-nikolas this one's ready now, will love to get some reviews here if you have some time. |
vincbeck
approved these changes
Jul 10, 2026
Contributor
Author
joshuabvarghese
pushed a commit
to joshuabvarghese/airflow
that referenced
this pull request
Jul 16, 2026
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.
Was generative AI tooling used to co-author this PR?
What
RedshiftDataOperatorsubmits a statement to the Redshift Data API and polls it to completion on the worker. On a worker crash or preemption mid-poll, Airflow retries the task by callingexecute()again which resubmits the SQL from scratch, since nothing about the in-flight statement id is persisted across attempts.Current behaviour
A retry after a crash always resubmits the full SQL, even if the original statement is still running (or already finished) in Redshift.
INSERT,COPY,UPDATE,CREATE TABLE) this risks duplicate writes;This also applies, in a narrower window, to
wait_for_completion=False: even though that mode does not poll at all, a retry after a successful submission still resubmits today, since the id is never persisted regardless of whether the task waits.Proposed change
Adds
ResumableJobMixinsupport (Airflow 3.3+) toRedshiftDataOperator, following the same pattern already done for the Databricks operators andSnowflakeSqlApiOperator. Before polling begins, the submitted statement id is persisted totask_state_store. On retry, the operator reads it back and:ClientError)durable=Trueis the default; setdurable=Falseto keep the old "always submit fresh on retry" behavior.deferrable=Truetakes precedence overdurable, the Triggerer already tracks the statement across the wait in that mode.Critically, the persistence-on-submit design means the
wait_for_completion=Falsecase is protected too, not just the blocking-wait case & the id is written totask_state_storeimmediately after submission regardless of whether the task polls for it.Changes of Note
submit_jobnow always callshook.execute_query(..., wait_for_completion=False, ...)and hence the mixin, not the hook, owns polling. The statement id is set on the operator immediately after submission (not after the wait completes), which also fixes a pre-existingon_killgap: today, killing the task mid-wait doesn't cancel the statement becauseself.statement_idisn't set yet.get_job_statuscallsdescribe_statementdirectly rather than reusingcheck_query_is_finished/parse_statement_response, since those raise on failure states rather than returning a status string, which would break the mixin's status-in/decision-out contract.is_job_activeuses a negative test (not in (FINISHED, *FAILURE_STATES, NOT_FOUND)) rather than a positive running-states allowlist, deliberately avoiding a real pitfall found inRedshiftDataTrigger.is_still_running, which uses a positive allowlist and would misclassify any future unlisted Redshift status as "done."get_sql_results(the actual return value fetch) was moved intopoll_until_completerather than left solely inget_job_result, because the mixin calls onlypoll_until_completeon reconnect -get_job_resultis skipped entirely on that path. A first pass missed returning the fetched value frompoll_until_completeitself, which silently turnedexecute()'s return value intoNoneon reconnect; caught via the durable test suite and fixed.BLOCKED/WAITING_FOR_RETRYadded to a state allowlist first) - Redshift'sRUNNING_STATES/FINISHED_STATE/FAILURE_STATESare already complete against the real AWS API'sStatusenum.User implications / backcompat
No breaking change.
durabledefaults toTrueon Airflow 3.3+; on earlier versions it's a no-op stub and the operator always submits fresh, exactly as before. Iftask_state_storeisn't available at runtime, the operator logs that crash recovery is disabled and falls back to the same fresh-submit behavior.Testing
aws_defaultconnectiontickitdata loaded and am trying to run some queries using this DAG:Before my changes
Try 1:
Worker was killed above, once worker comes back up, and a new redshift query gets submitted:
Waste / Duplicate submission.
After my changes
Try 1:
But now its saved to task state store:
Since I am using a custom backend, this is the data:
[Breeze:3.10.20] root@7ce59ff19824:/opt/airflow$ cat /tmp/airflow_state/ti_query_tickit/redshift_statement_id.json "dd5d4af5-c05d-401f-9958-2d0622a8764a"[Breeze:3.10.20] root@7ce59ff19824:/opt/airflowTry 2 when worker comes up:
{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.