Skip to content

Reduce redundant Dag team lookups in authorization checks#68020

Merged
vincbeck merged 1 commit into
apache:mainfrom
yuseok89:skip-redundant-team-lookups-in-dag-access
Jun 5, 2026
Merged

Reduce redundant Dag team lookups in authorization checks#68020
vincbeck merged 1 commit into
apache:mainfrom
yuseok89:skip-redundant-team-lookups-in-dag-access

Conversation

@yuseok89

@yuseok89 yuseok89 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

closes: #61485

requires_access_dag resolves a Dag's owning team via DagModel.get_team_name on every call — twice per grid endpoint (TASK_INSTANCE + RUN) and on every auto-refresh poll.

This caches the lookup with a short per-process TTL.

Other large costs discussed in #61485 have already been addressed by separate PRs. I left the detailed breakdown in the issue comment, and this PR covers the remaining redundant Dag team lookup.

Impact

Team-ownership DB queries per grid page load:

first load follow-up poll
before 6 6
after 1 0

Also included

  • New config [core] team_name_cache_ttl (default 30s).

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

  • 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:ConfigTemplates labels Jun 4, 2026
@yuseok89 yuseok89 force-pushed the skip-redundant-team-lookups-in-dag-access branch 2 times, most recently from e8a6b1b to d7b301e Compare June 4, 2026 14:13
@yuseok89 yuseok89 marked this pull request as ready for review June 4, 2026 14:19
@yuseok89 yuseok89 requested review from XD-DENG and ashb as code owners June 4, 2026 14:19
@vincbeck

vincbeck commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

CI is failing

@henry3260 henry3260 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good!
test_bulk_delete_query_count_scales_linearly_with_task_count still asserts
the old exact counts, we should update it

Comment thread airflow-core/newsfragments/68020.improvement.rst Outdated
@yuseok89 yuseok89 force-pushed the skip-redundant-team-lookups-in-dag-access branch from d7b301e to 7a919b3 Compare June 4, 2026 19:57
@yuseok89

yuseok89 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

CI is failing

You're right, sorry. Once it was approved I moved it out of draft, but an approval isn't the same as a green CI, and I should have confirmed that first.

The failure wasn't in the tests this change adds. Caching the Dag team lookup shaves one query off the bulk task-instance delete path, which tripped a pre-existing query-count regression guard there. That's the cross-area interaction I didn't catch. It's now fixed and pushed.

@vincbeck vincbeck merged commit 477cbed into apache:main Jun 5, 2026
143 checks passed
@vatsrahul1001 vatsrahul1001 added this to the Airflow 3.3.0 milestone Jun 15, 2026
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:ConfigTemplates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache requires_access_dag checks and other auth manager redundant checks

4 participants