Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions vllm/v1/engine/llm_engine.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project

import time
from collections.abc import Mapping
from copy import copy
from typing import Any, Callable, Optional, Union
Expand Down Expand Up @@ -31,8 +32,7 @@
from vllm.v1.engine.parallel_sampling import ParentRequest
from vllm.v1.engine.processor import Processor
from vllm.v1.executor.abstract import Executor
from vllm.v1.metrics.loggers import (PrometheusStatLogger, StatLoggerBase,
StatLoggerFactory)
from vllm.v1.metrics.loggers import StatLoggerFactory, StatLoggerManager
from vllm.v1.metrics.reader import Metric, get_metrics_snapshot
from vllm.v1.metrics.stats import IterationStats
from vllm.v1.worker.worker_base import WorkerBase
Expand Down Expand Up @@ -74,9 +74,6 @@ def __init__(
self.cache_config = vllm_config.cache_config

self.log_stats = log_stats
self.stat_logger: Optional[StatLoggerBase] = None
if self.log_stats:
self.stat_logger = PrometheusStatLogger(vllm_config)

executor_backend = (
self.vllm_config.parallel_config.distributed_executor_backend)
Expand Down Expand Up @@ -122,6 +119,15 @@ def __init__(
log_stats=self.log_stats,
)

self.logger_manager: Optional[StatLoggerManager] = None
if self.log_stats:
self.logger_manager = StatLoggerManager(
vllm_config=vllm_config,
custom_stat_loggers=stat_loggers,
enable_default_loggers=log_stats,
)
self.logger_manager.log_engine_initialized()
Comment on lines +122 to +129
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

For better code clarity and to ensure all instance attributes are initialized in one place, it's recommended to initialize _last_log_time here within the if self.log_stats: block. This avoids lazy initialization in the do_log_stats_with_interval method, makes the class's attributes more explicit, and improves overall maintainability.

Suggested change
self.logger_manager: Optional[StatLoggerManager] = None
if self.log_stats:
self.logger_manager = StatLoggerManager(
vllm_config=vllm_config,
custom_stat_loggers=stat_loggers,
enable_default_loggers=log_stats,
)
self.logger_manager.log_engine_initialized()
self.logger_manager: Optional[StatLoggerManager] = None
if self.log_stats:
self.logger_manager = StatLoggerManager(
vllm_config=vllm_config,
custom_stat_loggers=stat_loggers,
enable_default_loggers=log_stats,
)
self.logger_manager.log_engine_initialized()
self._last_log_time = time.time()


if not multiprocess_mode:
# for v0 compatibility
self.model_executor = self.engine_core.engine_core.model_executor # type: ignore
Expand Down Expand Up @@ -269,10 +275,13 @@ def step(self) -> Union[list[RequestOutput], list[PoolingRequestOutput]]:
self.engine_core.abort_requests(processed_outputs.reqs_to_abort)

# 4) Record stats
if self.stat_logger is not None:
if self.logger_manager is not None:
assert outputs.scheduler_stats is not None
self.stat_logger.record(scheduler_stats=outputs.scheduler_stats,
iteration_stats=iteration_stats)
self.logger_manager.record(
scheduler_stats=outputs.scheduler_stats,
iteration_stats=iteration_stats,
)
self.do_log_stats_with_interval()

return processed_outputs.request_outputs

Expand Down Expand Up @@ -315,6 +324,20 @@ def get_tokenizer(self) -> AnyTokenizer:

return self.tokenizer

def do_log_stats(self) -> None:
"""Log stats if logging is enabled."""
if self.logger_manager:
self.logger_manager.log()

def do_log_stats_with_interval(self) -> None:
"""Log stats when the time interval has passed."""
now = time.time()
if not hasattr(self, "_last_log_time"):
self._last_log_time = now
if now - self._last_log_time >= envs.VLLM_LOG_STATS_INTERVAL:
self.do_log_stats()
self._last_log_time = now
Comment on lines +332 to +339
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With _last_log_time being initialized in __init__, this lazy initialization check becomes redundant. Removing it simplifies the do_log_stats_with_interval method, making it cleaner and more straightforward.

Suggested change
def do_log_stats_with_interval(self) -> None:
"""Log stats when the time interval has passed."""
now = time.time()
if not hasattr(self, "_last_log_time"):
self._last_log_time = now
if now - self._last_log_time >= envs.VLLM_LOG_STATS_INTERVAL:
self.do_log_stats()
self._last_log_time = now
def do_log_stats_with_interval(self) -> None:
"""Log stats when the time interval has passed."""
now = time.time()
if now - self._last_log_time >= envs.VLLM_LOG_STATS_INTERVAL:
self.do_log_stats()
self._last_log_time = now


def add_lora(self, lora_request: LoRARequest) -> bool:
"""Load a new LoRA adapter into the engine for future requests."""
return self.engine_core.add_lora(lora_request)
Expand Down
1 change: 0 additions & 1 deletion vllm/v1/metrics/loggers.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ def record(self,
iteration_stats: Optional[IterationStats],
engine_idx: int = 0):
"""Log Stats to standard output."""

if iteration_stats:
self._track_iteration_stats(iteration_stats)

Expand Down