Skip to content

Fix new DAG versions from serializing a retry_policy object#69243

Merged
vatsrahul1001 merged 4 commits into
mainfrom
fix-mapped-retry-policy-serialization
Jul 3, 2026
Merged

Fix new DAG versions from serializing a retry_policy object#69243
vatsrahul1001 merged 4 commits into
mainfrom
fix-mapped-retry-policy-serialization

Conversation

@vatsrahul1001

Copy link
Copy Markdown
Contributor

What's wrong

A mapped task that uses a retry policy (.partial(retry_policy=…).expand(…) or
@task(retry_policy=…).expand(…)) gets a new DAG version created on nearly every
DAG-processor re-serialization (~30s)
— even when the DAG is idle and never run —
spamming the DAG version history and bloating the metadata DB (dag_version /
serialized_dag).

Root cause

A mapped operator keeps retry_policy in partial_kwargs, and the mapped-operator
serializer has no serializer for a RetryPolicy, so it falls back to str(obj)
"<…ExceptionRetryPolicy object at 0x…>". That memory address changes on every parse,
so dag_hash changes each time, defeating write_dag's "unchanged" check and creating
a new DagVersion on every re-serialization. Non-mapped operators already avoid this by
excluding the object and storing only a has_retry_policy flag.

Fix

Mirror the non-mapped path in the mapped-operator serializer: don't serialize the object,
store only a has_retry_policy flag, and add has_retry_policy to SerializedMappedOperator
so it round-trips. Runtime is unaffected — the worker re-parses the DAG source for the live
policy, so retries still work.

Testing

Adds regression tests that a mapped task with a retry_policy (a) serializes
deterministically (identical across parses, no embedded object) and (b) round-trips
has_retry_policy=True. Both fail on main and pass with the fix.

Before

  1. Try below DAG and create a dag_run.
  2. Execute SELECT count(*) FROM dag_version WHERE dag_id='aip105_mapped_serialize_repro' and confirm new version is created
from airflow.sdk import DAG, ExceptionRetryPolicy, RetryAction, RetryRule, task

POLICY = ExceptionRetryPolicy(
    rules=[
        RetryRule(
            exception=ConnectionError,
            action=RetryAction.RETRY,
            retry_delay=timedelta(seconds=30),
            reason="Transient, backing off 30s",
        ),
    ],
)

with DAG(
    dag_id="aip105_mapped_serialize_repro",
    schedule=None,
    catchup=False,
    tags=["qa", "aip105", "serialization-bug"],
):

    @task(retries=3, retry_policy=POLICY)
    def mapped_with_policy(x):
        return x

    @task(retries=3, retry_policy=POLICY)
    def plain_with_policy():
        return 1

    mapped_with_policy.expand(x=[1, 2, 3])
    plain_with_policy()

After

  1. Run same DAG and check new version is not getting created
Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.8)

Generated-by: Claude Code (Opus 4.8) following the guidelines

A mapped operator keeps retry_policy in partial_kwargs, but the mapped-operator
serializer has no serializer for a RetryPolicy, so it fell back to str(obj) --
embedding the object's per-process memory address. That made the serialized DAG
non-deterministic: dag_hash changed on every parse, defeating write_dag's
"unchanged" check and creating a new DagVersion on nearly every DAG-processor
re-serialization (even for idle, never-run DAGs), polluting version history and
bloating the metadata DB.

