Skip to content
Open
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
4 changes: 4 additions & 0 deletions python/ray/_private/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,10 @@ def start_raylet(
node_name=self._ray_params.node_name,
webui=self._webui_url,
resource_isolation_config=self.resource_isolation_config,
enable_gpu_metrics_collection=self._config.get(
"enable_gpu_metrics_collection",
ray_constants.env_bool("RAY_enable_gpu_metrics_collection", True),
),
)
assert ray_constants.PROCESS_TYPE_RAYLET not in self.all_processes
self.all_processes[ray_constants.PROCESS_TYPE_RAYLET] = [process_info]
Expand Down
6 changes: 6 additions & 0 deletions python/ray/_private/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -1658,6 +1658,7 @@ def start_raylet(
env_updates: Optional[dict] = None,
node_name: Optional[str] = None,
webui: Optional[str] = None,
enable_gpu_metrics_collection: bool = True,
):
"""Start a raylet, which is a combined local scheduler and object manager.

Expand Down Expand Up @@ -1741,6 +1742,8 @@ def start_raylet(
env_updates: Environment variable overrides.
node_name: The name of the node.
webui: The url of the UI.
enable_gpu_metrics_collection: Whether the dashboard agent should collect
GPU metrics.
Returns:
ProcessInfo for the process that was started.
"""
Expand Down Expand Up @@ -1915,6 +1918,9 @@ def start_raylet(
if is_head_node:
dashboard_agent_command.append("--head")

if not enable_gpu_metrics_collection:
dashboard_agent_command.append("--disable-gpu-metrics")

runtime_env_agent_command = [
*_build_python_executable_command_memory_profileable(
ray_constants.PROCESS_TYPE_RUNTIME_ENV_AGENT, session_dir
Expand Down
8 changes: 8 additions & 0 deletions python/ray/dashboard/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def __init__(
events_export_addr=None,
listen_port=ray_constants.DEFAULT_DASHBOARD_AGENT_LISTEN_PORT,
disable_metrics_collection: bool = False,
disable_gpu_metrics: bool = False,
is_head: bool = False,
Comment on lines 51 to 53

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Adding disable_gpu_metrics before is_head changes the positional argument order of DashboardAgent.__init__. If any callers or tests instantiate DashboardAgent using positional arguments (e.g., passing is_head positionally), this change will silently break backward compatibility by misaligning the arguments.

To maintain backward compatibility, please place disable_gpu_metrics after is_head (right before the * keyword-only argument marker).

Suggested change
disable_metrics_collection: bool = False,
disable_gpu_metrics: bool = False,
is_head: bool = False,
disable_metrics_collection: bool = False,
is_head: bool = False,
disable_gpu_metrics: bool = False,

*, # the following are required kwargs
object_store_name: str,
Expand Down Expand Up @@ -80,6 +81,7 @@ def __init__(
self.raylet_name = raylet_name
self.node_id = os.environ["RAY_NODE_ID"]
self.metrics_collection_disabled = disable_metrics_collection
self.gpu_metrics_enabled = not disable_gpu_metrics
self.session_name = session_name

# grpc server is None in mininal.
Expand Down Expand Up @@ -446,6 +448,11 @@ async def wait_forever():
action="store_true",
help=("If this arg is set, metrics report won't be enabled from the agent."),
)
parser.add_argument(
"--disable-gpu-metrics",
action="store_true",
help="Disable GPU metric collection in the dashboard agent.",
)
parser.add_argument(
"--head",
action="store_true",
Expand Down Expand Up @@ -520,6 +527,7 @@ async def wait_forever():
object_store_name=args.object_store_name,
raylet_name=args.raylet_name,
disable_metrics_collection=args.disable_metrics_collection,
disable_gpu_metrics=args.disable_gpu_metrics,
is_head=args.head,
session_name=args.session_name,
)
Expand Down
8 changes: 6 additions & 2 deletions python/ray/dashboard/modules/reporter/gpu_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,14 +526,18 @@ def get_gpu_utilization(self) -> List[GpuUtilizationInfo]:
class GpuMetricProvider:
"""Provider class for GPU metrics collection."""

def __init__(self):
def __init__(self, enable_metric_report: bool = True):
self._provider: Optional[GpuProvider] = None
self._enable_metric_report = True
self._enable_metric_report = enable_metric_report
self._providers = [NvidiaGpuProvider(), AmdGpuProvider()]
Comment on lines +529 to 532

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When enable_metric_report is False, there is no need to instantiate the GPU providers (NvidiaGpuProvider and AmdGpuProvider). Instantiating them unnecessarily consumes memory and could potentially trigger side effects (such as loading driver libraries or executing system queries) during initialization.

We can optimize this by only instantiating the providers when enable_metric_report is True.

Suggested change
def __init__(self, enable_metric_report: bool = True):
self._provider: Optional[GpuProvider] = None
self._enable_metric_report = True
self._enable_metric_report = enable_metric_report
self._providers = [NvidiaGpuProvider(), AmdGpuProvider()]
def __init__(self, enable_metric_report: bool = True):
self._provider: Optional[GpuProvider] = None
self._enable_metric_report = enable_metric_report
self._providers = [NvidiaGpuProvider(), AmdGpuProvider()] if enable_metric_report else []

self._initialized = False

def initialize(self) -> bool:
"""Initialize the GPU metric provider by detecting available GPU providers."""
if not self._enable_metric_report:
self._initialized = True
return False

if self._initialized:
return True

Expand Down
4 changes: 3 additions & 1 deletion python/ray/dashboard/modules/reporter/reporter_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,9 @@ def __init__(self, dashboard_agent, raylet_client=None):
)

# Create GPU metric provider instance
self._gpu_metric_provider = GpuMetricProvider()
self._gpu_metric_provider = GpuMetricProvider(
enable_metric_report=dashboard_agent.gpu_metrics_enabled
)
Comment on lines +581 to +583

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Directly accessing dashboard_agent.gpu_metrics_enabled can raise an AttributeError if dashboard_agent is a mock object (common in unit tests) or a custom agent implementation that does not define this attribute.

Using getattr with a default value of True is a safer, more defensive approach that prevents potential runtime or test failures.

Suggested change
self._gpu_metric_provider = GpuMetricProvider(
enable_metric_report=dashboard_agent.gpu_metrics_enabled
)
self._gpu_metric_provider = GpuMetricProvider(
enable_metric_report=getattr(dashboard_agent, "gpu_metrics_enabled", True)
)


if raylet_client:
self._raylet_client = raylet_client
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,18 @@ def test_init(self):
self.assertEqual(len(self.provider._providers), 2)
self.assertFalse(self.provider._initialized)

@patch.object(GpuMetricProvider, "_detect_gpu_provider")
def test_initialize_disabled(self, mock_detect):
"""Test disabled GPU metrics do not probe GPU providers."""
provider = GpuMetricProvider(enable_metric_report=False)

self.assertFalse(provider.initialize())
self.assertFalse(provider.initialize())
self.assertEqual(provider.get_gpu_usage(), [])
self.assertTrue(provider._initialized)
self.assertFalse(provider.is_metric_report_enabled())
mock_detect.assert_not_called()

@patch.object(NvidiaGpuProvider, "is_available", return_value=True)
@patch.object(AmdGpuProvider, "is_available", return_value=False)
def test_detect_gpu_provider_nvidia(
Expand Down
4 changes: 4 additions & 0 deletions src/ray/common/ray_config_def.h
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,10 @@ RAY_CONFIG(uint64_t, gcs_mark_task_failed_on_worker_dead_delay_ms, /* 1 secs */
/// Whether or not we enable metrics collection.
RAY_CONFIG(bool, enable_metrics_collection, true)

/// Whether the dashboard agent collects GPU metrics. Disabling this avoids
/// polling GPU management libraries such as NVML.
RAY_CONFIG(bool, enable_gpu_metrics_collection, true)

/// Determine if the high cardinality labels such as WorkerId, task and actor Name
/// should be used in the metrics. For the complete definition, see
/// RAY_METRIC_CARDINALITY_LEVEL in ray_constants.py
Expand Down
Loading