You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#71704 originally bundled four separable things. Per review feedback there, it now ships only the scheduler cache bound — one line, no new configuration, no DBDagBag API change — so it can be cherry-picked cleanly. This branch carries the rest.
What is still here
[api] dag_cache_size = 0 silently 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.
DBDagBag gains stats_prefix, so each component reports cache activity under its own metric namespace instead of every caller emitting api_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 their version_added correction (3.3.0 was wrong; those options shipped in 3.2.2).
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.
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 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
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