Skip to content

Commit

Permalink
Throttle connectivity check on connectivity issue (#5342)
Browse files Browse the repository at this point in the history
* Throttle connectivity check on connectivity issue

If Supervisor detects a connectivity issue, currenlty every function
which requires internet get delayed by 10s due to the connectivity
check. This especially slows down initial startup when there are
connectivity issues. It is unlikely to resolve immeaditly, so throttle
the connectivity check to check every 30s.

* Fix pytest

* Reset throttle in test and refactor helper

* CodeRabbit suggestion

---------

Co-authored-by: Mike Degatano <[email protected]>
  • Loading branch information
agners and mdegat01 authored Oct 10, 2024
1 parent d5f33de commit 180a7c3
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 6 deletions.
2 changes: 1 addition & 1 deletion supervisor/supervisor.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def _check_connectivity_throttle_period(coresys: CoreSys, *_) -> timedelta:
if coresys.supervisor.connectivity:
return timedelta(minutes=10)

return timedelta()
return timedelta(seconds=30)


class Supervisor(CoreSysAttributes):
Expand Down
17 changes: 17 additions & 0 deletions tests/common.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
"""Common test functions."""

from datetime import datetime
from importlib import import_module
from inspect import getclosurevars
import json
from pathlib import Path
from typing import Any

from dbus_fast.aio.message_bus import MessageBus

from supervisor.jobs.decorator import Job
from supervisor.resolution.validate import get_valid_modules
from supervisor.utils.yaml import read_yaml_file

Expand Down Expand Up @@ -82,3 +85,17 @@ async def mock_dbus_services(
services[module] = service_module.setup(to_mock[module]).export(bus)

return services


def get_job_decorator(func) -> Job:
"""Get Job object of decorated function."""
# Access the closure of the wrapper function
job = getclosurevars(func).nonlocals["self"]
if not isinstance(job, Job):
raise TypeError(f"{func.__qualname__} is not a Job")
return job


def reset_last_call(func, group: str | None = None) -> None:
"""Reset last call for a function using the Job decorator."""
get_job_decorator(func).set_last_call(datetime.min, group)
4 changes: 4 additions & 0 deletions tests/jobs/test_job_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@
from supervisor.os.manager import OSManager
from supervisor.plugins.audio import PluginAudio
from supervisor.resolution.const import UnhealthyReason
from supervisor.supervisor import Supervisor
from supervisor.utils.dt import utcnow

from tests.common import reset_last_call


async def test_healthy(coresys: CoreSys, caplog: pytest.LogCaptureFixture):
"""Test the healty decorator."""
Expand Down Expand Up @@ -73,6 +76,7 @@ async def test_internet(
):
"""Test the internet decorator."""
coresys.core.state = CoreState.RUNNING
reset_last_call(Supervisor.check_connectivity)

class TestClass:
"""Test class."""
Expand Down
25 changes: 20 additions & 5 deletions tests/test_supervisor.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
"""Test supervisor object."""

from datetime import datetime
from datetime import datetime, timedelta
import errno
from unittest.mock import AsyncMock, Mock, PropertyMock, patch

from aiohttp import ClientTimeout
from aiohttp.client_exceptions import ClientError
from awesomeversion import AwesomeVersion
import pytest
from time_machine import travel

from supervisor.const import UpdateChannel
from supervisor.coresys import CoreSys
Expand All @@ -22,6 +23,8 @@
from supervisor.resolution.data import Issue
from supervisor.supervisor import Supervisor

from tests.common import reset_last_call


@pytest.fixture(name="websession", scope="function")
async def fixture_webession(coresys: CoreSys) -> AsyncMock:
Expand Down Expand Up @@ -58,21 +61,33 @@ async def test_connectivity_check(
assert supervisor_unthrottled.connectivity is connectivity


@pytest.mark.parametrize("side_effect,call_count", [(ClientError(), 3), (None, 1)])
@pytest.mark.parametrize(
"side_effect,call_interval,throttled",
[
(None, timedelta(minutes=5), True),
(None, timedelta(minutes=15), False),
(ClientError(), timedelta(seconds=20), True),
(ClientError(), timedelta(seconds=40), False),
],
)
async def test_connectivity_check_throttling(
coresys: CoreSys,
websession: AsyncMock,
side_effect: Exception | None,
call_count: int,
call_interval: timedelta,
throttled: bool,
):
"""Test connectivity check throttled when checks succeed."""
coresys.supervisor.connectivity = None
websession.head.side_effect = side_effect

for _ in range(3):
reset_last_call(Supervisor.check_connectivity)
with travel(datetime.now(), tick=False) as traveller:
await coresys.supervisor.check_connectivity()
traveller.shift(call_interval)
await coresys.supervisor.check_connectivity()

assert websession.head.call_count == call_count
assert websession.head.call_count == (1 if throttled else 2)


async def test_update_failed(coresys: CoreSys, capture_exception: Mock):
Expand Down

0 comments on commit 180a7c3

Please sign in to comment.