aeroyorch commented on code in PR #69007:
URL: https://github.com/apache/airflow/pull/69007#discussion_r3484855120
##########
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:
Hi @kaxil. I agree with your analysis for the scheduler use case. Before
opening #69001, I went back and read through the design decisions in #60804
because I expected we should first discuss the right approach before
implementing (or not) anything.
One thing I wasn't completely sure about from that discussion is whether the
intended solution for the scheduler was simply to rely on `num_runs` and
periodically restart the scheduler. If that's the recommended mitigation, I
think it would be worth documenting somewhere, since it's not immediately
obvious.
If we do want to introduce cache eviction for the scheduler, defaulting
`dag_cache_size` to `0` (unbounded) and relying only on TTL eviction seems like
a conservative choice. That addresses the leak of superseded `dag_version_id`
while avoiding the LRU thrashing concerns you described for large deployments.
Regarding the `RLock` overhead mentioned in #60804, I'm not sure whether the
performance impact is significant enough in practice to justify keeping an
unbounded `dict` in the scheduler forever.
--
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]
