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]