Fix ring buffer gap cleanup code (#578)

When cleaning up gaps the case that a gaps end date can become older
then the oldest stored value in the buffer wasn't covered. Furthermore
the cases for cleaning up a single gap didn't run for the last gaps,
since the loop was stopping to early.

RELEASE_NOTES.md

@@ -16,3 +16,7 @@ Now the microgrid API v0.15.x is being used. The SDK can only connect to microgr
 - The `ConfigManagingActor` constructor now can accept a `pathlib.Path` as `config_path` too (before it accepted only a `str`).
 
 - The `PowerDistributingActor` now considers exclusion bounds, when finding an optimal distribution for power between batteries.
+
+## Bug Fixes
+
+- Fixes a bug in the ring buffer updating the end timestamp of gaps when they are outdated.

src/frequenz/sdk/timeseries/_ringbuffer/buffer.py

@@ -328,17 +328,24 @@ def _cleanup_gaps(self) -> None:
         self._gaps = sorted(self._gaps, key=lambda x: x.start.timestamp())
 
         i = 0
-        while i < len(self._gaps) - 1:
+        while i < len(self._gaps):
             w_1 = self._gaps[i]
-            w_2 = self._gaps[i + 1]
+            if i < len(self._gaps) - 1:
+                w_2 = self._gaps[i + 1]
+            else:
+                w_2 = None
+
             # Delete out-of-date gaps
             if w_1.end <= self._datetime_oldest:
                 del self._gaps[i]
+            # Update start of gap if it's rolled out of the buffer
+            elif w_1.start < self._datetime_oldest:
+                self._gaps[i].start = self._datetime_oldest
             # If w2 is a subset of w1 we can delete it
-            elif w_1.start <= w_2.start and w_1.end >= w_2.end:
+            elif w_2 and w_1.start <= w_2.start and w_1.end >= w_2.end:
                 del self._gaps[i + 1]
             # If the gaps are direct neighbors, merge them
-            elif w_1.end >= w_2.start:
+            elif w_2 and w_1.end >= w_2.start:
                 w_1.end = w_2.end
                 del self._gaps[i + 1]
             else:

tests/timeseries/test_ringbuffer.py

@@ -369,3 +369,46 @@ def test_off_by_one_gap_logic_bug() -> None:
 
     assert buffer.is_missing(times[0]) is False
     assert buffer.is_missing(times[1]) is False
+
+
+def test_cleanup_oldest_gap_timestamp() -> None:
+    """Test that gaps are updated such that they are fully contained in the buffer."""
+    buffer = OrderedRingBuffer(
+        np.empty(shape=15, dtype=float),
+        sampling_period=timedelta(seconds=1),
+        align_to=datetime(1, 1, 1, tzinfo=timezone.utc),
+    )
+
+    for i in range(10):
+        buffer.update(
+            Sample(datetime.fromtimestamp(200 + i, tz=timezone.utc), Quantity(i))
+        )
+
+    gap = Gap(
+        datetime.fromtimestamp(195, tz=timezone.utc),
+        datetime.fromtimestamp(200, tz=timezone.utc),
+    )
+
+    assert gap == buffer.gaps[0]
+
+
+def test_delete_oudated_gap() -> None:
+    """
+    Update the buffer such that the gap is no longer valid.
+    We introduce two gaps and check that the oldest is removed.
+    """
+    buffer = OrderedRingBuffer(
+        np.empty(shape=3, dtype=float),
+        sampling_period=timedelta(seconds=1),
+        align_to=datetime(1, 1, 1, tzinfo=timezone.utc),
+    )
+
+    for i in range(2):
+        buffer.update(
+            Sample(datetime.fromtimestamp(200 + i, tz=timezone.utc), Quantity(i))
+        )
+    assert len(buffer.gaps) == 1
+
+    buffer.update(Sample(datetime.fromtimestamp(202, tz=timezone.utc), Quantity(2)))
+
+    assert len(buffer.gaps) == 0