Report Dag cache metrics under each component's own namespace - #71815
Conversation
boring-cyborg
Bot
added
area:API
19d7b1d to
f6847cc
Compare
Every DBDagBag emitted its cache counters under api_server.dag_bag.*, so once the scheduler gained a cache its traffic was silently counted against the API server's series. An operator reading cache_hit or cache_size could not tell the two components apart, and the API server's numbers became wrong rather than merely incomplete. Each caller now supplies the namespace it reports under, and a cache built without one fails at construction instead of emitting a partially-formed metric name mid-request. The registry check matched dynamic metric names only by the static prefix ahead of their first variable, which cannot express a name assembled from a caller supplied prefix. It now matches on all static parts wherever the variable sits.
f6847cc to
8b2ad41
Compare
|
Instead of an argument in DBDagBag to make it double-duty, I feel it’s cleaner if we make the |
DBDagBag is used by callers that do not configure cache eviction or metrics. Keeping those concerns behind a dedicated subtype prevents optional constructor combinations and makes metric ownership explicit.
jason810496
left a comment
jason810496
left a comment
There was a problem hiding this comment.
Instead of an argument in DBDagBag to make it double-duty, I feel it’s cleaner if we make the
_use_cache=Truevariant a separate subclass i.e.class CachedDBDagBag. This way, the arguments will be cleaner (no more complicated optional arguments + checks in__init__, and the stats functions only need to be on the class they’re actually needed.
It makes sense to separate them by purpose, thanks for the suggestion. I just updated it.
Backport failed to create: v3-3-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker bcf1dbf v3-3-testThis should apply the commit to the v3-3-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
fix/api-dag-cache-ttl-ignored...feat/dagbag-cache-metric-namespaceWhy
Every configured
DBDagBagemitted cache counters underapi_server.dag_bag.*. Once the scheduler gained a bounded cache in #71704, scheduler traffic was counted against the API server's series, so operators could not distinguish the two components.The cache settings and metrics also made every
DBDagBagcaller opt into an interface intended only for long-lived component caches.How
DBDagBagremains the simple base implementation for callers that do not configure eviction or cache metrics.CachedDBDagBagowns LRU, TTL, and no-eviction cache modes, along with thread safety and the required component metric namespace.The API server always constructs
CachedDBDagBagfrom its cache configuration. With size and TTL both set to 0, the subclass uses a plain unbounded dict and performs no eviction. The scheduler constructsCachedDBDagBagunderscheduler.dag_bag.*.The metrics registry check now matches every static part of a dynamic metric name, including names assembled from a caller-supplied prefix.
What
api_server.dag_bag.*.scheduler.dag_bag.*.DBDagBag.0/0no-eviction behavior while keeping configured component caches onCachedDBDagBag.Supersedes #71276.
Was generative AI tooling used to co-author this PR?