Mady356 commented on code in PR #69007:
URL: https://github.com/apache/airflow/pull/69007#discussion_r3484874303


##########
airflow-core/src/airflow/jobs/scheduler_job_runner.py:
##########
@@ -347,8 +347,25 @@ def __init__(
 
         if log:
             self._log = log
+        dag_cache_size = conf.getint("scheduler", "dag_cache_size", 
fallback=1024)
+        dag_cache_ttl_config = conf.getint("scheduler", "dag_cache_ttl", 
fallback=10800)
 
-        self.scheduler_dag_bag = DBDagBag(load_op_links=False)
+        if dag_cache_size < 0:
+            self.log.warning("scheduler dag_cache_size must be >= 0, using 
unbounded dict")
+            dag_cache_size = 0
+
+        if dag_cache_ttl_config < 0:
+            self.log.warning("scheduler dag_cache_ttl must be >= 0, disabling 
TTL")
+            dag_cache_ttl_config = 0
+
+        dag_cache_ttl = dag_cache_ttl_config if dag_cache_ttl_config > 0 else 
None
+
+        self.scheduler_dag_bag = DBDagBag(
+            load_op_links=False,
+            cache_size=dag_cache_size,
+            cache_ttl=dag_cache_ttl,
+            stats_prefix="scheduler.dag_bag",

Review Comment:
   That makes a lot of sense and I understand why a low count-based LRU default 
is risky for the scheduler: if the active dag_version_id working set is larger 
than the cache size, the scheduler can end up thrashing and repeatedly paying 
the DB read/deserialization cost.
   
   My current thinking is to revise this so scheduler eviction is TTL-driven by 
default instead of size-cap driven. Concretely, that would mean defaulting 
scheduler dag_cache_size to 0, keeping dag_cache_ttl set, and updating DBDagBag 
so a non-zero TTL can evict entries even when there is no count-based maxsize. 
That should target the superseded dag_version_id growth without evicting the 
active cyclic working set.
   
   I’d also remove the redundant conf fallbacks and add tests for the 
non-positive cache size / TTL path.
   
   Does that direction sound reasonable before I rework the PR?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to