[
https://issues.apache.org/jira/browse/YUNIKORN-3310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18093101#comment-18093101
]
Wilfred Spiegelenburg commented on YUNIKORN-3310:
-------------------------------------------------
This affects not just the dynamic queues but also the configured queues. Both
queues can be removed and added back in later. The change fixes it consistently
for both queue types.
The documentation also needs to be updated to add the new metric to the [queue
metrics|https://yunikorn.apache.org/docs/next/metrics/queue] page. A PR can be
raised using this same jira
> Dynamic queue removal resets terminal app state metrics to zero
> ---------------------------------------------------------------
>
> Key: YUNIKORN-3310
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3310
> Project: Apache YuniKorn
> Issue Type: Bug
> Components: core - scheduler
> Affects Versions: 1.7.0, 1.8.0
> Reporter: Guruprasad Veerannavaru
> Assignee: Guruprasad Veerannavaru
> Priority: Minor
> Labels: metrics, pull-request-available
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> h3. Background
> {{yunikorn_queue_app}} is a Gauge that tracks application counts per state
> per queue. For terminal states (completed, failed, rejected), the value only
> ever increments — apps enter these states and never leave. This makes
> terminal states effectively cumulative counters, not point-in-time gauges.
> Dynamic (unmanaged) queues are removed when they have no applications. The
> completed app is moved out of {{queue.applications}} immediately on entering
> Completed (via {{UnSetQueue}} → {{RemoveApplication}}), so the queue can
> become empty within seconds — well before the 3-day {{terminatedTimeout}}
> that governs app cleanup from the partition.
> h3. What YUNIKORN-2908 introduced
> [YUNIKORN-2908|https://issues.apache.org/jira/browse/YUNIKORN-2908] added
> {{RemoveQueueMetrics()}} on queue removal to fix two issues with *resource
> metrics*:
> # After queue removal, metrics like guaranteed and max resources continued to
> be reported by Prometheus, giving a wrong impression that resources were
> still allocated to a non-existent queue.
> # When resource config was removed from a queue (or a resource type removed
> from config), stale metric values were never cleaned up — metrics are only
> updated when there's a new valid value, not a 'null' value.
> The fix correctly unregisters all queue metrics on removal. This is the right
> behavior for resource metrics (guaranteed, max, allocated) — they represent
> instantaneous state that should not persist after queue removal.
> h3. What broke
> Terminal state gauges (completed, failed, rejected) were also wiped by the
> blanket {{RemoveQueueMetrics()}}. When the queue is recreated on next app
> arrival, all metrics restart at 0. Since Prometheus often hasn't scraped the
> final value before removal (queue empties within seconds of the last
> completion), Grafana shows drops in
> {{yunikorn_queue_app\{state="completed"\}}} even though apps are completing
> successfully.
> Before YUNIKORN-2908, terminal gauges leaked in the registry — which
> accidentally preserved their values across queue lifecycles.
> h3. Why terminal states should not be a Gauge
> A gauge represents a value that goes up and down (e.g., currently running
> apps). Terminal states are monotonically increasing — an app that completed
> stays completed. A Counter is the correct Prometheus type for this semantic.
> h3. Proposed fix
> Add a new CounterVec {{yunikorn_queue_app_total}} for terminal states
> (completed, failed, rejected). This counter is excluded from
> {{RemoveQueueMetrics()}} — it stays in the Prometheus registry when the queue
> is removed. On queue recreation, the existing counter is reused (via
> Prometheus {{AlreadyRegisteredError}}), preserving accumulated values.
> The existing {{yunikorn_queue_app}} gauge remains unchanged for backward
> compatibility.
> *Usage in Grafana:*
> {code}
> increase(yunikorn_queue_app_total{queue="root.shared.sparksynthetics",
> state="completed"}[1h])
> {code}
> h3. FAQ
> *Will this increase memory usage?*
> Minimally. Each dynamic queue retains one CounterVec with 3 label values
> (completed, failed, rejected) in the Prometheus registry after removal. This
> is a few hundred bytes per unique queue path. In practice, dynamic queues are
> created per-user or per-namespace — this is bounded and far smaller than the
> memory already used by the queue objects themselves during their active
> lifetime. This is also the same behavior as before YUNIKORN-2908 where all
> metrics leaked in the registry.
> *Does this increase metric cardinality?*
> It adds 3 new time series per queue (one per terminal state). Cardinality
> scales with the number of unique queue paths, which is already the case for
> existing metrics. No new label dimensions are introduced.
> *Does this break backward compatibility?*
> No. The existing {{yunikorn_queue_app}} gauge is unchanged. The new
> {{yunikorn_queue_app_total}} counter is additive — existing dashboards and
> alerts continue to work as before.
> *Why not fix the existing gauge instead of adding a new metric?*
> The gauge semantic (value goes up and down) is fundamentally wrong for
> terminal states. Fixing it within the gauge would mean either never
> unregistering it (reintroducing the leak YUNIKORN-2908 fixed for resource
> metrics) or adding special-case logic to {{UnregisterMetrics()}}. A separate
> Counter with the correct Prometheus type is cleaner and follows Prometheus
> naming conventions ({{_total}} suffix for counters).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
