Fix retry policy overrides not persisted to task instance history - #69235
Conversation
AIP-105's per-try audit trail (retry_delay_override, retry_reason) is meant to live durably in task_instance_history — the columns on the live task_instance row are transient and cleared when the task next enters RUNNING. The Execution API retry handler wrote the overrides only into the live-row UPDATE and archived the task instance to history before those values were applied, so the history columns were always NULL. That silently dropped the audit trail the feature advertises, while retry timing (which reads the live row) stayed correct. Setting the overrides on the task instance before prepare_db_for_next_try() lets record_ti() snapshot them into task_instance_history, restoring the per-try record.
|
Lgtm and can be merged. Although unsure if this in TIH would be useful at all for anything. Maybe auditing 🤷♂️ |
Yeah auditing is what was mentioned in docs |
|
Hi maintainer, this PR was merged without a milestone set.
|
Backport successfully created: v3-3-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
… history (apache#69235) AIP-105's per-try audit trail (retry_delay_override, retry_reason) is meant to live durably in task_instance_history — the columns on the live task_instance row are transient and cleared when the task next enters RUNNING. The Execution API retry handler wrote the overrides only into the live-row UPDATE and archived the task instance to history before those values were applied, so the history columns were always NULL. That silently dropped the audit trail the feature advertises, while retry timing (which reads the live row) stayed correct. Setting the overrides on the task instance before prepare_db_for_next_try() lets record_ti() snapshot them into task_instance_history, restoring the per-try record. (cherry picked from commit 3bfa5ff) Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
Follow-up to apache#69235. The task_instance_history row for a retried try stamped archive-time utcnow() as end_date instead of the end date the worker reported, and missed the final rendered map index when the mid-run update was suppressed (e.g. template errors during failure handling). Snapshot both onto the TI before archiving, and let record_ti() respect a pre-set end_date so the audit trail reflects when the try actually ended.
… history (apache#69235) AIP-105's per-try audit trail (retry_delay_override, retry_reason) is meant to live durably in task_instance_history — the columns on the live task_instance row are transient and cleared when the task next enters RUNNING. The Execution API retry handler wrote the overrides only into the live-row UPDATE and archived the task instance to history before those values were applied, so the history columns were always NULL. That silently dropped the audit trail the feature advertises, while retry timing (which reads the live row) stayed correct. Setting the overrides on the task instance before prepare_db_for_next_try() lets record_ti() snapshot them into task_instance_history, restoring the per-try record. (cherry picked from commit 3bfa5ff) Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
… history (#69235) (#69241) AIP-105's per-try audit trail (retry_delay_override, retry_reason) is meant to live durably in task_instance_history — the columns on the live task_instance row are transient and cleared when the task next enters RUNNING. The Execution API retry handler wrote the overrides only into the live-row UPDATE and archived the task instance to history before those values were applied, so the history columns were always NULL. That silently dropped the audit trail the feature advertises, while retry timing (which reads the live row) stayed correct. Setting the overrides on the task instance before prepare_db_for_next_try() lets record_ti() snapshot them into task_instance_history, restoring the per-try record. (cherry picked from commit 3bfa5ff) Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
… history (#69235) (#69241) AIP-105's per-try audit trail (retry_delay_override, retry_reason) is meant to live durably in task_instance_history — the columns on the live task_instance row are transient and cleared when the task next enters RUNNING. The Execution API retry handler wrote the overrides only into the live-row UPDATE and archived the task instance to history before those values were applied, so the history columns were always NULL. That silently dropped the audit trail the feature advertises, while retry timing (which reads the live row) stayed correct. Setting the overrides on the task instance before prepare_db_for_next_try() lets record_ti() snapshot them into task_instance_history, restoring the per-try record. (cherry picked from commit 3bfa5ff) Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
…#69248) * Archive worker-reported end date and rendered map index on task retry Follow-up to #69235. The task_instance_history row for a retried try stamped archive-time utcnow() as end_date instead of the end date the worker reported, and missed the final rendered map index when the mid-run update was suppressed (e.g. template errors during failure handling). Snapshot both onto the TI before archiving, and let record_ti() respect a pre-set end_date so the audit trail reflects when the try actually ended. * Add test for record_ti fallback end_date stamping Cover the conditional branch where record_ti() archives a non-finished TI with end_date=None, verifying it gets stamped with utcnow() and duration is computed correctly. * Clarify record_ti comment covers pre-set duration too * Snapshot rendered_map_index in TIH when a retry explicitly clears it
…n task retry (apache#69248) * Archive worker-reported end date and rendered map index on task retry Follow-up to apache#69235. The task_instance_history row for a retried try stamped archive-time utcnow() as end_date instead of the end date the worker reported, and missed the final rendered map index when the mid-run update was suppressed (e.g. template errors during failure handling). Snapshot both onto the TI before archiving, and let record_ti() respect a pre-set end_date so the audit trail reflects when the try actually ended. * Add test for record_ti fallback end_date stamping Cover the conditional branch where record_ti() archives a non-finished TI with end_date=None, verifying it gets stamped with utcnow() and duration is computed correctly. * Clarify record_ti comment covers pre-set duration too * Snapshot rendered_map_index in TIH when a retry explicitly clears it (cherry picked from commit 66b803d) Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com>
…n task retry (apache#69248) * Archive worker-reported end date and rendered map index on task retry Follow-up to apache#69235. The task_instance_history row for a retried try stamped archive-time utcnow() as end_date instead of the end date the worker reported, and missed the final rendered map index when the mid-run update was suppressed (e.g. template errors during failure handling). Snapshot both onto the TI before archiving, and let record_ti() respect a pre-set end_date so the audit trail reflects when the try actually ended. * Add test for record_ti fallback end_date stamping Cover the conditional branch where record_ti() archives a non-finished TI with end_date=None, verifying it gets stamped with utcnow() and duration is computed correctly. * Clarify record_ti comment covers pre-set duration too * Snapshot rendered_map_index in TIH when a retry explicitly clears it (cherry picked from commit 66b803d) Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com>
…n task retry (#69248) (#69458) * Archive worker-reported end date and rendered map index on task retry Follow-up to #69235. The task_instance_history row for a retried try stamped archive-time utcnow() as end_date instead of the end date the worker reported, and missed the final rendered map index when the mid-run update was suppressed (e.g. template errors during failure handling). Snapshot both onto the TI before archiving, and let record_ti() respect a pre-set end_date so the audit trail reflects when the try actually ended. * Add test for record_ti fallback end_date stamping Cover the conditional branch where record_ti() archives a non-finished TI with end_date=None, verifying it gets stamped with utcnow() and duration is computed correctly. * Clarify record_ti comment covers pre-set duration too * Snapshot rendered_map_index in TIH when a retry explicitly clears it (cherry picked from commit 66b803d) Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com>
…n task retry (#69248) (#69458) * Archive worker-reported end date and rendered map index on task retry Follow-up to #69235. The task_instance_history row for a retried try stamped archive-time utcnow() as end_date instead of the end date the worker reported, and missed the final rendered map index when the mid-run update was suppressed (e.g. template errors during failure handling). Snapshot both onto the TI before archiving, and let record_ti() respect a pre-set end_date so the audit trail reflects when the try actually ended. * Add test for record_ti fallback end_date stamping Cover the conditional branch where record_ti() archives a non-finished TI with end_date=None, verifying it gets stamped with utcnow() and duration is computed correctly. * Clarify record_ti comment covers pre-set duration too * Snapshot rendered_map_index in TIH when a retry explicitly clears it (cherry picked from commit 66b803d) Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com>
… history (apache#69235) (apache#69241) AIP-105's per-try audit trail (retry_delay_override, retry_reason) is meant to live durably in task_instance_history — the columns on the live task_instance row are transient and cleared when the task next enters RUNNING. The Execution API retry handler wrote the overrides only into the live-row UPDATE and archived the task instance to history before those values were applied, so the history columns were always NULL. That silently dropped the audit trail the feature advertises, while retry timing (which reads the live row) stayed correct. Setting the overrides on the task instance before prepare_db_for_next_try() lets record_ti() snapshot them into task_instance_history, restoring the per-try record. (cherry picked from commit 3bfa5ff) Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
…n task retry (#69248) (#69458) * Archive worker-reported end date and rendered map index on task retry Follow-up to #69235. The task_instance_history row for a retried try stamped archive-time utcnow() as end_date instead of the end date the worker reported, and missed the final rendered map index when the mid-run update was suppressed (e.g. template errors during failure handling). Snapshot both onto the TI before archiving, and let record_ti() respect a pre-set end_date so the audit trail reflects when the try actually ended. * Add test for record_ti fallback end_date stamping Cover the conditional branch where record_ti() archives a non-finished TI with end_date=None, verifying it gets stamped with utcnow() and duration is computed correctly. * Clarify record_ti comment covers pre-set duration too * Snapshot rendered_map_index in TIH when a retry explicitly clears it (cherry picked from commit 66b803d) Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com>
…n task retry (#69248) (#69458) * Archive worker-reported end date and rendered map index on task retry Follow-up to #69235. The task_instance_history row for a retried try stamped archive-time utcnow() as end_date instead of the end date the worker reported, and missed the final rendered map index when the mid-run update was suppressed (e.g. template errors during failure handling). Snapshot both onto the TI before archiving, and let record_ti() respect a pre-set end_date so the audit trail reflects when the try actually ended. * Add test for record_ti fallback end_date stamping Cover the conditional branch where record_ti() archives a non-finished TI with end_date=None, verifying it gets stamped with utcnow() and duration is computed correctly. * Clarify record_ti comment covers pre-set duration too * Snapshot rendered_map_index in TIH when a retry explicitly clears it (cherry picked from commit 66b803d) Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com>
What's wrong
AIP-105 lets a retry policy choose a custom retry delay and reason each time a task fails. Those are meant to be recorded per-try in
task_instance_historyas a durable audit trail — "why did the policy wait N seconds on try 3, and for what reason?"Currently TIH table is always empty:
retry_delay_overrideandretry_reasoncome outNULLon everytask_instance_historyrow, even when a policy clearly set them.Root cause
In the Execution API retry handler, the override + reason are written only into the live-row
UPDATE, and the task instance is archived to history (prepare_db_for_next_try()→TaskInstanceHistory.record_ti()) before those values are set on the object being archived.record_ti()snapshots columns off the in-memory task-instance object, so it captures the still-unset (NULL) values.Fix
Set
retry_delay_override/retry_reasonon the task instance before it's archived, sorecord_ti()picks them up. Live-row behaviour and retry timing are unchanged.Testing
Adds a regression test that a policy-driven retry persists the override + reason into
task_instance_history(fails onmain, passes with the fix). The existing tests only asserted the live row, which is why this shipped.Before


After
DAG used for testing
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.8) following the guidelines