Skip to content

Conversation

@kaxil
Copy link
Member

@kaxil kaxil commented Aug 11, 2025

Similar to #53838 but prevents it for all queries needing consumed_asset_events as mentioned in Future Considerations section. Alternative to #54334

Instead of adding .selectinload(DR.consumed_asset_events)) wherever needed, I am eagerly loading them now.

Changes:

  • Add lazy='selectin' to DagRun.consumed_asset_events relationship for always-eager loading
  • Changed backref to back_populates in AssetEvent.created_dagruns to enable explicit control

Why This Fix Works:

  • Eliminates lazy loading entirely by pre-loading the relationship at the model level
  • Prevents dependency on consistent session state in concurrent scheduler operations

Closes #54306


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@kaxil kaxil added this to the Airflow 3.0.5 milestone Aug 11, 2025
@kaxil kaxil requested review from XD-DENG and ashb as code owners August 11, 2025 00:01
@kaxil kaxil added the backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch label Aug 11, 2025
@kaxil kaxil force-pushed the fix-detached-error branch from 78fa9be to d932b79 Compare August 11, 2025 00:03
Similar to apache#53838 but prevents it for all queries needing `consumed_asset_events`.

Instead of adding `.selectinload(DR.consumed_asset_events))` wherever needed, I am eagerly loading them now.

Changes:
- Add lazy='selectin' to `DagRun.consumed_asset_events` relationship for always-eager loading
- Changed `backref` to `back_populates` in `AssetEvent.created_dagruns` to enable explicit control

Why This Fix Works:
- Eliminates lazy loading entirely by pre-loading the relationship at the model level
- Prevents dependency on consistent session state in concurrent scheduler operations

Closes apache#54306
@kaxil kaxil force-pushed the fix-detached-error branch from d932b79 to 964e710 Compare August 11, 2025 01:15
kaxil added a commit to astronomer/airflow that referenced this pull request Aug 11, 2025
Similar to apache#53838 and alternative for apache#54331

This is a more localized change and only eagerly loads for this specific instance.

Closes apache#54306
Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

I prefer #54334 -- this seems to run a lot more queries.

@kaxil kaxil closed this Aug 11, 2025
@kaxil kaxil removed this from the Airflow 3.0.5 milestone Aug 13, 2025
@kaxil kaxil deleted the fix-detached-error branch December 31, 2025 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants