From aa1711b2fae9cecdb90eddf2fa2e5281d2995ef7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 10:46:23 -1000 Subject: [PATCH 01/15] Always fetch orm rows for latest short term stats maybe fixes #101613 --- .../components/recorder/statistics.py | 23 +++---------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/recorder/statistics.py b/homeassistant/components/recorder/statistics.py index 0ea16e09df4a4..ed454aaf0298e 100644 --- a/homeassistant/components/recorder/statistics.py +++ b/homeassistant/components/recorder/statistics.py @@ -1871,7 +1871,7 @@ def get_latest_short_term_statistics_by_ids( return list( cast( Sequence[Row], - execute_stmt_lambda_element(session, stmt, orm_rows=False), + execute_stmt_lambda_element(session, stmt), ) ) @@ -1924,13 +1924,9 @@ def get_latest_short_term_statistics( for metadata_id in missing_metadata_ids if ( latest_id := cache_latest_short_term_statistic_id_for_metadata_id( - # orm_rows=False is used here because we are in - # a read-only session, and there will never be - # any pending inserts in the session. run_cache, session, metadata_id, - orm_rows=False, ) ) is not None @@ -2316,14 +2312,8 @@ def _import_statistics_with_session( # We just inserted new short term statistics, so we need to update the # ShortTermStatisticsRunCache with the latest id for the metadata_id run_cache = get_short_term_statistics_run_cache(instance.hass) - # - # Because we are in the same session and we want to read rows - # that have not been flushed yet, we need to pass orm_rows=True - # to cache_latest_short_term_statistic_id_for_metadata_id - # to ensure that it gets the rows that were just inserted - # cache_latest_short_term_statistic_id_for_metadata_id( - run_cache, session, metadata_id, orm_rows=True + run_cache, session, metadata_id ) return True @@ -2341,7 +2331,6 @@ def cache_latest_short_term_statistic_id_for_metadata_id( run_cache: ShortTermStatisticsRunCache, session: Session, metadata_id: int, - orm_rows: bool, ) -> int | None: """Cache the latest short term statistic for a given metadata_id. @@ -2352,13 +2341,7 @@ def cache_latest_short_term_statistic_id_for_metadata_id( if latest := cast( Sequence[Row], execute_stmt_lambda_element( - session, - _find_latest_short_term_statistic_for_metadata_id_stmt(metadata_id), - orm_rows=orm_rows - # _import_statistics_with_session needs to be able - # to read back the rows it just inserted without - # a flush so we have to pass orm_rows so we get - # back the latest data. + session, _find_latest_short_term_statistic_for_metadata_id_stmt(metadata_id) ), ): id_: int = latest[0].id From fb7bf6926a4dbb972d44da46f59d665a1cd3e776 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 13:59:54 -1000 Subject: [PATCH 02/15] cover --- tests/components/sensor/test_recorder.py | 31 ++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/components/sensor/test_recorder.py b/tests/components/sensor/test_recorder.py index 34aaeda674058..8ebb69ed28e43 100644 --- a/tests/components/sensor/test_recorder.py +++ b/tests/components/sensor/test_recorder.py @@ -35,10 +35,12 @@ from homeassistant.components.sensor import ATTR_OPTIONS, SensorDeviceClass from homeassistant.const import ATTR_FRIENDLY_NAME, STATE_UNAVAILABLE from homeassistant.core import HomeAssistant, State +from homeassistant.helpers import recorder as recorder_helper from homeassistant.setup import async_setup_component, setup_component import homeassistant.util.dt as dt_util from homeassistant.util.unit_system import METRIC_SYSTEM, US_CUSTOMARY_SYSTEM +from tests.common import get_test_home_assistant from tests.components.recorder.common import ( assert_dict_of_states_equal_without_context_and_last_changed, assert_multiple_states_equal_without_context_and_last_changed, @@ -2148,6 +2150,35 @@ def test_compile_hourly_statistics_unchanged( assert "Error while processing event StatisticsTask" not in caplog.text +def test_compile_missing_statistics(caplog: pytest.LogCaptureFixture) -> None: + """Test compile missing statistics.""" + start_time = dt_util.utcnow() + hass: HomeAssistant = get_test_home_assistant() + recorder_helper.async_initialize_recorder(hass) + wait_recording_done(hass) + wait_recording_done(hass) + + two_days_ago = start_time - timedelta(days=2) + with freeze_time(two_days_ago) as freezer: + setup_component(hass, "sensor", {}) + wait_recording_done(hass) + + past_time = two_days_ago + while past_time < start_time: + freezer.move_to(past_time) + hass.states.set("sensor.test1", "10") + wait_recording_done(hass) + past_time += timedelta(minutes=5) + + wait_recording_done(hass) + hass.stop() + hass: HomeAssistant = get_test_home_assistant() + recorder_helper.async_initialize_recorder(hass) + wait_recording_done(hass) + wait_recording_done(hass) + hass.stop() + + def test_compile_hourly_statistics_partially_unavailable( hass_recorder: Callable[..., HomeAssistant], caplog: pytest.LogCaptureFixture ) -> None: From 388a9f93093d4caa9658cdfbf9746a0506fcb21a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 15:53:52 -1000 Subject: [PATCH 03/15] new test --- tests/components/sensor/test_recorder.py | 31 ------ .../sensor/test_recorder_missing_stats.py | 100 ++++++++++++++++++ 2 files changed, 100 insertions(+), 31 deletions(-) create mode 100644 tests/components/sensor/test_recorder_missing_stats.py diff --git a/tests/components/sensor/test_recorder.py b/tests/components/sensor/test_recorder.py index 8ebb69ed28e43..34aaeda674058 100644 --- a/tests/components/sensor/test_recorder.py +++ b/tests/components/sensor/test_recorder.py @@ -35,12 +35,10 @@ from homeassistant.components.sensor import ATTR_OPTIONS, SensorDeviceClass from homeassistant.const import ATTR_FRIENDLY_NAME, STATE_UNAVAILABLE from homeassistant.core import HomeAssistant, State -from homeassistant.helpers import recorder as recorder_helper from homeassistant.setup import async_setup_component, setup_component import homeassistant.util.dt as dt_util from homeassistant.util.unit_system import METRIC_SYSTEM, US_CUSTOMARY_SYSTEM -from tests.common import get_test_home_assistant from tests.components.recorder.common import ( assert_dict_of_states_equal_without_context_and_last_changed, assert_multiple_states_equal_without_context_and_last_changed, @@ -2150,35 +2148,6 @@ def test_compile_hourly_statistics_unchanged( assert "Error while processing event StatisticsTask" not in caplog.text -def test_compile_missing_statistics(caplog: pytest.LogCaptureFixture) -> None: - """Test compile missing statistics.""" - start_time = dt_util.utcnow() - hass: HomeAssistant = get_test_home_assistant() - recorder_helper.async_initialize_recorder(hass) - wait_recording_done(hass) - wait_recording_done(hass) - - two_days_ago = start_time - timedelta(days=2) - with freeze_time(two_days_ago) as freezer: - setup_component(hass, "sensor", {}) - wait_recording_done(hass) - - past_time = two_days_ago - while past_time < start_time: - freezer.move_to(past_time) - hass.states.set("sensor.test1", "10") - wait_recording_done(hass) - past_time += timedelta(minutes=5) - - wait_recording_done(hass) - hass.stop() - hass: HomeAssistant = get_test_home_assistant() - recorder_helper.async_initialize_recorder(hass) - wait_recording_done(hass) - wait_recording_done(hass) - hass.stop() - - def test_compile_hourly_statistics_partially_unavailable( hass_recorder: Callable[..., HomeAssistant], caplog: pytest.LogCaptureFixture ) -> None: diff --git a/tests/components/sensor/test_recorder_missing_stats.py b/tests/components/sensor/test_recorder_missing_stats.py new file mode 100644 index 0000000000000..98c4ed98a23e9 --- /dev/null +++ b/tests/components/sensor/test_recorder_missing_stats.py @@ -0,0 +1,100 @@ +"""The tests for sensor recorder platform can catch up.""" +from datetime import datetime, timedelta +from pathlib import Path + +from freezegun.api import FrozenDateTimeFactory +import pytest + +from homeassistant.components.recorder import SQLITE_URL_PREFIX +from homeassistant.components.recorder.history import get_significant_states +from homeassistant.components.recorder.statistics import ( + get_latest_short_term_statistics, + statistics_during_period, +) +from homeassistant.core import CoreState, HomeAssistant +from homeassistant.helpers import recorder as recorder_helper +from homeassistant.setup import setup_component +import homeassistant.util.dt as dt_util + +from tests.common import get_test_home_assistant +from tests.components.recorder.common import do_adhoc_statistics, wait_recording_done + +POWER_SENSOR_ATTRIBUTES = { + "device_class": "power", + "state_class": "measurement", + "unit_of_measurement": "kW", +} + + +def test_compile_missing_statistics( + caplog: pytest.LogCaptureFixture, freezer: FrozenDateTimeFactory, tmp_path: Path +) -> None: + """Test compile missing statistics.""" + test_dir = tmp_path.joinpath("sqlite") + test_dir.mkdir() + test_db_file = test_dir.joinpath("test_run_info.db") + dburl = f"{SQLITE_URL_PREFIX}//{test_db_file}" + three_days_ago = datetime(2021, 1, 1, 0, 0, 0, tzinfo=dt_util.UTC) + start_time = three_days_ago + timedelta(days=3) + freezer.move_to(three_days_ago) + hass: HomeAssistant = get_test_home_assistant() + hass.state = CoreState.not_running + recorder_helper.async_initialize_recorder(hass) + setup_component(hass, "sensor", {}) + setup_component(hass, "recorder", {"recorder": {"db_url": dburl}}) + hass.start() + wait_recording_done(hass) + wait_recording_done(hass) + + hass.states.set("sensor.test1", "0", POWER_SENSOR_ATTRIBUTES) + wait_recording_done(hass) + + two_days_ago = three_days_ago + timedelta(days=1) + freezer.move_to(two_days_ago) + do_adhoc_statistics(hass, start=two_days_ago) + wait_recording_done(hass) + + latest = get_latest_short_term_statistics(hass, {"sensor.test1"}, {"state", "sum"}) + latest_stat = latest["sensor.test1"][0] + assert latest_stat["start"] == 1609545600.0 + assert latest_stat["end"] == 1609545600.0 + 300 + count = 1 + past_time = two_days_ago + while past_time < start_time: + freezer.move_to(past_time) + hass.states.set("sensor.test1", str(count), POWER_SENSOR_ATTRIBUTES) + past_time += timedelta(minutes=5) + count += 1 + + wait_recording_done(hass) + + states = get_significant_states(hass, three_days_ago, past_time, ["sensor.test1"]) + assert len(states["sensor.test1"]) == 576 + + hass.stop() + freezer.move_to(start_time) + hass: HomeAssistant = get_test_home_assistant() + hass.state = CoreState.not_running + recorder_helper.async_initialize_recorder(hass) + setup_component(hass, "sensor", {}) + setup_component(hass, "recorder", {"recorder": {"db_url": dburl}}) + hass.start() + wait_recording_done(hass) + wait_recording_done(hass) + + latest = get_latest_short_term_statistics(hass, {"sensor.test1"}, {"state", "sum"}) + latest_stat = latest["sensor.test1"][0] + assert latest_stat["start"] == 1609545600.0 + assert latest_stat["end"] == 1609545600.0 + 300 + + stats = statistics_during_period( + hass, + two_days_ago, + None, + units=None, + statistic_ids={"sensor.test1"}, + period="5minute", + types={"state", "sum"}, + ) + assert stats is not None + hass.stop() From f560d98c17d88d084b4b361f01376886074aeaa4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 16:01:07 -1000 Subject: [PATCH 04/15] missing stats --- .../sensor/test_recorder_missing_stats.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/components/sensor/test_recorder_missing_stats.py b/tests/components/sensor/test_recorder_missing_stats.py index 98c4ed98a23e9..f9be047044831 100644 --- a/tests/components/sensor/test_recorder_missing_stats.py +++ b/tests/components/sensor/test_recorder_missing_stats.py @@ -60,7 +60,7 @@ def test_compile_missing_statistics( assert latest_stat["end"] == 1609545600.0 + 300 count = 1 past_time = two_days_ago - while past_time < start_time: + while past_time <= start_time: freezer.move_to(past_time) hass.states.set("sensor.test1", str(count), POWER_SENSOR_ATTRIBUTES) past_time += timedelta(minutes=5) @@ -69,7 +69,7 @@ def test_compile_missing_statistics( wait_recording_done(hass) states = get_significant_states(hass, three_days_ago, past_time, ["sensor.test1"]) - assert len(states["sensor.test1"]) == 576 + assert len(states["sensor.test1"]) == 577 hass.stop() freezer.move_to(start_time) @@ -77,24 +77,28 @@ def test_compile_missing_statistics( hass.state = CoreState.not_running recorder_helper.async_initialize_recorder(hass) setup_component(hass, "sensor", {}) + hass.states.set("sensor.test1", "0", POWER_SENSOR_ATTRIBUTES) setup_component(hass, "recorder", {"recorder": {"db_url": dburl}}) hass.start() wait_recording_done(hass) wait_recording_done(hass) - latest = get_latest_short_term_statistics(hass, {"sensor.test1"}, {"state", "sum"}) + latest = get_latest_short_term_statistics( + hass, {"sensor.test1"}, {"state", "sum", "max", "mean", "min"} + ) latest_stat = latest["sensor.test1"][0] - assert latest_stat["start"] == 1609545600.0 - assert latest_stat["end"] == 1609545600.0 + 300 + assert latest_stat["start"] == 1609718100.0 + assert latest_stat["end"] == 1609718100.0 + 300 stats = statistics_during_period( hass, two_days_ago, - None, + start_time, units=None, statistic_ids={"sensor.test1"}, period="5minute", - types={"state", "sum"}, + types={"state", "sum", "max", "mean", "min"}, ) + # TODO: this looks wrong assert stats is not None hass.stop() From 06240836e8abd2900d51575afbf5275042ea7c96 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 16:32:01 -1000 Subject: [PATCH 05/15] got problem to happen --- .../components/recorder/statistics.py | 8 +++++ .../sensor/test_recorder_missing_stats.py | 33 +++++++++++++++---- tests/conftest.py | 2 +- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/recorder/statistics.py b/homeassistant/components/recorder/statistics.py index ed454aaf0298e..dec16f6f363d0 100644 --- a/homeassistant/components/recorder/statistics.py +++ b/homeassistant/components/recorder/statistics.py @@ -1695,6 +1695,14 @@ def _statistics_during_period_with_session( table: type[Statistics | StatisticsShortTerm] = ( Statistics if period != "5minute" else StatisticsShortTerm ) + _LOGGER.warning( + "_generate_statistics_during_period_stmt: start_time=%s, end_time=%s, metadata_ids=%s, table=%s, types=%s", + start_time, + end_time, + metadata_ids, + table, + types, + ) stmt = _generate_statistics_during_period_stmt( start_time, end_time, metadata_ids, table, types ) diff --git a/tests/components/sensor/test_recorder_missing_stats.py b/tests/components/sensor/test_recorder_missing_stats.py index f9be047044831..c6c93b0f50777 100644 --- a/tests/components/sensor/test_recorder_missing_stats.py +++ b/tests/components/sensor/test_recorder_missing_stats.py @@ -4,8 +4,9 @@ from freezegun.api import FrozenDateTimeFactory import pytest +from sqlalchemy import select -from homeassistant.components.recorder import SQLITE_URL_PREFIX +from homeassistant.components.recorder import SQLITE_URL_PREFIX, db_schema, get_instance from homeassistant.components.recorder.history import get_significant_states from homeassistant.components.recorder.statistics import ( get_latest_short_term_statistics, @@ -20,9 +21,9 @@ from tests.components.recorder.common import do_adhoc_statistics, wait_recording_done POWER_SENSOR_ATTRIBUTES = { - "device_class": "power", + "device_class": "energy", "state_class": "measurement", - "unit_of_measurement": "kW", + "unit_of_measurement": "kWh", } @@ -90,15 +91,35 @@ def test_compile_missing_statistics( assert latest_stat["start"] == 1609718100.0 assert latest_stat["end"] == 1609718100.0 + 300 + import pprint + + pprint.pprint( + [ + "two_days_ago", + two_days_ago, + two_days_ago.timestamp(), + "start_time", + start_time, + start_time.timestamp(), + ] + ) stats = statistics_during_period( hass, two_days_ago, start_time, - units=None, + units={"energy": "kWh"}, statistic_ids={"sensor.test1"}, - period="5minute", - types={"state", "sum", "max", "mean", "min"}, + period="hour", + types={"change"}, ) + import pprint + + pprint.pprint(["wrong?", stats]) + instance = get_instance(hass) + session = instance.get_session() + result = session.execute(select("*").select_from(db_schema.Statistics)) + pprint.pprint(["all", result.fetchall()]) + # TODO: this looks wrong assert stats is not None hass.stop() diff --git a/tests/conftest.py b/tests/conftest.py index 015cae1720519..ab9bc8233de12 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -104,7 +104,7 @@ _LOGGER = logging.getLogger(__name__) logging.basicConfig(level=logging.INFO) -logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) +logging.getLogger("sqlalchemy.engine").setLevel(logging.DEBUG) asyncio.set_event_loop_policy(runner.HassEventLoopPolicy(False)) # Disable fixtures overriding our beautiful policy From cc63143907ca925ffbbdee2f2553aab3a3b6c459 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 22:01:00 -1000 Subject: [PATCH 06/15] fixes --- .../components/recorder/statistics.py | 122 ++++++++++-------- homeassistant/components/sensor/recorder.py | 27 +--- .../sensor/test_recorder_missing_stats.py | 52 +++----- 3 files changed, 90 insertions(+), 111 deletions(-) diff --git a/homeassistant/components/recorder/statistics.py b/homeassistant/components/recorder/statistics.py index dec16f6f363d0..eae80aaee52ff 100644 --- a/homeassistant/components/recorder/statistics.py +++ b/homeassistant/components/recorder/statistics.py @@ -437,7 +437,6 @@ def compile_missing_statistics(instance: Recorder) -> bool: start = start.replace(minute=0, second=0, microsecond=0) # Commit every 12 hours of data commit_interval = 60 / period_size * 12 - with session_scope( session=instance.get_session(), exception_filter=_filter_unique_constraint_integrity_error(instance), @@ -526,7 +525,7 @@ def _compile_statistics( ): continue compiled: PlatformCompiledStatistics = platform_compile_statistics( - instance.hass, start, end + instance.hass, session, start, end ) _LOGGER.debug( "Statistics for %s during %s-%s: %s", @@ -1902,64 +1901,79 @@ def get_latest_short_term_statistics( metadata: dict[str, tuple[int, StatisticMetaData]] | None = None, ) -> dict[str, list[StatisticsRow]]: """Return the latest short term statistics for a list of statistic_ids.""" + _LOGGER.warning( + "Calling get_latest_short_term_statistics is deprecated; " + "use get_latest_short_term_statistics_with_session instead" + ) with session_scope(hass=hass, read_only=True) as session: - # Fetch metadata for the given statistic_ids - if not metadata: - metadata = get_instance(hass).statistics_meta_manager.get_many( - session, statistic_ids=statistic_ids - ) - if not metadata: - return {} - metadata_ids = set( - _extract_metadata_and_discard_impossible_columns(metadata, types) + return get_latest_short_term_statistics_with_session( + hass, session, statistic_ids, types, metadata ) - run_cache = get_short_term_statistics_run_cache(hass) - # Try to find the latest short term statistics ids for the metadata_ids - # from the run cache first if we have it. If the run cache references - # a non-existent id because of a purge, we will detect it missing in the - # next step and run a query to re-populate the cache. - stats: list[Row] = [] - if metadata_id_to_id := run_cache.get_latest_ids(metadata_ids): - stats = get_latest_short_term_statistics_by_ids( - session, metadata_id_to_id.values() - ) - # If we are missing some metadata_ids in the run cache, we need run a query - # to populate the cache for each metadata_id, and then run another query - # to get the latest short term statistics for the missing metadata_ids. - if (missing_metadata_ids := metadata_ids - set(metadata_id_to_id)) and ( - found_latest_ids := { - latest_id - for metadata_id in missing_metadata_ids - if ( - latest_id := cache_latest_short_term_statistic_id_for_metadata_id( - run_cache, - session, - metadata_id, - ) + + +def get_latest_short_term_statistics_with_session( + hass: HomeAssistant, + session: Session, + statistic_ids: set[str], + types: set[Literal["last_reset", "max", "mean", "min", "state", "sum"]], + metadata: dict[str, tuple[int, StatisticMetaData]] | None = None, +) -> dict[str, list[StatisticsRow]]: + """Return the latest short term statistics for a list of statistic_ids with a session.""" + # Fetch metadata for the given statistic_ids + if not metadata: + metadata = get_instance(hass).statistics_meta_manager.get_many( + session, statistic_ids=statistic_ids + ) + if not metadata: + return {} + metadata_ids = set( + _extract_metadata_and_discard_impossible_columns(metadata, types) + ) + run_cache = get_short_term_statistics_run_cache(hass) + # Try to find the latest short term statistics ids for the metadata_ids + # from the run cache first if we have it. If the run cache references + # a non-existent id because of a purge, we will detect it missing in the + # next step and run a query to re-populate the cache. + stats: list[Row] = [] + if metadata_id_to_id := run_cache.get_latest_ids(metadata_ids): + stats = get_latest_short_term_statistics_by_ids( + session, metadata_id_to_id.values() + ) + # If we are missing some metadata_ids in the run cache, we need run a query + # to populate the cache for each metadata_id, and then run another query + # to get the latest short term statistics for the missing metadata_ids. + if (missing_metadata_ids := metadata_ids - set(metadata_id_to_id)) and ( + found_latest_ids := { + latest_id + for metadata_id in missing_metadata_ids + if ( + latest_id := cache_latest_short_term_statistic_id_for_metadata_id( + run_cache, + session, + metadata_id, ) - is not None - } - ): - stats.extend( - get_latest_short_term_statistics_by_ids(session, found_latest_ids) ) + is not None + } + ): + stats.extend(get_latest_short_term_statistics_by_ids(session, found_latest_ids)) - if not stats: - return {} + if not stats: + return {} - # Return statistics combined with metadata - return _sorted_statistics_to_dict( - hass, - session, - stats, - statistic_ids, - metadata, - False, - StatisticsShortTerm, - None, - None, - types, - ) + # Return statistics combined with metadata + return _sorted_statistics_to_dict( + hass, + session, + stats, + statistic_ids, + metadata, + False, + StatisticsShortTerm, + None, + None, + types, + ) def _generate_statistics_at_time_stmt( diff --git a/homeassistant/components/sensor/recorder.py b/homeassistant/components/sensor/recorder.py index cb5a81d6b84dc..3cf1dc975ec7c 100644 --- a/homeassistant/components/sensor/recorder.py +++ b/homeassistant/components/sensor/recorder.py @@ -16,7 +16,6 @@ get_instance, history, statistics, - util as recorder_util, ) from homeassistant.components.recorder.models import ( StatisticData, @@ -374,27 +373,7 @@ def _timestamp_to_isoformat_or_none(timestamp: float | None) -> str | None: return dt_util.utc_from_timestamp(timestamp).isoformat() -def compile_statistics( - hass: HomeAssistant, start: datetime.datetime, end: datetime.datetime -) -> statistics.PlatformCompiledStatistics: - """Compile statistics for all entities during start-end. - - Note: This will query the database and must not be run in the event loop - """ - # There is already an active session when this code is called since - # it is called from the recorder statistics. We need to make sure - # this session never gets committed since it would be out of sync - # with the recorder statistics session so we mark it as read only. - # - # If we ever need to write to the database from this function we - # will need to refactor the recorder statistics to use a single - # session. - with recorder_util.session_scope(hass=hass, read_only=True) as session: - compiled = _compile_statistics(hass, session, start, end) - return compiled - - -def _compile_statistics( # noqa: C901 +def compile_statistics( # noqa: C901 hass: HomeAssistant, session: Session, start: datetime.datetime, @@ -471,8 +450,8 @@ def _compile_statistics( # noqa: C901 if "sum" in wanted_statistics[entity_id]: to_query.add(entity_id) - last_stats = statistics.get_latest_short_term_statistics( - hass, to_query, {"last_reset", "state", "sum"}, metadata=old_metadatas + last_stats = statistics.get_latest_short_term_statistics_with_session( + hass, session, to_query, {"last_reset", "state", "sum"}, metadata=old_metadatas ) for ( # pylint: disable=too-many-nested-blocks entity_id, diff --git a/tests/components/sensor/test_recorder_missing_stats.py b/tests/components/sensor/test_recorder_missing_stats.py index c6c93b0f50777..9ce8b637e2e8d 100644 --- a/tests/components/sensor/test_recorder_missing_stats.py +++ b/tests/components/sensor/test_recorder_missing_stats.py @@ -4,14 +4,14 @@ from freezegun.api import FrozenDateTimeFactory import pytest -from sqlalchemy import select -from homeassistant.components.recorder import SQLITE_URL_PREFIX, db_schema, get_instance +from homeassistant.components.recorder import SQLITE_URL_PREFIX from homeassistant.components.recorder.history import get_significant_states from homeassistant.components.recorder.statistics import ( - get_latest_short_term_statistics, + get_latest_short_term_statistics_with_session, statistics_during_period, ) +from homeassistant.components.recorder.util import session_scope from homeassistant.core import CoreState, HomeAssistant from homeassistant.helpers import recorder as recorder_helper from homeassistant.setup import setup_component @@ -54,8 +54,10 @@ def test_compile_missing_statistics( freezer.move_to(two_days_ago) do_adhoc_statistics(hass, start=two_days_ago) wait_recording_done(hass) - - latest = get_latest_short_term_statistics(hass, {"sensor.test1"}, {"state", "sum"}) + with session_scope(hass=hass, read_only=True) as session: + latest = get_latest_short_term_statistics_with_session( + hass, session, {"sensor.test1"}, {"state", "sum"} + ) latest_stat = latest["sensor.test1"][0] assert latest_stat["start"] == 1609545600.0 assert latest_stat["end"] == 1609545600.0 + 300 @@ -83,26 +85,16 @@ def test_compile_missing_statistics( hass.start() wait_recording_done(hass) wait_recording_done(hass) - - latest = get_latest_short_term_statistics( - hass, {"sensor.test1"}, {"state", "sum", "max", "mean", "min"} - ) + with session_scope(hass=hass, read_only=True) as session: + latest = get_latest_short_term_statistics_with_session( + hass, session, {"sensor.test1"}, {"state", "sum", "max", "mean", "min"} + ) latest_stat = latest["sensor.test1"][0] assert latest_stat["start"] == 1609718100.0 assert latest_stat["end"] == 1609718100.0 + 300 - - import pprint - - pprint.pprint( - [ - "two_days_ago", - two_days_ago, - two_days_ago.timestamp(), - "start_time", - start_time, - start_time.timestamp(), - ] - ) + assert latest_stat["mean"] == 576.0 + assert latest_stat["min"] == 575.0 + assert latest_stat["max"] == 576.0 stats = statistics_during_period( hass, two_days_ago, @@ -110,16 +102,10 @@ def test_compile_missing_statistics( units={"energy": "kWh"}, statistic_ids={"sensor.test1"}, period="hour", - types={"change"}, + types={"mean"}, ) - import pprint - - pprint.pprint(["wrong?", stats]) - instance = get_instance(hass) - session = instance.get_session() - result = session.execute(select("*").select_from(db_schema.Statistics)) - pprint.pprint(["all", result.fetchall()]) - - # TODO: this looks wrong - assert stats is not None + # Make sure we have 48 hours of statistics + assert len(stats["sensor.test1"]) == 48 + # Make sure the last mean is 570.5 + assert stats["sensor.test1"][-1]["mean"] == 570.5 hass.stop() From def891a25f66d7829ab07df3f73f52533dc5b0f9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 22:03:41 -1000 Subject: [PATCH 07/15] fixes --- tests/components/recorder/test_statistics.py | 100 +++++++++++------- .../components/recorder/test_websocket_api.py | 13 ++- 2 files changed, 72 insertions(+), 41 deletions(-) diff --git a/tests/components/recorder/test_statistics.py b/tests/components/recorder/test_statistics.py index e56b2b8327439..fc66606751652 100644 --- a/tests/components/recorder/test_statistics.py +++ b/tests/components/recorder/test_statistics.py @@ -22,7 +22,7 @@ async_import_statistics, get_last_short_term_statistics, get_last_statistics, - get_latest_short_term_statistics, + get_latest_short_term_statistics_with_session, get_metadata, get_short_term_statistics_run_cache, list_statistic_ids, @@ -71,9 +71,13 @@ def test_compile_hourly_statistics(hass_recorder: Callable[..., HomeAssistant]) assert_dict_of_states_equal_without_context_and_last_changed(states, hist) # Should not fail if there is nothing there yet - stats = get_latest_short_term_statistics( - hass, {"sensor.test1"}, {"last_reset", "max", "mean", "min", "state", "sum"} - ) + with session_scope(hass=hass, read_only=True) as session: + stats = get_latest_short_term_statistics_with_session( + hass, + session, + {"sensor.test1"}, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) assert stats == {} for kwargs in ({}, {"statistic_ids": ["sensor.test1"]}): @@ -154,46 +158,60 @@ def test_compile_hourly_statistics(hass_recorder: Callable[..., HomeAssistant]) assert stats == {} # Test get_last_short_term_statistics and get_latest_short_term_statistics - stats = get_last_short_term_statistics( - hass, - 0, - "sensor.test1", - True, - {"last_reset", "max", "mean", "min", "state", "sum"}, - ) + with session_scope(hass=hass, read_only=True) as session: + stats = get_latest_short_term_statistics_with_session( + hass, + session, + 0, + "sensor.test1", + True, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) assert stats == {} - stats = get_last_short_term_statistics( - hass, - 1, - "sensor.test1", - True, - {"last_reset", "max", "mean", "min", "state", "sum"}, - ) + with session_scope(hass=hass, read_only=True) as session: + stats = get_latest_short_term_statistics_with_session( + hass, + session, + 1, + "sensor.test1", + True, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) assert stats == {"sensor.test1": [expected_2]} - stats = get_latest_short_term_statistics( - hass, {"sensor.test1"}, {"last_reset", "max", "mean", "min", "state", "sum"} - ) + with session_scope(hass=hass, read_only=True) as session: + stats = get_latest_short_term_statistics_with_session( + hass, + session, + {"sensor.test1"}, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) assert stats == {"sensor.test1": [expected_2]} # Now wipe the latest_short_term_statistics_ids table and test again # to make sure we can rebuild the missing data run_cache = get_short_term_statistics_run_cache(instance.hass) run_cache._latest_id_by_metadata_id = {} - stats = get_latest_short_term_statistics( - hass, {"sensor.test1"}, {"last_reset", "max", "mean", "min", "state", "sum"} - ) + with session_scope(hass=hass, read_only=True) as session: + stats = get_latest_short_term_statistics_with_session( + hass, + session, + {"sensor.test1"}, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) assert stats == {"sensor.test1": [expected_2]} metadata = get_metadata(hass, statistic_ids={"sensor.test1"}) - stats = get_latest_short_term_statistics( - hass, - {"sensor.test1"}, - {"last_reset", "max", "mean", "min", "state", "sum"}, - metadata=metadata, - ) + with session_scope(hass=hass, read_only=True) as session: + stats = get_latest_short_term_statistics_with_session( + hass, + session, + {"sensor.test1"}, + {"last_reset", "max", "mean", "min", "state", "sum"}, + metadata=metadata, + ) assert stats == {"sensor.test1": [expected_2]} stats = get_last_short_term_statistics( @@ -225,10 +243,14 @@ def test_compile_hourly_statistics(hass_recorder: Callable[..., HomeAssistant]) instance.get_session().query(StatisticsShortTerm).delete() # Should not fail there is nothing in the table - stats = get_latest_short_term_statistics( - hass, {"sensor.test1"}, {"last_reset", "max", "mean", "min", "state", "sum"} - ) - assert stats == {} + with session_scope(hass=hass, read_only=True) as session: + stats = get_latest_short_term_statistics_with_session( + hass, + session, + {"sensor.test1"}, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) + assert stats == {} # Delete again, and manually wipe the cache since we deleted all the data instance.get_session().query(StatisticsShortTerm).delete() @@ -236,9 +258,13 @@ def test_compile_hourly_statistics(hass_recorder: Callable[..., HomeAssistant]) run_cache._latest_id_by_metadata_id = {} # And test again to make sure there is no data - stats = get_latest_short_term_statistics( - hass, {"sensor.test1"}, {"last_reset", "max", "mean", "min", "state", "sum"} - ) + with session_scope(hass=hass, read_only=True) as session: + stats = get_latest_short_term_statistics_with_session( + hass, + session, + {"sensor.test1"}, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) assert stats == {} diff --git a/tests/components/recorder/test_websocket_api.py b/tests/components/recorder/test_websocket_api.py index 969fdd63ae58f..b371d69fe5f15 100644 --- a/tests/components/recorder/test_websocket_api.py +++ b/tests/components/recorder/test_websocket_api.py @@ -14,11 +14,12 @@ from homeassistant.components.recorder.statistics import ( async_add_external_statistics, get_last_statistics, - get_latest_short_term_statistics, + get_latest_short_term_statistics_with_session, get_metadata, get_short_term_statistics_run_cache, list_statistic_ids, ) +from homeassistant.components.recorder.util import session_scope from homeassistant.components.recorder.websocket_api import UNIT_SCHEMA from homeassistant.components.sensor import UNIT_CONVERTERS from homeassistant.core import HomeAssistant @@ -636,9 +637,13 @@ def next_id(): "change": (imported_stats_5min[-1]["sum"] - imported_stats_5min[0]["sum"]) * 1000, } - stats = get_latest_short_term_statistics( - hass, {"sensor.test"}, {"last_reset", "max", "mean", "min", "state", "sum"} - ) + with session_scope(hass=hass, read_only=True) as session: + stats = get_latest_short_term_statistics_with_session( + hass, + session, + {"sensor.test"}, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) start = imported_stats_5min[-1]["start"].timestamp() end = start + (5 * 60) assert stats == { From b6a640dc0c1054d5ccb9bd458f7d95a9c8b7c8cd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 22:06:48 -1000 Subject: [PATCH 08/15] fixes --- tests/components/recorder/test_statistics.py | 34 +++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/tests/components/recorder/test_statistics.py b/tests/components/recorder/test_statistics.py index fc66606751652..03dc7b84caa57 100644 --- a/tests/components/recorder/test_statistics.py +++ b/tests/components/recorder/test_statistics.py @@ -158,26 +158,22 @@ def test_compile_hourly_statistics(hass_recorder: Callable[..., HomeAssistant]) assert stats == {} # Test get_last_short_term_statistics and get_latest_short_term_statistics - with session_scope(hass=hass, read_only=True) as session: - stats = get_latest_short_term_statistics_with_session( - hass, - session, - 0, - "sensor.test1", - True, - {"last_reset", "max", "mean", "min", "state", "sum"}, - ) + stats = get_last_short_term_statistics( + hass, + 0, + "sensor.test1", + True, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) assert stats == {} - with session_scope(hass=hass, read_only=True) as session: - stats = get_latest_short_term_statistics_with_session( - hass, - session, - 1, - "sensor.test1", - True, - {"last_reset", "max", "mean", "min", "state", "sum"}, - ) + stats = get_last_short_term_statistics( + hass, + 1, + "sensor.test1", + True, + {"last_reset", "max", "mean", "min", "state", "sum"}, + ) assert stats == {"sensor.test1": [expected_2]} with session_scope(hass=hass, read_only=True) as session: @@ -285,7 +281,7 @@ def sensor_stats(entity_id, start): "stat": {"start": start}, } - def get_fake_stats(_hass, start, _end): + def get_fake_stats(_hass, session, start, _end): return statistics.PlatformCompiledStatistics( [ sensor_stats("sensor.test1", start), From 9acc20c7d1d2bc64df0c5105399ef8372ab139b6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 22:07:51 -1000 Subject: [PATCH 09/15] merge --- homeassistant/components/recorder/statistics.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/homeassistant/components/recorder/statistics.py b/homeassistant/components/recorder/statistics.py index eae80aaee52ff..83cbdd71eeafa 100644 --- a/homeassistant/components/recorder/statistics.py +++ b/homeassistant/components/recorder/statistics.py @@ -437,6 +437,7 @@ def compile_missing_statistics(instance: Recorder) -> bool: start = start.replace(minute=0, second=0, microsecond=0) # Commit every 12 hours of data commit_interval = 60 / period_size * 12 + with session_scope( session=instance.get_session(), exception_filter=_filter_unique_constraint_integrity_error(instance), @@ -1694,14 +1695,6 @@ def _statistics_during_period_with_session( table: type[Statistics | StatisticsShortTerm] = ( Statistics if period != "5minute" else StatisticsShortTerm ) - _LOGGER.warning( - "_generate_statistics_during_period_stmt: start_time=%s, end_time=%s, metadata_ids=%s, table=%s, types=%s", - start_time, - end_time, - metadata_ids, - table, - types, - ) stmt = _generate_statistics_during_period_stmt( start_time, end_time, metadata_ids, table, types ) From ab8be71bb206419ab6dfd463b512ca23b176c278 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 22:08:27 -1000 Subject: [PATCH 10/15] merge --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index ab9bc8233de12..015cae1720519 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -104,7 +104,7 @@ _LOGGER = logging.getLogger(__name__) logging.basicConfig(level=logging.INFO) -logging.getLogger("sqlalchemy.engine").setLevel(logging.DEBUG) +logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) asyncio.set_event_loop_policy(runner.HassEventLoopPolicy(False)) # Disable fixtures overriding our beautiful policy From 250d83d75a1a6d72e4fd2f17fab43785264b4fdf Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 22:45:48 -1000 Subject: [PATCH 11/15] energy tests --- tests/components/energy/test_sensor.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/components/energy/test_sensor.py b/tests/components/energy/test_sensor.py index f5fea153380e7..bf1513507f882 100644 --- a/tests/components/energy/test_sensor.py +++ b/tests/components/energy/test_sensor.py @@ -6,6 +6,7 @@ import pytest from homeassistant.components.energy import data +from homeassistant.components.recorder.util import session_scope from homeassistant.components.sensor import ( ATTR_LAST_RESET, ATTR_STATE_CLASS, @@ -155,7 +156,10 @@ async def test_cost_sensor_price_entity_total_increasing( """Test energy cost price from total_increasing type sensor entity.""" def _compile_statistics(_): - return compile_statistics(hass, now, now + timedelta(seconds=1)).platform_stats + with session_scope(hass=hass) as session: + return compile_statistics( + hass, session, now, now + timedelta(seconds=1) + ).platform_stats energy_attributes = { ATTR_UNIT_OF_MEASUREMENT: UnitOfEnergy.KILO_WATT_HOUR, @@ -365,9 +369,10 @@ async def test_cost_sensor_price_entity_total( """Test energy cost price from total type sensor entity.""" def _compile_statistics(_): - return compile_statistics( - hass, now, now + timedelta(seconds=0.17) - ).platform_stats + with session_scope(hass=hass) as session: + return compile_statistics( + hass, session, now, now + timedelta(seconds=0.17) + ).platform_stats energy_attributes = { ATTR_UNIT_OF_MEASUREMENT: UnitOfEnergy.KILO_WATT_HOUR, @@ -579,7 +584,10 @@ async def test_cost_sensor_price_entity_total_no_reset( """Test energy cost price from total type sensor entity with no last_reset.""" def _compile_statistics(_): - return compile_statistics(hass, now, now + timedelta(seconds=1)).platform_stats + with session_scope(hass=hass) as session: + return compile_statistics( + hass, session, now, now + timedelta(seconds=1) + ).platform_stats energy_attributes = { ATTR_UNIT_OF_MEASUREMENT: UnitOfEnergy.KILO_WATT_HOUR, From b6f1f87fd80467ddb9c8d3b102e9da87543b6e11 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Oct 2023 23:08:51 -1000 Subject: [PATCH 12/15] avoid timeout on postgresql --- tests/components/sensor/test_recorder_missing_stats.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/components/sensor/test_recorder_missing_stats.py b/tests/components/sensor/test_recorder_missing_stats.py index 9ce8b637e2e8d..1fddf40dec8a8 100644 --- a/tests/components/sensor/test_recorder_missing_stats.py +++ b/tests/components/sensor/test_recorder_missing_stats.py @@ -27,6 +27,7 @@ } +@pytest.mark.timeout(25) def test_compile_missing_statistics( caplog: pytest.LogCaptureFixture, freezer: FrozenDateTimeFactory, tmp_path: Path ) -> None: From fd3bc756a2366fa37220b261a7e878c045995209 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Oct 2023 05:07:51 -1000 Subject: [PATCH 13/15] remove get_latest_short_term_statistics as its unused now --- homeassistant/components/recorder/statistics.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/homeassistant/components/recorder/statistics.py b/homeassistant/components/recorder/statistics.py index 83cbdd71eeafa..a6fe7ddb22f9d 100644 --- a/homeassistant/components/recorder/statistics.py +++ b/homeassistant/components/recorder/statistics.py @@ -1887,23 +1887,6 @@ def _latest_short_term_statistics_by_ids_stmt( ) -def get_latest_short_term_statistics( - hass: HomeAssistant, - statistic_ids: set[str], - types: set[Literal["last_reset", "max", "mean", "min", "state", "sum"]], - metadata: dict[str, tuple[int, StatisticMetaData]] | None = None, -) -> dict[str, list[StatisticsRow]]: - """Return the latest short term statistics for a list of statistic_ids.""" - _LOGGER.warning( - "Calling get_latest_short_term_statistics is deprecated; " - "use get_latest_short_term_statistics_with_session instead" - ) - with session_scope(hass=hass, read_only=True) as session: - return get_latest_short_term_statistics_with_session( - hass, session, statistic_ids, types, metadata - ) - - def get_latest_short_term_statistics_with_session( hass: HomeAssistant, session: Session, From dba417b831b508707a88477ff138814b550b1056 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Oct 2023 05:30:35 -1000 Subject: [PATCH 14/15] make test work on postgresql/mysql --- .../sensor/test_recorder_missing_stats.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/components/sensor/test_recorder_missing_stats.py b/tests/components/sensor/test_recorder_missing_stats.py index 1fddf40dec8a8..08b3ef6272907 100644 --- a/tests/components/sensor/test_recorder_missing_stats.py +++ b/tests/components/sensor/test_recorder_missing_stats.py @@ -5,7 +5,6 @@ from freezegun.api import FrozenDateTimeFactory import pytest -from homeassistant.components.recorder import SQLITE_URL_PREFIX from homeassistant.components.recorder.history import get_significant_states from homeassistant.components.recorder.statistics import ( get_latest_short_term_statistics_with_session, @@ -29,13 +28,16 @@ @pytest.mark.timeout(25) def test_compile_missing_statistics( - caplog: pytest.LogCaptureFixture, freezer: FrozenDateTimeFactory, tmp_path: Path + freezer: FrozenDateTimeFactory, recorder_db_url: str, tmp_path: Path ) -> None: """Test compile missing statistics.""" - test_dir = tmp_path.joinpath("sqlite") - test_dir.mkdir() - test_db_file = test_dir.joinpath("test_run_info.db") - dburl = f"{SQLITE_URL_PREFIX}//{test_db_file}" + if recorder_db_url == "sqlite://": + # On-disk database because we need to stop and start hass + # and have it persist. + recorder_db_url = "sqlite:///" + str(tmp_path / "pytest.db") + config = { + "db_url": recorder_db_url, + } three_days_ago = datetime(2021, 1, 1, 0, 0, 0, tzinfo=dt_util.UTC) start_time = three_days_ago + timedelta(days=3) freezer.move_to(three_days_ago) @@ -43,7 +45,7 @@ def test_compile_missing_statistics( hass.state = CoreState.not_running recorder_helper.async_initialize_recorder(hass) setup_component(hass, "sensor", {}) - setup_component(hass, "recorder", {"recorder": {"db_url": dburl}}) + setup_component(hass, "recorder", {"recorder": config}) hass.start() wait_recording_done(hass) wait_recording_done(hass) @@ -82,7 +84,7 @@ def test_compile_missing_statistics( recorder_helper.async_initialize_recorder(hass) setup_component(hass, "sensor", {}) hass.states.set("sensor.test1", "0", POWER_SENSOR_ATTRIBUTES) - setup_component(hass, "recorder", {"recorder": {"db_url": dburl}}) + setup_component(hass, "recorder", {"recorder": config}) hass.start() wait_recording_done(hass) wait_recording_done(hass) From 444eba631102289da9feca9c9663e73e243a2521 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Oct 2023 05:48:10 -1000 Subject: [PATCH 15/15] patch out issue since we are manually stop/start and we still test on mariadb 10.3 --- tests/components/sensor/test_recorder_missing_stats.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/components/sensor/test_recorder_missing_stats.py b/tests/components/sensor/test_recorder_missing_stats.py index 08b3ef6272907..f6f6445a0fb02 100644 --- a/tests/components/sensor/test_recorder_missing_stats.py +++ b/tests/components/sensor/test_recorder_missing_stats.py @@ -1,6 +1,7 @@ """The tests for sensor recorder platform can catch up.""" from datetime import datetime, timedelta from pathlib import Path +from unittest.mock import patch from freezegun.api import FrozenDateTimeFactory import pytest @@ -26,6 +27,15 @@ } +@pytest.fixture(autouse=True) +def disable_db_issue_creation(): + """Disable the creation of the database issue.""" + with patch( + "homeassistant.components.recorder.util._async_create_mariadb_range_index_regression_issue" + ): + yield + + @pytest.mark.timeout(25) def test_compile_missing_statistics( freezer: FrozenDateTimeFactory, recorder_db_url: str, tmp_path: Path