fix(metrics): clear forward occupancy on idle (#33562)
Co-authored-by: Jialin Ouyang <Jialin.Ouyang@gmail.com>
This commit is contained in:
co-authored by
Jialin Ouyang
parent
dea2be5ae3
commit
5081c063c0
@@ -3904,9 +3904,6 @@ class Scheduler(
|
|||||||
# reset token ratio
|
# reset token ratio
|
||||||
self.new_token_ratio_tracker.reset()
|
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.
|
# Publish the idle state so /get_loads and DP balancing do not see stale load.
|
||||||
self.publish_load_snapshot(force=True)
|
self.publish_load_snapshot(force=True)
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import dataclasses
|
import dataclasses
|
||||||
import logging
|
import logging
|
||||||
|
import math
|
||||||
import tempfile
|
import tempfile
|
||||||
import time
|
import time
|
||||||
from collections import defaultdict
|
from collections import defaultdict
|
||||||
@@ -1080,7 +1081,7 @@ class SchedulerMetricsReporter:
|
|||||||
# the gauge. Readers sample it asynchronously, and the window
|
# the gauge. Readers sample it asynchronously, and the window
|
||||||
# boundary can phase-lock with the decode-log cadence, turning a
|
# boundary can phase-lock with the decode-log cadence, turning a
|
||||||
# one-tick NaN into NaN on every log line. NaN is published only
|
# 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_start = now
|
||||||
self._device_timer_window_gpu_time = 0.0
|
self._device_timer_window_gpu_time = 0.0
|
||||||
else:
|
else:
|
||||||
@@ -1093,13 +1094,20 @@ class SchedulerMetricsReporter:
|
|||||||
if self._device_timer_window_batch_count >= self.decode_log_interval:
|
if self._device_timer_window_batch_count >= self.decode_log_interval:
|
||||||
self._device_timer_window_batch_count = 0
|
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:
|
if ENABLE_METRICS_DEVICE_TIMER:
|
||||||
self._device_timer_window_batch_count = 0
|
self._device_timer_window_batch_count = 0
|
||||||
self.fwd_occupancy = float("nan")
|
self.fwd_occupancy = float("nan")
|
||||||
|
self.stats.fwd_occupancy = float("nan")
|
||||||
|
|
||||||
def _maybe_log_idle_metrics(self):
|
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:
|
if not self.current_scheduler_metrics_enabled:
|
||||||
return
|
return
|
||||||
# The running-reqs gauge holds the last batch report until the next
|
# The running-reqs gauge holds the last batch report until the next
|
||||||
@@ -1111,6 +1119,7 @@ class SchedulerMetricsReporter:
|
|||||||
)
|
)
|
||||||
if (
|
if (
|
||||||
not gauge_stale
|
not gauge_stale
|
||||||
|
and not is_fwd_occupancy_stale
|
||||||
and time.perf_counter() <= self.metrics_collector.last_log_time + 30
|
and time.perf_counter() <= self.metrics_collector.last_log_time + 30
|
||||||
):
|
):
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -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")
|
register_cpu_ci(est_time=5, suite="base-a-test-cpu")
|
||||||
|
|
||||||
|
import math
|
||||||
import types
|
import types
|
||||||
import unittest
|
import unittest
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
@@ -331,5 +332,72 @@ class TestForwardPassMetrics(unittest.TestCase):
|
|||||||
self.assertFalse(scheduler.enable_fpm)
|
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__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user