diff --git a/dimos/protocol/service/system_configurator/base.py b/dimos/protocol/service/system_configurator/base.py index 347699536a..51413ea658 100644 --- a/dimos/protocol/service/system_configurator/base.py +++ b/dimos/protocol/service/system_configurator/base.py @@ -45,7 +45,7 @@ def _read_sysctl_int(name: str) -> int | None: def _write_sysctl_int(name: str, value: int) -> None: - prompt.sudo_run("sysctl", "-w", f"{name}={value}", check=True, text=True, capture_output=False) + prompt.sudo_run("sysctl", "-w", f"{name}={value}", check=True, text=True, capture_output=True) # base class for system config checks/requirements diff --git a/dimos/protocol/service/system_configurator/lcm.py b/dimos/protocol/service/system_configurator/lcm.py index 360d1c988a..13538c5419 100644 --- a/dimos/protocol/service/system_configurator/lcm.py +++ b/dimos/protocol/service/system_configurator/lcm.py @@ -14,10 +14,12 @@ from __future__ import annotations +import json import re import resource import subprocess +from dimos.constants import STATE_DIR from dimos.protocol.service.system_configurator.base import ( SystemConfigurator, _read_sysctl_int, @@ -25,6 +27,21 @@ ) from dimos.utils import prompt +_SYSCTL_CONF = STATE_DIR / "sysctl.json" + + +def _load_sysctl_conf() -> dict[str, int]: + try: + return json.loads(_SYSCTL_CONF.read_text()) # type: ignore[no-any-return] + except (OSError, json.JSONDecodeError): + return {} + + +def _save_sysctl_conf(data: dict[str, int]) -> None: + _SYSCTL_CONF.parent.mkdir(parents=True, exist_ok=True) + _SYSCTL_CONF.write_text(json.dumps(data)) + + # specific checks: multicast @@ -219,39 +236,47 @@ def fix(self) -> None: class BufferConfiguratorMacOS(SystemConfigurator): critical = False - MAX_POSSIBLE_RECVSPACE = 2_097_152 - MAX_POSSIBLE_BUFFER_SIZE = 8_388_608 - MAX_POSSIBLE_DGRAM_SIZE = 65_535 - # these values are based on macos 26 - TARGET_BUFFER_SIZE = MAX_POSSIBLE_BUFFER_SIZE - TARGET_RECVSPACE = MAX_POSSIBLE_RECVSPACE # we want this to be IDEAL_RMEM_SIZE but MacOS 26 (and probably in general) doesn't support it - TARGET_DGRAM_SIZE = MAX_POSSIBLE_DGRAM_SIZE + TARGET = IDEAL_RMEM_SIZE + + KEYS = ( + "kern.ipc.maxsockbuf", + "net.inet.udp.recvspace", + "net.inet.udp.maxdgram", + ) def __init__(self) -> None: - self.needs: list[tuple[str, int]] = [] + self.needs: list[tuple[str, int, int]] = [] # (key, target, current) def check(self) -> bool: self.needs.clear() - for key, target in [ - ("kern.ipc.maxsockbuf", self.TARGET_BUFFER_SIZE), - ("net.inet.udp.recvspace", self.TARGET_RECVSPACE), - ("net.inet.udp.maxdgram", self.TARGET_DGRAM_SIZE), - ]: - current = _read_sysctl_int(key) - if current is None or current < target: - self.needs.append((key, target)) + saved = _load_sysctl_conf() + for key in self.KEYS: + target = saved.get(key, self.TARGET) + current = _read_sysctl_int(key) or 0 + if current < target: + self.needs.append((key, target, current)) return not self.needs def explanation(self) -> str | None: lines = [] - for key, target in self.needs: + for key, target, _ in self.needs: lines.append(f"- socket buffer optimization for LCM: sudo sysctl -w {key}={target}") return "\n".join(lines) def fix(self) -> None: - for key, target in self.needs: - _write_sysctl_int(key, target) + saved = _load_sysctl_conf() + for key, target, current in self.needs: + while target > current: + try: + _write_sysctl_int(key, target) + except subprocess.CalledProcessError: + target //= 2 + else: + saved[key] = target + break + # Write current amounts to config to avoid requesting TARGET every startup. + _save_sysctl_conf(saved) # specific checks: ulimit diff --git a/dimos/protocol/service/test_system_configurator.py b/dimos/protocol/service/test_system_configurator.py index 8b2400aab6..ff41d3f66c 100644 --- a/dimos/protocol/service/test_system_configurator.py +++ b/dimos/protocol/service/test_system_configurator.py @@ -12,9 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import os import resource import struct +import subprocess from unittest.mock import MagicMock, patch import pytest @@ -96,7 +98,7 @@ def test_calls_sudo_run_with_correct_args(self) -> None: ["sudo", "sysctl", "-w", "net.core.rmem_max=67108864"], check=True, text=True, - capture_output=False, + capture_output=True, ) @@ -338,9 +340,9 @@ def test_check_returns_true_when_buffers_sufficient(self) -> None: configurator = BufferConfiguratorMacOS() with patch("dimos.protocol.service.system_configurator.lcm._read_sysctl_int") as mock_read: mock_read.side_effect = [ - BufferConfiguratorMacOS.TARGET_BUFFER_SIZE, - BufferConfiguratorMacOS.TARGET_RECVSPACE, - BufferConfiguratorMacOS.TARGET_DGRAM_SIZE, + BufferConfiguratorMacOS.TARGET, + BufferConfiguratorMacOS.TARGET, + BufferConfiguratorMacOS.TARGET, ] assert configurator.check() is True assert configurator.needs == [] @@ -352,10 +354,27 @@ def test_check_returns_false_when_values_low(self) -> None: assert configurator.check() is False assert len(configurator.needs) == 3 + def test_check_uses_saved_config_as_target(self, tmp_path) -> None: + conf = tmp_path / "sysctl.json" + user_limit = 32 * 2**20 # 32 MiB + conf.write_text(json.dumps({"kern.ipc.maxsockbuf": user_limit})) + configurator = BufferConfiguratorMacOS() + with ( + patch("dimos.protocol.service.system_configurator.lcm._SYSCTL_CONF", conf), + patch("dimos.protocol.service.system_configurator.lcm._read_sysctl_int") as mock_read, + ): + # maxsockbuf: saved target is 32M, current is 32M → ok + # recvspace/maxdgram: no saved value → uses IDEAL (64M) → needs fix + mock_read.side_effect = [user_limit] * 3 + assert configurator.check() is False + assert len(configurator.needs) == 2 + # maxsockbuf should not be in needs + assert all(k != "kern.ipc.maxsockbuf" for k, _, _ in configurator.needs) + def test_explanation_lists_needed_changes(self) -> None: configurator = BufferConfiguratorMacOS() configurator.needs = [ - ("kern.ipc.maxsockbuf", BufferConfiguratorMacOS.TARGET_BUFFER_SIZE), + ("kern.ipc.maxsockbuf", BufferConfiguratorMacOS.TARGET, BufferConfiguratorMacOS.TARGET), ] explanation = configurator.explanation() assert "kern.ipc.maxsockbuf" in explanation @@ -363,16 +382,51 @@ def test_explanation_lists_needed_changes(self) -> None: def test_fix_writes_needed_values(self) -> None: configurator = BufferConfiguratorMacOS() configurator.needs = [ - ("kern.ipc.maxsockbuf", BufferConfiguratorMacOS.TARGET_BUFFER_SIZE), + ("kern.ipc.maxsockbuf", BufferConfiguratorMacOS.TARGET, 0), ] - with patch( - "dimos.protocol.service.system_configurator.lcm._write_sysctl_int" - ) as mock_write: + with ( + patch("dimos.protocol.service.system_configurator.lcm._write_sysctl_int") as mock_write, + patch("dimos.protocol.service.system_configurator.lcm._save_sysctl_conf"), + ): configurator.fix() mock_write.assert_called_once_with( - "kern.ipc.maxsockbuf", BufferConfiguratorMacOS.TARGET_BUFFER_SIZE + "kern.ipc.maxsockbuf", BufferConfiguratorMacOS.TARGET ) + def test_fix_halves_on_failure_and_saves(self, tmp_path) -> None: + conf = tmp_path / "sysctl.json" + configurator = BufferConfiguratorMacOS() + configurator.needs = [("kern.ipc.maxsockbuf", 64000000, 0)] + with ( + patch("dimos.protocol.service.system_configurator.lcm._SYSCTL_CONF", conf), + patch("dimos.protocol.service.system_configurator.lcm._write_sysctl_int") as mock_write, + ): + # Fail at 64M, fail at 32M, succeed at 16M + mock_write.side_effect = [ + subprocess.CalledProcessError(1, "sysctl"), + subprocess.CalledProcessError(1, "sysctl"), + None, + ] + configurator.fix() + assert mock_write.call_count == 3 + mock_write.assert_called_with("kern.ipc.maxsockbuf", 16000000) + # Saved value should be 16M + saved = json.loads(conf.read_text()) + assert saved["kern.ipc.maxsockbuf"] == 16000000 + + def test_fix_stops_at_current_value(self) -> None: + configurator = BufferConfiguratorMacOS() + configurator.needs = [("kern.ipc.maxsockbuf", 64000000, 32000000)] + with ( + patch("dimos.protocol.service.system_configurator.lcm._write_sysctl_int") as mock_write, + patch("dimos.protocol.service.system_configurator.lcm._save_sysctl_conf"), + ): + # All attempts fail — should stop when halved below current (32M) + mock_write.side_effect = subprocess.CalledProcessError(1, "sysctl") + configurator.fix() + # 64M fails, 32M == current → stops + assert mock_write.call_count == 1 + # MaxFileConfiguratorMacOS tests