Non-mapped operators already avoid this by excluding the object and storing only
a has_retry_policy flag; this makes the mapped path do the same. Runtime is
unaffected -- the worker re-parses the DAG source for the live policy.
Comment thread airflow-core/src/airflow/serialization/serialized_objects.py Outdated
Comment thread airflow-core/tests/unit/serialization/test_serialized_objects.py Outdated
A RetryPolicy set via DAG default_args hit the same str(obj) fallback as the
mapped-operator case, embedding a memory address and producing a new DagVersion
on every parse. Route both the mapped partial_kwargs loop and the default_args
cleanup through a shared has-flag set (callbacks + retry_policy) so the object is
never serialized on either path.
@vatsrahul1001 vatsrahul1001 changed the title Fix mapped task retry policy creating a new DAG version each parse Fix spurious DAG versions from serializing a retry_policy object Jul 3, 2026
@vatsrahul1001 vatsrahul1001 changed the title Fix spurious DAG versions from serializing a retry_policy object Fix new DAG versions from serializing a retry_policy object Jul 3, 2026
@vatsrahul1001 vatsrahul1001 requested a review from kaxil July 3, 2026 04:50
Comment thread airflow-core/tests/unit/serialization/test_serialized_objects.py
Mapped operators resolve has_retry_policy through _get_partial_kwargs_or_operator_default
(falling back to SerializedBaseOperator's default) rather than the dataclass default, so the
false case warrants its own test alongside the existing non-mapped one.
@vatsrahul1001 vatsrahul1001 added this to the Airflow 3.3.0 milestone Jul 3, 2026
Comment thread airflow-core/tests/unit/serialization/test_serialized_objects.py
Comment thread airflow-core/tests/unit/serialization/test_serialized_objects.py Outdated
Comment thread airflow-core/tests/unit/serialization/test_serialized_objects.py Outdated
Comment thread airflow-core/tests/unit/serialization/test_serialized_objects.py Outdated
Comment thread airflow-core/tests/unit/serialization/test_serialized_objects.py

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

Few non-blocking comments but lgtm otherwise

Review follow-ups: move the retry_policy imports to module top (no collision, unlike
the SDK DAG import which stays local), shrink the has-flag branch comment to point at
_HAS_FLAG_FIELDS, and assert the positive side (has_retry_policy written, retry_policy
stripped) in the default_args test.
@vatsrahul1001 vatsrahul1001 merged commit b56771c into main Jul 3, 2026
78 checks passed
@vatsrahul1001 vatsrahul1001 deleted the fix-mapped-retry-policy-serialization branch July 3, 2026 09:02
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-3-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
v3-3-test PR Link

vatsrahul1001 added a commit that referenced this pull request Jul 3, 2026
…ct (#69243) (#69315)

* Fix non-deterministic serialization of mapped task retry_policy

A mapped operator keeps retry_policy in partial_kwargs, but the mapped-operator
serializer has no serializer for a RetryPolicy, so it fell back to str(obj) --
embedding the object's per-process memory address. That made the serialized DAG
non-deterministic: dag_hash changed on every parse, defeating write_dag's
"unchanged" check and creating a new DagVersion on nearly every DAG-processor
re-serialization (even for idle, never-run DAGs), polluting version history and
bloating the metadata DB.

Non-mapped operators already avoid this by excluding the object and storing only
a has_retry_policy flag; this makes the mapped path do the same. Runtime is
unaffected -- the worker re-parses the DAG source for the live policy.

* Also strip retry_policy from DAG default_args serialization

A RetryPolicy set via DAG default_args hit the same str(obj) fallback as the
mapped-operator case, embedding a memory address and producing a new DagVersion
on every parse. Route both the mapped partial_kwargs loop and the default_args
cleanup through a shared has-flag set (callbacks + retry_policy) so the object is
never serialized on either path.

* Add negative test for mapped task without a retry_policy

Mapped operators resolve has_retry_policy through _get_partial_kwargs_or_operator_default
(falling back to SerializedBaseOperator's default) rather than the dataclass default, so the
false case warrants its own test alongside the existing non-mapped one.

* Hoist retry_policy test imports and assert default_args has_retry_policy

Review follow-ups: move the retry_policy imports to module top (no collision, unlike
the SDK DAG import which stays local), shrink the has-flag branch comment to point at
_HAS_FLAG_FIELDS, and assert the positive side (has_retry_policy written, retry_policy
stripped) in the default_args test.
(cherry picked from commit b56771c)

Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants