From 5081c063c0105363ac01acf051fd94dd44ddd915 Mon Sep 17 00:00:00 2001 From: Lianmin Zheng Date: Tue, 4 Aug 2026 12:49:17 -0700 Subject: [PATCH] fix(metrics): clear forward occupancy on idle (#33562) Co-authored-by: Jialin Ouyang --- python/sglang/srt/managers/scheduler.py | 3 - .../scheduler_components/metrics_reporter.py | 15 +++- .../test_forward_pass_metrics.py | 68 +++++++++++++++++++ 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/python/sglang/srt/managers/scheduler.py b/python/sglang/srt/managers/scheduler.py index ea281ee48..8f4ed02be 100644 --- a/python/sglang/srt/managers/scheduler.py +++ b/python/sglang/srt/managers/scheduler.py @@ -3904,9 +3904,6 @@ class Scheduler( # reset token ratio self.new_token_ratio_tracker.reset() - # reset device timer window so idle time isn't counted - self.metrics_reporter.reset_device_timer_window() - # Publish the idle state so /get_loads and DP balancing do not see stale load. self.publish_load_snapshot(force=True) diff --git a/python/sglang/srt/managers/scheduler_components/metrics_reporter.py b/python/sglang/srt/managers/scheduler_components/metrics_reporter.py index ab4880801..13ba75da5 100644 --- a/python/sglang/srt/managers/scheduler_components/metrics_reporter.py +++ b/python/sglang/srt/managers/scheduler_components/metrics_reporter.py @@ -2,6 +2,7 @@ from __future__ import annotations import dataclasses import logging +import math import tempfile import time from collections import defaultdict @@ -1080,7 +1081,7 @@ class SchedulerMetricsReporter: # the gauge. Readers sample it asynchronously, and the window # boundary can phase-lock with the decode-log cadence, turning a # one-tick NaN into NaN on every log line. NaN is published only - # when truly stale (reset_device_timer_window after idle). + # when truly stale (_reset_device_timer_window after idle). self._device_timer_window_start = now self._device_timer_window_gpu_time = 0.0 else: @@ -1093,13 +1094,20 @@ class SchedulerMetricsReporter: if self._device_timer_window_batch_count >= self.decode_log_interval: self._device_timer_window_batch_count = 0 - def reset_device_timer_window(self): + def _reset_device_timer_window(self): + """Exclude idle time and invalidate the last forward-occupancy sample.""" if ENABLE_METRICS_DEVICE_TIMER: self._device_timer_window_batch_count = 0 self.fwd_occupancy = float("nan") + self.stats.fwd_occupancy = float("nan") def _maybe_log_idle_metrics(self): - """Collect and log metrics every 30 seconds during idle.""" + """Reset forward timing and publish idle metrics when needed.""" + # Preserve the transition so the rate limit cannot leave a finite idle gauge. + is_fwd_occupancy_stale = ENABLE_METRICS_DEVICE_TIMER and not math.isnan( + self.stats.fwd_occupancy + ) + self._reset_device_timer_window() if not self.current_scheduler_metrics_enabled: return # The running-reqs gauge holds the last batch report until the next @@ -1111,6 +1119,7 @@ class SchedulerMetricsReporter: ) if ( not gauge_stale + and not is_fwd_occupancy_stale and time.perf_counter() <= self.metrics_collector.last_log_time + 30 ): return diff --git a/test/registered/unit/observability/test_forward_pass_metrics.py b/test/registered/unit/observability/test_forward_pass_metrics.py index fdb7774b9..4e94f44f6 100644 --- a/test/registered/unit/observability/test_forward_pass_metrics.py +++ b/test/registered/unit/observability/test_forward_pass_metrics.py @@ -3,6 +3,7 @@ from sglang.test.ci.ci_register import register_cpu_ci register_cpu_ci(est_time=5, suite="base-a-test-cpu") +import math import types import unittest from unittest.mock import patch @@ -331,5 +332,72 @@ class TestForwardPassMetrics(unittest.TestCase): self.assertFalse(scheduler.enable_fpm) +class TestIdleMetrics(unittest.TestCase): + def setUp(self): + self.scheduler = types.SimpleNamespace( + running_batch=types.SimpleNamespace(reqs=[]), + waiting_queue=[], + grammar_manager=[], + enable_priority_scheduling=False, + disaggregation_mode=DisaggregationMode.NULL, + pool_stats_observer=types.SimpleNamespace( + get_pool_stats=lambda: types.SimpleNamespace( + update_scheduler_stats=lambda _: None + ), + streaming_session_count=lambda: 0, + session_held_tokens=lambda: 0, + ), + ) + self.reporter = _make_reporter(self, self.scheduler) + self.published_occupancies = [] + self.reporter.metrics_collector = types.SimpleNamespace( + last_log_time=100.0, + log_stats=lambda stats: self.published_occupancies.append( + stats.fwd_occupancy + ), + ) + + def test_idle_clears_cached_forward_occupancy_immediately(self): + self.reporter.current_scheduler_metrics_enabled = True + self.reporter.fwd_occupancy = 72.0 + self.reporter.stats.fwd_occupancy = 72.0 + self.reporter._device_timer_window_batch_count = 7 + + with ( + patch( + "sglang.srt.managers.scheduler_components.metrics_reporter.ENABLE_METRICS_DEVICE_TIMER", + True, + ), + patch( + "sglang.srt.managers.scheduler_components.metrics_reporter.time.perf_counter", + return_value=101.0, + ), + ): + self.reporter._maybe_log_idle_metrics() + self.reporter._maybe_log_idle_metrics() + + self.assertEqual(len(self.published_occupancies), 1) + self.assertTrue(math.isnan(self.published_occupancies[0])) + self.assertTrue(math.isnan(self.reporter.fwd_occupancy)) + self.assertTrue(math.isnan(self.reporter.stats.fwd_occupancy)) + self.assertEqual(self.reporter._device_timer_window_batch_count, 0) + + def test_idle_resets_forward_timing_when_metrics_are_disabled(self): + self.reporter.fwd_occupancy = 72.0 + self.reporter.stats.fwd_occupancy = 72.0 + self.reporter._device_timer_window_batch_count = 7 + + with patch( + "sglang.srt.managers.scheduler_components.metrics_reporter.ENABLE_METRICS_DEVICE_TIMER", + True, + ): + self.reporter._maybe_log_idle_metrics() + + self.assertTrue(math.isnan(self.reporter.fwd_occupancy)) + self.assertTrue(math.isnan(self.reporter.stats.fwd_occupancy)) + self.assertEqual(self.reporter._device_timer_window_batch_count, 0) + self.assertEqual(self.published_occupancies, []) + + if __name__ == "__main__": unittest.main()