Fix scheduler DBDagBag unbounded cache - #71704
Conversation
e3a4ee1 to
09561d4
Compare
09561d4 to
522a76d
Compare
522a76d to
95c235d
Compare
I'm thinking its too large. 512 or 256 might be a safer default? (I just worry about how much memory 1024 dags in memory at once could take up |
|
Let’s say a dag needs 500 bytes to store a task, and each dag has 100 tasks. That’s 50k for a dag. 256→13m, 512→25m, 1024→50m. It doesn’t sound too bad to me. Let’s do 512? |
|
I think we should roll back all API changes in this PR and simply change the scheduler. The scheduler cache also should not rely on any configurations. (It currently still does |
Sure, then I will just make this one a minimal fix (just hardcoded the cache size as |
4b308b2 to
f2d6575
Compare
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.
The scheduler kept every Dag version it deserialized in a mapping that never evicted, so a long-running scheduler grew with the number of versions it had ever seen until it was restarted or OOM killed. Deployments that redeploy Dags frequently accumulate versions fastest and hit this soonest. A least-recently-used cap is the only thing that bounds this outright. An idle timeout would not: the scheduler re-checks an entry on each lookup, which re-arms its expiry, so a timeout reclaims a version only once its runs finish and it stops being requested, leaving memory a function of the concurrently active set rather than a fixed ceiling. Deliberately not configurable here, so the fix stays small enough to cherry-pick. Cache activity currently reports under the existing api_server.dag_bag.* metrics; a scheduler-specific namespace, along with configuration, follows separately. closes: apache#69001
f2d6575 to
addab6b
Compare
Backport successfully created: v3-3-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
…1821) The scheduler kept every Dag version it deserialized in a mapping that never evicted, so a long-running scheduler grew with the number of versions it had ever seen until it was restarted or OOM killed. Deployments that redeploy Dags frequently accumulate versions fastest and hit this soonest. A least-recently-used cap is the only thing that bounds this outright. An idle timeout would not: the scheduler re-checks an entry on each lookup, which re-arms its expiry, so a timeout reclaims a version only once its runs finish and it stops being requested, leaving memory a function of the concurrently active set rather than a fixed ceiling. Deliberately not configurable here, so the fix stays small enough to cherry-pick. Cache activity currently reports under the existing api_server.dag_bag.* metrics; a scheduler-specific namespace, along with configuration, follows separately. (cherry picked from commit 29dd99d) closes: #69001 Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com>
DBDagBagcache is never evicted and grows unbounded #69001DBDagBagTTL cache eviction without a size cap #69774, Use bounded DBDagBag cache in scheduler #69007Why
The scheduler kept every Dag version it deserialized in a mapping that never evicted, so a long-running scheduler grew with the number of versions it had ever seen until it was restarted or OOM killed. Deployments that redeploy Dags frequently accumulate versions fastest and hit this soonest.
How
One line.
DBDagBagalready builds a bounded LRU cache when given acache_size; the scheduler simply never passed one.No changes to
DBDagBag, to the API server, or to configuration — deliberately, so this cherry-picks cleanly.Why a size cap rather than a TTL
A least-recently-used cap is the only thing that bounds the cache outright. An idle timeout would not:
_get_dagre-checks an entry on each lookup, and cachetools re-arms expiry on assignment, so a TTL reclaims a version only once its runs finish and it stops being requested. That leaves memory a function of the concurrently active set rather than a fixed ceiling.Where the working set exceeds the cap, the evicted version is re-fetched and deserialized on the next loop. Correctness is unaffected — a cache miss is a re-read, never a wrong Dag.
Known limitation
Enabling the cache also enables
DBDagBag's metrics, which are currently hard-coded toapi_server.dag_bag.*. Those counters therefore now include scheduler traffic. Giving each component its own namespace requires aDBDagBagAPI change, deferred to #71815. Making the cap configurable is #71816, and #71814 fixes[api] dag_cache_size = 0silently ignoring[api] dag_cache_ttl.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5) following the guidelines