Make the Dag cache configurable and per-component in metrics - #71813
Closed
jason810496 wants to merge 6 commits into
Closed
Make the Dag cache configurable and per-component in metrics#71813jason810496 wants to merge 6 commits into
jason810496 wants to merge 6 commits into
Conversation
The scheduler cached deserialized Dags in a dict that never evicted, so every Dag version it had ever seen stayed resident and the process was eventually OOM killed. ``[api] dag_cache_size = 0`` had a related gap: it selected that same never-evicting dict and silently ignored ``[api] dag_cache_ttl``, so TTL eviction could not be enabled without also accepting a size limit. Each re-check resets a cached entry's expiry, so the TTL reclaims a version once its Dag runs finish and it stops being requested; ``dag_cache_size`` remains the only hard ceiling. closes: apache#69001
- Move the scheduler's DagBag factory into `scheduler_job_runner` as `_create_scheduler_dag_bag`, dropping the `airflow/jobs/scheduler_dagbag.py` module and the `SchedulerDBDagBag` subclass it held. - Collapse `APIServerDBDagBag` back into the existing `create_dag_bag`, so both components resolve their config section and metric prefix in a plain factory next to where they build the bag. - Drop `airflow-core/tests/unit/jobs/test_scheduler_dagbag.py`. - Trim `test_cache_selection` from 11 cases to the 5 distinct branches of the mapping selection, and drop `test_uncapped_ttl_cache_accepts_entries`, which only asserted that cachetools honours an unbounded `maxsize`. - Un-parametrize the four cache-metric tests over the two components. The subclasses never overrode the `_stat_*` hooks, so both runs exercised the same base code; `test_stats_prefix_expands_to_registered_metrics` still pins each component's real prefix against the metrics registry. - Drop `dev/airflow_perf/dag_bag_cache_overhead.py`. An in-memory dict lookup is orders of magnitude cheaper than the DB load and deserialize it guards, so the harness has no long-term value in the repo.
A TTL cannot cap the cache on its own. Each re-check re-arms an entry's expiry, so a TTL reclaims a Dag version only once its runs finish and it stops being requested — that bounds memory by the concurrently active set, which no fixed number predicts, rather than outright. A default that leaves memory dependent on request patterns is the wrong default for the OOM this PR set out to fix. A size limit is the only hard ceiling, so the scheduler now defaults to `dag_cache_size = 1024` with `dag_cache_ttl = 0`. 1024 is meant to sit above the versions-with-runs-in-flight working set of a typical deployment, so eviction costs a re-fetch only where that working set is genuinely larger; the `scheduler.dag_bag.cache_miss` metric is what tells an operator to raise it. The TTL-only and no-eviction modes both remain reachable by configuration.
These options are new on main, which is 3.4.0. Claiming 3.3.2 would advertise them as available in a patch release that never carried them, sending anyone on 3.3.x looking for settings they cannot configure. The surrounding docs described the pre-cache behaviour as ending at the same wrong version, so they move together. The `[api]` pair keeps 3.2.2: those options already shipped, and that value is the correction this PR makes to their previously mis-stated 3.3.0.
Ash flagged 1024 as possibly too aggressive a default. 512 still sits above the versions-with-runs-in-flight working set of a typical deployment while roughly halving the worst-case memory footprint of the default cache.
boring-cyborg
Bot
added
area:API
1 task
The scheduler cache bound it originally accompanied now ships separately in apache#71704; what remains here is the configuration, metric namespacing, and the `[api]` TTL fix, so the entry belongs to this PR's number.
This was referenced Aug 19, 2026
Member
Author
|
Split into three focused PRs, so closing this:
The scheduler cache bound itself ships separately in #71704. |
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
1 participant
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.
Draft. Holds the work carved out of #71704 so nothing is lost while that PR is reduced to the minimal, cherry-pickable scheduler fix.
related: #71704
Why this exists
#71704 originally bundled four separable things. Per review feedback there, it now ships only the scheduler cache bound — one line, no new configuration, no
DBDagBagAPI change — so it can be cherry-picked cleanly. This branch carries the rest.What is still here
[api] dag_cache_size = 0silently ignored[api] dag_cache_ttl. Selecting "no size limit" also disabled TTL eviction, so TTL could not be enabled without also accepting a size cap. A pre-existing bug in shipped behaviour, and backportable on its own.DBDagBaggainsstats_prefix, so each component reports cache activity under its own metric namespace instead of every caller emittingapi_server.dag_bag.*. The minimal fix in Fix scheduler DBDagBag unbounded cache #71704 leaves the scheduler reporting under the API server's names; this is what corrects that.[scheduler] dag_cache_size/dag_cache_ttl, making the bound introduced in Fix scheduler DBDagBag unbounded cache #71704 tunable, plus the[api]option docs and theirversion_addedcorrection (3.3.0 was wrong; those options shipped in 3.2.2).{variable}can sit anywhere in a metric name rather than only after a fixed prefix. Supersedes Support dynamic metric name prefixes in the metrics registry check #71276.Planned split
This is not intended to merge as one PR. It will be split into at least:
[api]TTL bugfix, targeted for backport;stats_prefix/ metric-namespace change plus the registry check;[scheduler]configuration, for a minor release.Kept as a single draft for now so the deferred work is reviewable in one place and visibly not dropped.
Note
Branched before #71704 was reduced, so it needs rebasing onto current
mainbefore any of the above is split out for real review.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5) following the guidelines